[PATCH] D126891: [clang-tidy] The check should ignore final classes

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

steakhal updated this revision to Diff 435054.
steakhal marked an inline comment as done.
steakhal added a comment.

- rebase
- use double-backticks for the keyword `final` in the Release Notes.

polite ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126891

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -320,3 +320,22 @@
 #undef XMACRO
 #undef CONCAT
 } // namespace macro_tests
+
+namespace FinalClassCannotBeBaseClass {
+class Base {
+public:
+  Base() = default;
+  virtual void func() = 0;
+
+protected:
+  ~Base() = default;
+};
+
+// no-warning: 'MostDerived' cannot be a base class, since it's marked 'final'.
+class MostDerived final : public Base {
+public:
+  MostDerived() = default;
+  ~MostDerived() = default;
+  void func() final;
+};
+} // namespace FinalClassCannotBeBaseClass
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -165,6 +165,12 @@
   Fixed an issue when there was already an initializer in the constructor and
   the check would try to create another initializer for the same member.
 
+- Fixed a false positive in :doc:`cppcoreguidelines-virtual-class-destructor
+  ` involving
+  ``final`` classes. The check will not diagnose ``final`` marked classes, 
since
+  those cannot be used as base classes, consequently they can not violate the
+  rule.
+
 - Fixed a crash in :doc:`llvmlibc-callee-namespace
   ` when executing for C++ code
   that contain calls to advanced constructs, e.g. overloaded operators.
Index: 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===
--- 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ 
clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -41,6 +41,7 @@
   Finder->addMatcher(
   cxxRecordDecl(
   anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
+  unless(isFinal()),
   unless(hasPublicVirtualOrProtectedNonVirtualDestructor()))
   .bind("ProblematicClassOrStruct"),
   this);


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -320,3 +320,22 @@
 #undef XMACRO
 #undef CONCAT
 } // namespace macro_tests
+
+namespace FinalClassCannotBeBaseClass {
+class Base {
+public:
+  Base() = default;
+  virtual void func() = 0;
+
+protected:
+  ~Base() = default;
+};
+
+// no-warning: 'MostDerived' cannot be a base class, since it's marked 'final'.
+class MostDerived final : public Base {
+public:
+  MostDerived() = default;
+  ~MostDerived() = default;
+  void func() final;
+};
+} // namespace FinalClassCannotBeBaseClass
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -165,6 +165,12 @@
   Fixed an issue when there was already an initializer in the constructor and
   the check would try to create another initializer for the same member.
 
+- Fixed a false positive in :doc:`cppcoreguidelines-virtual-class-destructor
+  ` involving
+  ``final`` classes. The check will not diagnose ``final`` marked classes, since
+  those cannot be used as base classes, consequently they can not violate the
+  rule.
+
 - Fixed a crash in :doc:`llvmlibc-callee-namespace
   ` when executing for C++ code
   that contain calls to advanced constructs, e.g. overloaded operators.
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
@@ -41,6 +41,7 @@
   Finder->addMatcher(
   cxxRecordDecl(
   anyOf(has(cxxMethodDecl(isVirtual())), InheritsVirtualMethod),
+  unless(isFinal()),
   un

[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

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

steakhal added a comment.

I believe, the silence from the code owner means that he agrees with this 
change.
I'll land it tomorrow.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126215

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

[PATCH] D127105: [analyzer] Fix null pointer deref in CastValueChecker

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

steakhal added a comment.

Submitted a backport request to land this in `clang-14.0.5`, since many `llvm` 
devs might be affected by this crash due to `clangd` and `clang-tidy` 
integration.
https://github.com/llvm/llvm-project/issues/55937


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127105

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

[PATCH] D127269: Add llvm's Support lib to the psuedoCXX library

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127269

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

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

[Nathan James via Phabricator via cfe-commits]

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

LGTM, just a couple points but not essential.




Comment at: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp:184-187
+  if (ScopesCache.find(LocalScope) == ScopesCache.end())
+ScopesCache.insert(std::make_pair(
+LocalScope,
+std::make_unique(*LocalScope, *Context)));





Comment at: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h:35
+TransformPointersAsValues(
+Options.get("TransformPointersAsValues", false)) {}
+

It may be worth adding some validation to these. If AnalyzeValues, 
AnalyzeReferences and WarnPointersAsValues are all false, this whole check is 
basically a no-op.


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

https://reviews.llvm.org/D54943

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

[PATCH] D127224: [flang][OpenMP] Lowering support for atomic capture

[Nimish Mishra via Phabricator via cfe-commits]

NimishMishra updated this revision to Diff 435060.
NimishMishra added a comment.
Herald added subscribers: cfe-commits, llvm-commits, libc-commits, 
libcxx-commits, openmp-commits, lldb-commits, Sanitizers, jsji, Enna1, kosarev, 
mravishankar, jsilvanus, mattd, gchakrabarti, hsmhsm, pmatos, asb, 
pcwang-thead, yota9, ayermolo, awarzynski, arjunp, luke957, asavonic, 
carlosgalvezp, abrachet, Groverkss, armkevincheng, ormris, foad, jsmolens, 
eric-k256, mstorsjo, frasercrmck, ecnelises, ThomasRaoux, vkmr, martong, 
phosek, kerbowa, csigg, luismarques, apazos, sameer.abuasal, pengfei, 
s.egerton, dmgreen, Jim, jocewei, PkmX, arphaman, the_o, brucehoult, 
MartinMosbeck, rogfer01, steven_wu, atanasyan, mgrang, edward-jones, zzheng, 
MaskRay, jrtc27, gbedwell, niosHD, cryptoad, sabuasal, simoncook, johnrusso, 
rbar, fedor.sergeev, kbarton, aheejin, hiraditya, jgravelle-google, 
arichardson, sbc100, mgorny, nhaehnle, jvesely, nemanjai, sdardis, dylanmckay, 
jyknight, dschuff, arsenm, jholewinski.
Herald added a reviewer: bollu.
Herald added a reviewer: andreadb.
Herald added a reviewer: alexander-shaposhnikov.
Herald added a reviewer: rupprecht.
Herald added a reviewer: jhenderson.
Herald added a reviewer: antiagainst.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: aartbik.
Herald added a reviewer: ftynse.
Herald added a reviewer: aartbik.
Herald added a reviewer: aartbik.
Herald added a reviewer: sjarus.
Herald added a reviewer: zuban32.
Herald added a reviewer: rafauler.
Herald added a reviewer: Amir.
Herald added a reviewer: maksfb.
Herald added projects: clang, Sanitizers, LLDB, libc++, libc-project, 
libc++abi, libunwind, LLVM, lld-macho, clang-tools-extra.
Herald added a reviewer: libc++.
Herald added a reviewer: libc++abi.
Herald added a reviewer: libunwind.
Herald added a reviewer: lld-macho.

[flang][OpenMP] Added lowering support for atomic capture


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127224

Files:
  bolt/lib/Rewrite/DWARFRewriter.cpp
  bolt/lib/Rewrite/MachORewriteInstance.cpp
  bolt/lib/Rewrite/RewriteInstance.cpp
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/pseudo/lib/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/empty-member-spec.cpp
  clang-tools-extra/pseudo/test/cxx/keyword.cpp
  clang-tools-extra/pseudo/test/cxx/parameter-decl-clause.cpp
  clang-tools-extra/pseudo/test/cxx/predefined-identifier.cpp
  clang-tools-extra/pseudo/test/cxx/template-empty-type-parameter.cpp
  clang-tools-extra/pseudo/test/cxx/unsized-array.cpp
  clang-tools-extra/pseudo/test/glr.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/Mangle.cpp
  clang/lib/Basic/Targets/OSTargets.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/lib/CodeGen/SanitizerMetadata.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Headers/unwind.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Core/ConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
  clang/test/CodeGen/builtins-memcpy-inline.c
  clang/test/CodeGenCXX/externc-used-not-replaced.cpp
  clang/test/Driver/cl-zc.cpp
  clang/test/Driver/sanitizer-ld.c
  clang/test/Driver/windows-exceptions.cpp
  clang/test/Modules/Inputs/gmodules-deduction-guide.h
  clang/test/Modules/gmodules-deduction-guide.cpp
  clang/test/Sema/aarch64-sve-vector-arith-ops.c
  clang/test/Sema/aarch64-sve-vector-bitwise-ops.c
  clang/test/Sema/aarch64-sve-vector-compare-ops.c
  clang/test/Sema/builtins-memcpy-inline.cpp
  clang/test/SemaCXX/aarch64-sve-vector-conditional-op.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp
  clang/tools/clang-offload-packager/ClangOffloadPackager.cpp
  clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/www/c_status.html
  compiler-rt/lib/asan/asan_allocator.h
  compiler-rt/lib/asan/asan_errors.cpp

[PATCH] D126694: [C++20][Modules] Initial implementation of GMF decl elision.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126694#3565417 , @ChuanqiXu wrote:

> In D126694#3564629 , @iains wrote:
>
>> the first failure is like this:
>>
>>   x-h.h:
>>   struct A {
>> friend int operator+(const A &lhs, const A &rhs) {
>>   return 0;
>> }
>>   };
>>   
>>   X.cpp:
>>   module;
>>   #include "x-h.h"
>>   export module X;
>>   
>>   export using ::A;
>>
>> This does not mark *anything* in the GMF (x-h.h) as 'used', and so 
>> everything there is unreachable (and hence the fails).
>> I.e `export using ::A;` neither marks A as used or referenced.
>> I am not sure if we are supposed to special-case this - since 
>> `https://eel.is/c++draft/module#global.frag-3.6` explicitly says "In this 
>> determination, it is unspecified .. whether `using-declaration, ...  is 
>> replaced by the declarations they name prior to this determination,`
>> so .. not about how to proceed with this one at present; 
>> edit:  but it seems most reasonable to make it behave as if A was itself  
>> exported.
>
> I highly recommend we should mark A here. Maybe we need other interfaces than 
> markDeclUsed and setDeclReferenced. If we don't support this, we couldn't use 
> modules like 
> https://github.com/alibaba/async_simple/blob/CXX20Modules/third_party_module/asio/asio.cppm.
>  This manner is important to use C++20 modules before the build system is 
> ready. Also, I think it is an important tool to implement C++23's std 
> modules. So we should support it.

Actually, after thinking some more, what seems to be wrong here is that we 
should be making the exported item "VisibleIfImported" .. which is not being 
done - I guess this was a bug already and it has been exposed by the recent 
changes in the module ownership processing.  I will next take a look at that 
(and the other comments).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694

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

[PATCH] D127229: [clang][deps] Set -disable-free for module compilations

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:338
+// always true for a driver invocation.
+bool DisableFree = true;
 DependencyScanningAction Action(

I see the driver is adding `-disable-free` conditionally:

```
  if (!C.isForDiagnostics())
CmdArgs.push_back("-disable-free");
```

Does that change anything for this patch?



Comment at: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:338
+// always true for a driver invocation.
+bool DisableFree = true;
 DependencyScanningAction Action(

jansvoboda11 wrote:
> I see the driver is adding `-disable-free` conditionally:
> 
> ```
>   if (!C.isForDiagnostics())
> CmdArgs.push_back("-disable-free");
> ```
> 
> Does that change anything for this patch?
If this is always `true` for our purposes, is there a reason for passing this 
argument into `DependencyScanningAction` instead of just hard-coding it there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127229

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

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:181
 std::vector PrebuiltModuleDeps;
-std::map ClangModuleDeps;
+llvm::MapVector,
+llvm::StringMap>

What's the reason for wrapping `ModuleDeps` in `unique_ptr`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127243

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

[PATCH] D127150: [doc] Add release notes about SEH unwind information on ARM

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG20ca739701d7: [doc] Add release notes about SEH unwind 
information on ARM (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127150

Files:
  clang/docs/ReleaseNotes.rst
  llvm/docs/ReleaseNotes.rst


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -102,6 +102,8 @@
   versions.
 * Added a pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72
   Erratum 1655431. This is enabled by default when targeting either CPU.
+* Implemented generation of Windows SEH unwind information.
+* Switched the MinGW target to use SEH instead of DWARF for unwind information.
 
 Changes to the AVR Backend
 --
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -353,6 +353,11 @@
   JustMyCode feature. Note, you may need to manually add ``/JMC`` as additional
   compile options in the Visual Studio since it currently assumes clang-cl 
does not support ``/JMC``.
 
+- Implemented generation of SEH unwind information on ARM. (C++ exception
+  handling in MSVC mode is still unimplemented though.)
+
+- Switched MinGW mode on ARM to use SEH instead of DWARF for unwind 
information.
+
 AIX Support
 ---
 


Index: llvm/docs/ReleaseNotes.rst
===
--- llvm/docs/ReleaseNotes.rst
+++ llvm/docs/ReleaseNotes.rst
@@ -102,6 +102,8 @@
   versions.
 * Added a pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72
   Erratum 1655431. This is enabled by default when targeting either CPU.
+* Implemented generation of Windows SEH unwind information.
+* Switched the MinGW target to use SEH instead of DWARF for unwind information.
 
 Changes to the AVR Backend
 --
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -353,6 +353,11 @@
   JustMyCode feature. Note, you may need to manually add ``/JMC`` as additional
   compile options in the Visual Studio since it currently assumes clang-cl does not support ``/JMC``.
 
+- Implemented generation of SEH unwind information on ARM. (C++ exception
+  handling in MSVC mode is still unimplemented though.)
+
+- Switched MinGW mode on ARM to use SEH instead of DWARF for unwind information.
+
 AIX Support
 ---
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 20ca739 - [doc] Add release notes about SEH unwind information on ARM

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2022-06-08T11:32:17+03:00
New Revision: 20ca739701d7b2e2aa4028f1a7853f6d0fa0c412

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

LOG: [doc] Add release notes about SEH unwind information on ARM

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

Added: 


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

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8c4f7f48850f..1e95d3cef51c 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -353,6 +353,11 @@ Windows Support
   JustMyCode feature. Note, you may need to manually add ``/JMC`` as additional
   compile options in the Visual Studio since it currently assumes clang-cl 
does not support ``/JMC``.
 
+- Implemented generation of SEH unwind information on ARM. (C++ exception
+  handling in MSVC mode is still unimplemented though.)
+
+- Switched MinGW mode on ARM to use SEH instead of DWARF for unwind 
information.
+
 AIX Support
 ---
 

diff  --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
index e588128b2b92..d2813bb86973 100644
--- a/llvm/docs/ReleaseNotes.rst
+++ b/llvm/docs/ReleaseNotes.rst
@@ -102,6 +102,8 @@ Changes to the ARM Backend
   versions.
 * Added a pass to workaround Cortex-A57 Erratum 1742098 and Cortex-A72
   Erratum 1655431. This is enabled by default when targeting either CPU.
+* Implemented generation of Windows SEH unwind information.
+* Switched the MinGW target to use SEH instead of DWARF for unwind information.
 
 Changes to the AVR Backend
 --



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

[PATCH] D126560: [analyzer] Track assume call stack to detect fixpoint

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

steakhal added a comment.

New assertion failure:

  llvm-project/clang/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:72: 
bool isLeftShiftResultUnrepresentable(const clang::BinaryOperator *, 
clang::ento::CheckerContext &): Assertion `LHS && RHS && "Values unknown, 
inconsistent state"' failed.

Reproduction:

  cat > extent.c 

[PATCH] D126215: [analyzer] Deprecate `-analyzer-store region` flag

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D126215#3565572 , @steakhal wrote:

> I believe, the silence from the code owner means that he agrees with this 
> change.
> I'll land it tomorrow.

I believe there is a misconception about the role of a code owner, we cannot 
expect Artem to review all patches. According to the LLVM Developer Poicy

> The sole responsibility of a code owner is to ensure that a commit to their 
> area of the code is appropriately reviewed, either by themself or by 
> **someone else**.

Many other analyzer pathces in the past had been reviewed by me and you 
@steakhal and we didn't receive any complaints after they landed. So, I think 
it is okay to land this one as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126215

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

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:616
+setOperationAction(ISD::FROUNDEVEN, MVT::f16, Promote);
+setOperationAction(ISD::FP_ROUND, MVT::f16, Expand);
+setOperationAction(ISD::FP_EXTEND, MVT::f32, Expand);

Just confused how to expand it. Will the expand fail and finally turns to 
libcall?



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:763
+if (isTypeLegal(MVT::f16)) {
+  setOperationAction(ISD::FP_EXTEND, MVT::f80, Custom);
+  setOperationAction(ISD::STRICT_FP_EXTEND, MVT::f80, Custom);

Why f16 emulation affect f80 type? Are we checking isTypeLegal(MVT::f80)?



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22100
   SDValue Res;
+  if (SrcVT == MVT::f16 && !Subtarget.hasFP16()) {
+if (IsStrict)

Not sure if it is better to wrapper it into a readable function (e.g., 
isSoftF16).



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22448
+  if (SrcVT == MVT::f16)
+return SDValue();
+

Why we don't extent to f32 here?



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22522
+  if (!isScalarFPTypeInSSEReg(SrcVT) ||
+  (SrcVT == MVT::f16 && !Subtarget.hasFP16()))
 return SDValue();

Why we don't extent to f32 here? Will it be promoted finally?



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22765
+DAG.getIntPtrConstant(0, DL));
+  Res = DAG.getNode(X86ISD::STRICT_CVTPS2PH, DL, {MVT::v8i16, MVT::Other},
+{Chain, Res, DAG.getTargetConstant(4, DL, MVT::i32)});

Should MVT::v8i16 be MVT::v8f16?



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22766
+  Res = DAG.getNode(X86ISD::STRICT_CVTPS2PH, DL, {MVT::v8i16, MVT::Other},
+{Chain, Res, DAG.getTargetConstant(4, DL, MVT::i32)});
+  Chain = Res.getValue(1);

Is it rounding control? Can we use a macro or add comments for what is the 
rounding control?



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22775
+
+Res = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, MVT::i16, Res,
+  DAG.getIntPtrConstant(0, DL));

MVT::f16 and delete the bitcast?



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:44211
   VT != MVT::f80 && VT != MVT::f128 &&
+  !(VT.getScalarType() == MVT::f16 && !Subtarget.hasFP16()) &&
   (TLI.isTypeLegal(VT) || VT == MVT::v2f32) &&

Not sure if it is better to wrapper it into a readable function (e.g., 
isSoftF16).



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:1476
 }
-let Predicates = [HasFP16] in {
+let Predicates = [HasBWI] in {
   def : Pat<(v32f16 (X86VBroadcastld16 addr:$src)),

If target don't have avx512bw feature. There is some other pattern to lower the 
node or fp16 broadcast node is invalid?



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:4107
  _.ExeDomain>, EVEX_4V, Sched<[SchedWriteFShuffle.XMM]>;
+  let Predicates = [prd] in {
   def rrkz : AVX512PI<0x10, MRMSrcReg, (outs _.RC:$dst),

Previous prd only apply to "def rr"? Is it a bug for previous code?



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:4352
+defm : avx512_move_scalar_lowering<"VMOVSHZ", X86Movsh, fp16imm0, v8f16x_info>;
+defm : avx512_store_scalar_lowering<"VMOVSHZ", avx512vl_f16_info,
+   (v32i1 (bitconvert (and GR32:$mask, (i32 1, GR32>;

Why previous code don't have predicates?



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:11657
 
+let Predicates = [HasBWI], AddedComplexity = -10 in {
+  def : Pat<(f16 (load addr:$src)), (COPY_TO_REGCLASS (VPINSRWZrm (v8i16 
(IMPLICIT_DEF)), addr:$src, 0), FR16X)>;

Why set AddedComplexity to -10? There no such addtional complexity in previous 
code. Add comments for it? 



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:3970
 
+let Predicates = [UseSSE2], AddedComplexity = -10 in {
+  def : Pat<(f16 (load addr:$src)), (COPY_TO_REGCLASS (PINSRWrm (v8i16 
(IMPLICIT_DEF)), addr:$src, 0), FR16)>;

Why  AddedComplexity = -10? Add comments for it?



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:3978
+let Predicates = [HasAVX, NoBWI], AddedComplexity = -10 in {
+  def : Pat<(f16 (load addr:$src)), (COPY_TO_REGCLASS (VPINSRWrm (v8i16 
(IMPLICIT_DEF)), addr:$src, 0), FR16)>;
+  def : Pat<(i16 (bitconvert f16:$src)), (EXTRACT_SUBREG (VPEXTRWrr (v8i16 
(COPY_TO_REGCLASS FR16:$src, VR128)), 0), sub_16bit)>;

Miss pattern for store?



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:5214
+
+let Predicates = [HasAVX, NoBWI] in
+  def : Pat<(store f16:$src, addr:$

[PATCH] D126560: [analyzer] Track assume call stack to detect fixpoint

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Thanks for the report. I am looking into it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126560

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

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

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

balazske updated this revision to Diff 435081.
balazske marked 5 inline comments as done.
balazske added a comment.

applied review comments, test improvement


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122150

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
  clang/lib/StaticAnalyzer/Checkers/ErrnoTesterChecker.cpp
  clang/test/Analysis/analyzer-config.c
  clang/test/Analysis/errno-notes.c
  clang/test/Analysis/errno-options.c
  clang/test/Analysis/errno.c

Index: clang/test/Analysis/errno.c
===
--- clang/test/Analysis/errno.c
+++ clang/test/Analysis/errno.c
@@ -3,6 +3,7 @@
 // RUN:   -analyzer-checker=apiModeling.Errno \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-checker=debug.ErrnoTest \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
 // RUN:   -DERRNO_VAR
 
 // RUN: %clang_analyze_cc1 -verify %s \
@@ -10,8 +11,10 @@
 // RUN:   -analyzer-checker=apiModeling.Errno \
 // RUN:   -analyzer-checker=debug.ExprInspection \
 // RUN:   -analyzer-checker=debug.ErrnoTest \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
 // RUN:   -DERRNO_FUNC
 
+#include "Inputs/system-header-simulator.h"
 #ifdef ERRNO_VAR
 #include "Inputs/errno_var.h"
 #endif
@@ -24,6 +27,7 @@
 int ErrnoTesterChecker_getErrno();
 int ErrnoTesterChecker_setErrnoIfError();
 int ErrnoTesterChecker_setErrnoIfErrorRange();
+int ErrnoTesterChecker_setErrnoCheckState();
 
 void something();
 
@@ -61,3 +65,173 @@
 clang_analyzer_eval(errno == 1); // expected-warning{{FALSE}} expected-warning{{TRUE}}
   }
 }
+
+void testErrnoCheck0() {
+  // If the function returns a success result code, value of 'errno'
+  // is unspecified and it is unsafe to make any decision with it.
+  // The function did not promise to not change 'errno' if no failure happens.
+  int X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 0) {
+if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+}
+if (errno) { // no warning for second time (analysis stops at the first warning)
+}
+  }
+  X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 0) {
+if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+}
+errno = 0;
+  }
+  X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 0) {
+errno = 0;
+if (errno) { // no warning after overwritten 'errno'
+}
+  }
+}
+
+void testErrnoCheck1() {
+  // If the function returns error result code that is out-of-band (not a valid
+  // non-error return value) the value of 'errno' can be checked but it is not
+  // required to do so.
+  int X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 1) {
+if (errno) { // no warning
+}
+  }
+  X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 1) {
+errno = 0; // no warning
+  }
+}
+
+void testErrnoCheck2() {
+  // If the function returns an in-band error result the value of 'errno' is
+  // required to be checked to verify if error happened.
+  // The same applies to other functions that can indicate failure only by
+  // change of 'errno'.
+  int X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 2) {
+if (errno) {
+}
+errno = 0; // no warning after 'errno' was read
+  }
+  X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 2) {
+errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [alpha.unix.Errno]}}
+errno = 0;
+  }
+  X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 2) {
+errno = 0; // expected-warning{{Value of 'errno' was not checked and is overwritten here [alpha.unix.Errno]}}
+if (errno) {
+}
+  }
+}
+
+void testErrnoCheckUndefinedLoad() {
+  int X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 0) {
+if (errno) { // expected-warning{{An undefined value may be read from 'errno' [alpha.unix.Errno]}}
+}
+  }
+}
+
+void testErrnoNotCheckedAtSystemCall() {
+  int X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 2) {
+printf("%i", 1); // expected-warning{{Value of 'errno' was not checked and may be overwritten by function 'printf' [alpha.unix.Errno]}}
+printf("%i", 1); // no warning ('printf' does not change errno state)
+  }
+}
+
+void testErrnoCheckStateInvalidate() {
+  int X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 0) {
+something();
+if (errno) { // no warning after an invalidating function call
+}
+  }
+  X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 0) {
+printf("%i", 1);
+if (errno) { // no warning after an invalidating standard fu

[PATCH] D125400: [clang][Analyzer] Add errno state to standard functions modeling.

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

balazske updated this revision to Diff 435082.
balazske added a comment.

small test change, removed a compile warning


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125400

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/errno-stdlibraryfunctions-notes.c
  clang/test/Analysis/errno-stdlibraryfunctions.c

Index: clang/test/Analysis/errno-stdlibraryfunctions.c
===
--- /dev/null
+++ clang/test/Analysis/errno-stdlibraryfunctions.c
@@ -0,0 +1,55 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=apiModeling.Errno \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true
+
+#include "Inputs/errno_var.h"
+
+typedef typeof(sizeof(int)) size_t;
+typedef __typeof(sizeof(int)) off_t;
+typedef size_t ssize_t;
+ssize_t send(int sockfd, const void *buf, size_t len, int flags);
+off_t lseek(int fildes, off_t offset, int whence);
+
+void clang_analyzer_warnIfReached();
+void clang_analyzer_eval(int);
+
+int unsafe_errno_read(int sock, void *data, int data_size) {
+  if (send(sock, data, data_size, 0) != data_size) {
+if (errno == 1) {
+  // expected-warning@-1{{An undefined value may be read from 'errno'}}
+  return 0;
+}
+  }
+  return 1;
+}
+
+int errno_lseek(int fildes, off_t offset) {
+  off_t result = lseek(fildes, offset, 0);
+  if (result == (off_t)-1) {
+// Failure path.
+// check if the function is modeled
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+return 2;
+  }
+  if (result != offset) {
+// Not success path (?)
+// not sure if this is a valid case, allow to check 'errno'
+if (errno == 1) { // no warning
+  return 1;
+}
+  }
+  if (result == offset) {
+// The checker does not differentiate for this case.
+// In general case no relation exists between the arg 2 and the returned
+// value, only for SEEK_SET.
+if (errno == 1) { // no warning
+  return 1;
+}
+clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+  }
+  return 0;
+}
Index: clang/test/Analysis/errno-stdlibraryfunctions-notes.c
===
--- /dev/null
+++ clang/test/Analysis/errno-stdlibraryfunctions-notes.c
@@ -0,0 +1,49 @@
+// RUN: %clang_analyze_cc1 -verify -analyzer-output text %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=apiModeling.Errno \
+// RUN:   -analyzer-checker=alpha.unix.Errno \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:ModelPOSIX=true
+
+#include "Inputs/errno_var.h"
+
+int access(const char *path, int amode);
+
+void clang_analyzer_warnIfReached();
+
+void test1() {
+  access("path", 0); // no note here
+  access("path", 0);
+  // expected-note@-1{{Assuming that function 'access' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+  if (errno != 0) {
+// expected-warning@-1{{An undefined value may be read from 'errno'}}
+// expected-note@-2{{An undefined value may be read from 'errno'}}
+  }
+}
+
+void test2() {
+  if (access("path", 0) == -1) {
+// expected-note@-1{{Taking true branch}}
+// Failure path.
+if (errno != 0) {
+  // expected-note@-1{{'errno' is not equal to 0}}
+  // expected-note@-2{{Taking true branch}}
+  clang_analyzer_warnIfReached(); // expected-note {{REACHABLE}} expected-warning {{REACHABLE}}
+} else {
+  clang_analyzer_warnIfReached(); // no-warning: We are on the failure path.
+}
+  }
+}
+
+void test3() {
+  if (access("path", 0) != -1) {
+// Success path.
+// expected-note@-2{{Assuming that function 'access' is successful, in this case the value 'errno' may be undefined after the call and should not be used}}
+// expected-note@-3{{Taking true branch}}
+if (errno != 0) {
+  // expected-warning@-1{{An undefined value may be read from 'errno'}}
+  // expected-note@-2{{An undefined value may be read from 'errno'}}
+}
+  }
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -40,6 +40,7 @@
 //
 //===--===//
 
+#include "ErrnoModeling.h"
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/

[PATCH] D127277: [clang][analyzer] Fix StdLibraryFunctionsChecker 'mkdir' return value.

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

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

The functions 'mkdir' and 'mknod' return 0 on success and -1 on failure.
The checker modeled these functions with a >= 0 return value on success
which is incorrect.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127277

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


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1901,44 +1901,40 @@
 ArgumentCondition(1, WithinRange, Range(0, SizeMax;
 
 // int mkdir(const char *pathname, mode_t mode);
-// FIXME: returns 0 on success, ReturnsValidFileDescriptor is incorrect
 addToFunctionSummaryMap(
 "mkdir", Signature(ArgTypes{ConstCharPtrTy, Mode_tTy}, RetType{IntTy}),
 Summary(NoEvalCall)
-.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
+.Case(ReturnsZero, ErrnoMustNotBeChecked)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
 .ArgConstraint(NotNull(ArgNo(0;
 
 // int mkdirat(int dirfd, const char *pathname, mode_t mode);
-// FIXME: returns 0 on success, ReturnsValidFileDescriptor is incorrect
 addToFunctionSummaryMap(
 "mkdirat",
 Signature(ArgTypes{IntTy, ConstCharPtrTy, Mode_tTy}, RetType{IntTy}),
 Summary(NoEvalCall)
-.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
+.Case(ReturnsZero, ErrnoMustNotBeChecked)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
 .ArgConstraint(NotNull(ArgNo(1;
 
 Optional Dev_tTy = lookupTy("dev_t");
 
 // int mknod(const char *pathname, mode_t mode, dev_t dev);
-// FIXME: returns 0 on success, ReturnsValidFileDescriptor is incorrect
 addToFunctionSummaryMap(
 "mknod",
 Signature(ArgTypes{ConstCharPtrTy, Mode_tTy, Dev_tTy}, RetType{IntTy}),
 Summary(NoEvalCall)
-.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
+.Case(ReturnsZero, ErrnoMustNotBeChecked)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
 .ArgConstraint(NotNull(ArgNo(0;
 
 // int mknodat(int dirfd, const char *pathname, mode_t mode, dev_t dev);
-// FIXME: returns 0 on success, ReturnsValidFileDescriptor is incorrect
 addToFunctionSummaryMap(
 "mknodat",
 Signature(ArgTypes{IntTy, ConstCharPtrTy, Mode_tTy, Dev_tTy},
   RetType{IntTy}),
 Summary(NoEvalCall)
-.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
+.Case(ReturnsZero, ErrnoMustNotBeChecked)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
 .ArgConstraint(NotNull(ArgNo(1;
 


Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -1901,44 +1901,40 @@
 ArgumentCondition(1, WithinRange, Range(0, SizeMax;
 
 // int mkdir(const char *pathname, mode_t mode);
-// FIXME: returns 0 on success, ReturnsValidFileDescriptor is incorrect
 addToFunctionSummaryMap(
 "mkdir", Signature(ArgTypes{ConstCharPtrTy, Mode_tTy}, RetType{IntTy}),
 Summary(NoEvalCall)
-.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
+.Case(ReturnsZero, ErrnoMustNotBeChecked)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
 .ArgConstraint(NotNull(ArgNo(0;
 
 // int mkdirat(int dirfd, const char *pathname, mode_t mode);
-// FIXME: returns 0 on success, ReturnsValidFileDescriptor is incorrect
 addToFunctionSummaryMap(
 "mkdirat",
 Signature(ArgTypes{IntTy, ConstCharPtrTy, Mode_tTy}, RetType{IntTy}),
 Summary(NoEvalCall)
-.Case(ReturnsValidFileDescriptor, ErrnoMustNotBeChecked)
+.Case(ReturnsZero, ErrnoMustNotBeChecked)
 .Case(ReturnsMinusOne, ErrnoNEZeroIrrelevant)
 .ArgConstraint(NotNull(ArgNo(1;
 
 Optional Dev_tTy = lookupTy("dev_t");
 
 // int mknod(const char *pathname, mode_t mode, dev_t dev);
-// FIXME: returns 0 on success, ReturnsValidFileDescriptor is incorrect
 addToFunctionSummaryMap(
 "mknod",
 Signature(ArgTypes{ConstCharPtrTy, Mode_tTy, Dev_t

[PATCH] D122150: [clang][analyzer] Add checker for bad use of 'errno'.

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

balazske added inline comments.



Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:558
+  InAlpha,
+  Hide>, // ??
+  ]>,

steakhal wrote:
> I think we have 'Hidden', but not 'Hide'.
> It means that it is a modeling checker. Any checker that emits diagnostics, 
> should **not** be marked 'Hidden'.
This "Hide" applies for the option, not for the checker. It was copied from 
another place. I am not sure if it must be used here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122150

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

[PATCH] D126365: [git-clang-format] Stop ignoring changes for files with space in path

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

@Eitot, do you need help landing this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126365

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

[PATCH] D126461: [RISCV] Extract and store new vl of vleff iff destination isn't null

[Wang Pengcheng via Phabricator via cfe-commits]

pcwang-thead added a comment.

In D126461#3565305 , @khchen wrote:

>> Store to null will be changed to unreachable, so all instructions after 
>> vleff intrinsic call will be deleted and it causes runtime errors. If 
>> destination to store is null, we won't extract and store the new vl.
>
>
>
>> Yes, but only for vleff instructions, since it has two outputs actually. And 
>> this behavior is compatible with GCC. If necessary, I will propose it to 
>> rvv-intrinsic-doc.
>
> Compiling with -O0, I didn't see this behavior, so are you trying to make the 
> optimized code behavior is compatible with GCC?
> In addition, it seems this behavior also exist in scalar code, do we also 
> need to make the scalar result is compatible with GCC?

Store to nullptr is UB and it seems that LLVM and GCC have different behavior. 
As shown in 
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/Local.cpp#L2317,
 LLVM (`simplifycfg` in particular) will turn stores to nullptr into 
unreachable. When compiling with -O0, no optimization is done, so we can't see 
this behavior.
What I am doing in this patch is not making store to nullptr a defined 
behavior, but changing the way to store the new vl in vleff intrinsics (only do 
it if destination to store is not null).
Before:

  new_vl = vleff(...)
  *dst = new_vl;

Now:

  new_vl = vleff(...)
  if (dst) {
*dst = new_vl;
  }

So that we won't generate store to nullptr if we pass a nullptr to vleff 
intrinsics. This behavior is compatible with GCC.

As for the difference when compiling with -O3, it is because GCC handles this 
UB (store to nullptr) in other way. But as you can see, `sw zero,0(zero)` will 
cause interruption (runtime errors). I don't know if we should make LLVM and 
GCC with the same behavior to  handle stores to null, which is beyond this 
patch.

> ex.
>
>   int foo (int j){
>   int *k = nullptr;
>   *k = 10;
>   return 100;
>   }
>
> compiling with `-O3`, llvm generates empty function but gcc generates
>
>   foo(int):
>   sw  zero,0(zero)
>   ebreak
>
> https://godbolt.org/z/46vGrzs49
>
> Please correct me if I misunderstand something, thanks.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126461

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

[PATCH] D127283: [pseudo] Simplify the glrReduce implementation.

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

glrReduce maintains two priority queues (one for bases, and the other
for Sequence), these queues are in parallel with each other, corresponding to a
single family. They can be folded into one.

This patch removes the bases priority queue, which improves the glrParse by
10%.

ASTReader.cpp: 2.03M/s (before) vs 2.26M/s (after)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127283

Files:
  clang-tools-extra/pseudo/lib/GLR.cpp


Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -259,18 +259,19 @@
 }
   };
 
-  // The base nodes are the heads after popping the GSS nodes we are reducing.
-  // We don't care which rule yielded each base. If Family.Symbol is S, the
-  // base includes an item X := ... • S ... and since the grammar is
-  // context-free, *all* parses of S are valid here.
-  // FIXME: reuse the queues across calls instead of reallocating.
-  KeyedQueue Bases;
-
   // A sequence is the ForestNode payloads of the GSS nodes we are reducing.
   // These are the RHS of the rule, the RuleID is stored in the Family.
   // They specify a sequence ForestNode we may build (but we dedup first).
   using Sequence = llvm::SmallVector;
-  KeyedQueue Sequences;
+  struct SeqNodeSpec {
+// The base nodes are the heads after popping the GSS nodes we are 
reducing.
+// We don't care which rule yielded each base. If Family.Symbol is S, the
+// base includes an item X := ... • S ... and since the grammar is
+// context-free, *all* parses of S are valid here.
+const GSS::Node* Base = nullptr;
+Sequence Seq;
+  };
+  KeyedQueue Sequences;
 
   Sequence TempSequence;
   // Pop walks up the parent chain(s) for a reduction from Head by to Rule.
@@ -283,9 +284,8 @@
 auto DFS = [&](const GSS::Node *N, unsigned I, auto &DFS) {
   if (I == Rule.Size) {
 F.Start = TempSequence.front()->startTokenIndex();
-Bases.emplace(F, N);
 LLVM_DEBUG(llvm::dbgs() << "--> base at S" << N->State << "\n");
-Sequences.emplace(F, TempSequence);
+Sequences.emplace(F, SeqNodeSpec{N, TempSequence});
 return;
   }
   TempSequence[Rule.Size - 1 - I] = N->Payload;
@@ -311,20 +311,23 @@
   //  - process one family at a time, forming a forest node
   //  - produces new GSS heads which may enable more pops
   PopPending();
-  while (!Bases.empty()) {
-// We should always have bases and sequences for the same families.
-Family F = Bases.top().first;
-assert(!Sequences.empty());
-assert(Sequences.top().first == F);
+  while (!Sequences.empty()) {
+Family F = Sequences.top().first;
 
 LLVM_DEBUG(llvm::dbgs() << "  Push " << Params.G.symbolName(F.Symbol)
 << " from token " << F.Start << "\n");
 
-// Grab the sequences for this family.
+// Grab the sequences and bases for this family.
 FamilySequences.clear();
+FamilyBases.clear();
 do {
   FamilySequences.emplace_back(Sequences.top().first.Rule,
-   Sequences.top().second);
+   Sequences.top().second.Seq);
+  FamilyBases.emplace_back(
+  Params.Table.getGoToState(Sequences.top().second.Base->State,
+F.Symbol),
+  Sequences.top().second.Base);
+
   Sequences.pop();
 } while (!Sequences.empty() && Sequences.top().first == F);
 // Build a forest node for each unique sequence.
@@ -341,15 +344,7 @@
 : &Params.Forest.createAmbiguous(F.Symbol, SequenceNodes);
 LLVM_DEBUG(llvm::dbgs() << "--> " << Parsed->dump(Params.G) << "\n");
 
-// Grab the bases for this family.
-// As well as deduplicating them, we'll group by the goto state.
-FamilyBases.clear();
-do {
-  FamilyBases.emplace_back(
-  Params.Table.getGoToState(Bases.top().second->State, F.Symbol),
-  Bases.top().second);
-  Bases.pop();
-} while (!Bases.empty() && Bases.top().first == F);
+// Bases for this family, deduplicate them, and group by the goTo State.
 sortAndUnique(FamilyBases);
 // Create a GSS node for each unique goto state.
 llvm::ArrayRef BasesLeft = FamilyBases;


Index: clang-tools-extra/pseudo/lib/GLR.cpp
===
--- clang-tools-extra/pseudo/lib/GLR.cpp
+++ clang-tools-extra/pseudo/lib/GLR.cpp
@@ -259,18 +259,19 @@
 }
   };
 
-  // The base nodes are the heads after popping the GSS nodes we are reducing.
-  // We don't care which rule yielded each base. If Family.Symbol is S, the
-  // base includes an item X := ... •

[PATCH] D127284: [WIP] [clang-repl] Support statements on global scope in incremental mode.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev created this revision.
v.g.vassilev added reviewers: rsmith, rjmccall, lhames, sgraenitz.
Herald added a subscriber: StephenFan.
Herald added a project: All.
v.g.vassilev requested review of this revision.

This patch teaches clang to parse statements on the global scope to allow:

  ./bin/clang-repl
  clang-repl> int i = 12;
  clang-repl> ++i;
  clang-repl> extern "C" int printf(const char*,...);
   clang-repl> printf("%d\n", i);
   13
   clang-repl> quit
   

  

The patch conceptually models the possible "top-level" statements between two 
top-level declarations into a block which is emitted as part of the global init 
function.

The current implementation in CodeGen is a placeholder allowing us to discuss 
what would be the best approach to tackle this usecase. CodeGen works with 
"GlobalDecl"s and it is awkward to model a block of statements in that way. 
Currently, we attach each statement as an initializer of a global VarDecl. 
However, that granularity is not desirable.


Repository:
  rC Clang

https://reviews.llvm.org/D127284

Files:
  clang/include/clang/AST/ASTConsumer.h
  clang/include/clang/Parse/Parser.h
  clang/lib/CodeGen/CodeGenAction.cpp
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/Interpreter/execute-stmts.cpp

Index: clang/test/Interpreter/execute-stmts.cpp
===
--- /dev/null
+++ clang/test/Interpreter/execute-stmts.cpp
@@ -0,0 +1,18 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl | FileCheck %s
+
+int i = 12;
+++i;
+extern "C" int printf(const char*,...);
+printf("i = %d\n", i);
+// CHECK: i = 13
+
+// FIXME: isConstructorDeclarator thinks these are ctors:
+// namespace Ns { void f(){} }
+// Ns::f();
+
+// void g() {}
+// g();
+
+quit
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -582,13 +582,14 @@
 ///
 /// Note that in C, it is an error if there is no first declaration.
 bool Parser::ParseFirstTopLevelDecl(DeclGroupPtrTy &Result,
-Sema::ModuleImportState &ImportState) {
+Sema::ModuleImportState &ImportState,
+StmtVector *Stmts) {
   Actions.ActOnStartOfTranslationUnit();
 
   // For C++20 modules, a module decl must be the first in the TU.  We also
   // need to track module imports.
   ImportState = Sema::ModuleImportState::FirstDecl;
-  bool NoTopLevelDecls = ParseTopLevelDecl(Result, ImportState);
+  bool NoTopLevelDecls = ParseTopLevelDecl(Result, ImportState, Stmts);
 
   // C11 6.9p1 says translation units must have at least one top-level
   // declaration. C++ doesn't have this restriction. We also don't want to
@@ -609,7 +610,8 @@
 ///   declaration
 /// [C++20]   module-import-declaration
 bool Parser::ParseTopLevelDecl(DeclGroupPtrTy &Result,
-   Sema::ModuleImportState &ImportState) {
+   Sema::ModuleImportState &ImportState,
+   StmtVector *Stmts /*=nullptr*/) {
   DestroyTemplateIdAnnotationsRAIIObj CleanupRAII(*this);
 
   // Skip over the EOF token, flagging end of previous input for incremental
@@ -724,6 +726,23 @@
   ParsedAttributes attrs(AttrFactory);
   MaybeParseCXX11Attributes(attrs);
 
+  // FIXME: Remove the incremental processing pre-condition and verify clang
+  // still can pass its test suite, which will harden `isDeclarationStatement`.
+  // It is known to have several weaknesses, for example in
+  // isConstructorDeclarator, infinite loop in c-index-test, etc..
+  // Parse a block of top-level-stmts.
+  while (PP.isIncrementalProcessingEnabled() && Stmts &&
+ !isDeclarationStatement()) {
+//isStmtExpr ? ParsedStmtContext::InStmtExpr : ParsedStmtContext()
+ParsedStmtContext SubStmtCtx = ParsedStmtContext();
+auto R = ParseStatementOrDeclaration(*Stmts, SubStmtCtx);
+if (!R.isUsable())
+  return true;
+Stmts->push_back(R.get());
+if (Tok.is(tok::eof))
+  return false;
+  }
+
   Result = ParseExternalDeclaration(attrs);
   // An empty Result might mean a line with ';' or some parsing error, ignore
   // it.
Index: clang/lib/Parse/ParseTentative.cpp
===
--- clang/lib/Parse/ParseTentative.cpp
+++ clang/lib/Parse/ParseTentative.cpp
@@ -22,6 +22,7 @@
 ///
 /// declaration-statement:
 ///   block-declaration
+///   template-declaration
 ///
 /// block-declaration:
 ///   simple-declaration
@@ -46,12 +47,24 @@
 ///   'using' 'namespace' '::'[opt] nested-name-specifier[opt]
 /// namespace-name ';'
 ///
+///

[PATCH] D127285: [analyzer] Fix assertion failure after getKnwonValue call

[Gabor Marton via Phabricator via cfe-commits]

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

Depends on D126560 . `getKnwonValue` has been 
changed by the parent patch
in a way that simplification was removed. This is not correct when the
function is called by the Checkers. Thus, a new internal function is
introduced `getConstValue` which simply queries the constraint manager.
This `getConstValue` is used internally in the SimpleSValBuilder when a
binop is evaluated, this way we avoid the recursion into the `Simplifier`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127285

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/svalbuilder-simplify-no-crash.c

Index: clang/test/Analysis/svalbuilder-simplify-no-crash.c
===
--- /dev/null
+++ clang/test/Analysis/svalbuilder-simplify-no-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -verify
+
+// Here, we test that svalbuilder simplification does not produce any
+// assertion failure.
+
+void crashing(long a, _Bool b) {
+  (void)(a & 1 && 0);
+  b = a & 1;
+  (void)(b << 1); // expected-warning{{core.UndefinedBinaryOperatorResult}}
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -22,6 +22,11 @@
 namespace {
 class SimpleSValBuilder : public SValBuilder {
 
+  // Query the constraint manager whether the SVal has only one possible
+  // (integer) value. If that is the case, the value is returned. Otherwise,
+  // returns NULL.
+  const llvm::APSInt *getConstValue(ProgramStateRef state, SVal V);
+
   // With one `simplifySValOnce` call, a compound symbols might collapse to
   // simpler symbol tree that is still possible to further simplify. Thus, we
   // do the simplification on a new symbol tree until we reach the simplest
@@ -45,7 +50,7 @@
   SVal simplifyUntilFixpoint(ProgramStateRef State, SVal Val);
 
   // Recursively descends into symbolic expressions and replaces symbols
-  // with their known values (in the sense of the getKnownValue() method).
+  // with their known values (in the sense of the getConstValue() method).
   // We traverse the symbol tree and query the constraint values for the
   // sub-trees and if a value is a constant we do the constant folding.
   SVal simplifySValOnce(ProgramStateRef State, SVal V);
@@ -65,8 +70,9 @@
   SVal evalBinOpLN(ProgramStateRef state, BinaryOperator::Opcode op,
Loc lhs, NonLoc rhs, QualType resultTy) override;
 
-  /// getKnownValue - evaluates a given SVal. If the SVal has only one possible
-  ///  (integer) value, that value is returned. Otherwise, returns NULL.
+  /// Evaluates a given SVal by recursively evaluating and
+  /// simplifying the children SVals. If the SVal has only one possible
+  /// (integer) value, that value is returned. Otherwise, returns NULL.
   const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal V) override;
 
   SVal simplifySVal(ProgramStateRef State, SVal V) override;
@@ -532,7 +538,7 @@
   llvm::APSInt LHSValue = lhs.castAs().getValue();
 
   // If we're dealing with two known constants, just perform the operation.
-  if (const llvm::APSInt *KnownRHSValue = getKnownValue(state, rhs)) {
+  if (const llvm::APSInt *KnownRHSValue = getConstValue(state, rhs)) {
 llvm::APSInt RHSValue = *KnownRHSValue;
 if (BinaryOperator::isComparisonOp(op)) {
   // We're looking for a type big enough to compare the two values.
@@ -652,7 +658,7 @@
 }
 
 // For now, only handle expressions whose RHS is a constant.
-if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs)) {
+if (const llvm::APSInt *RHSValue = getConstValue(state, rhs)) {
   // If both the LHS and the current expression are additive,
   // fold their constants and try again.
   if (BinaryOperator::isAdditiveOp(op)) {
@@ -699,7 +705,7 @@
   }
 
   // Is the RHS a constant?
-  if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs))
+  if (const llvm::APSInt *RHSValue = getConstValue(state, rhs))
 return MakeSymIntVal(Sym, op, *RHSValue, resultTy);
 
   if (Optional V = tryRearrange(state, op, lhs, rhs, resultTy))
@@ -1187,8 +1193,8 @@
   return UnknownVal();
 }
 
-const llvm::APSInt *SimpleSValBuilder::getKnownValue

[PATCH] D127285: [analyzer] Fix assertion failure after getKnownValue call

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

steakhal added a comment.

LGTM
Schedule a measurement to see what changes. Let's hope it won't introduce more 
crashes.
If everything checks out, it gets approved.




Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:25-28
+  // Query the constraint manager whether the SVal has only one possible
+  // (integer) value. If that is the case, the value is returned. Otherwise,
+  // returns NULL.
+  const llvm::APSInt *getConstValue(ProgramStateRef state, SVal V);

I'm also wondering if this should be at the top. I would rather put it below 
`simplifySValOnce()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127285

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

[PATCH] D127285: [analyzer] Fix assertion failure after getKnownValue call

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

steakhal added a comment.

Check the summary for typos. Try to mark named entities like `this`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127285

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

[PATCH] D127207: [flang][driver] Fix support for `-x`

[Diana Picus via Phabricator via cfe-commits]

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

LGTM, thanks.




Comment at: flang/lib/Frontend/CompilerInvocation.cpp:268
+ // pre-processed inputs.
+.Case("f95", Language::Fortran)
+.Case("f95-cpp-input", Language::Fortran)

ekieri wrote:
> Is there a reason to change from "f90" to "f95"? In my understanding, "f90" 
> is more idiomatic for free-form fortran of any standard >= 90.
At least for `gfortran`, `f90` doesn't seem to be supported, only `f77`, 
`f77-cpp-input`, `f95`,  `f95-cpp-input` are.
https://raw.githubusercontent.com/gcc-mirror/gcc/master/gcc/doc/invoke.texi#:~:text=f77%20%20f77%2Dcpp%2Dinput%20f95%20%20f95%2Dcpp%2Dinput

Note that these are not file extensions, but values for the `-x` option.



Comment at: flang/test/Driver/parse-ir-error.f95:16
+!
+! CHECK: error: expected integer
+! CHECK: error: Could not parse IR

Nit: I think checking just the "Could not parse IR" message should be enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127207

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

[clang] b3c0918 - [AST] Make header self-contained

[Benjamin Kramer via cfe-commits]

Author: Benjamin Kramer
Date: 2022-06-08T12:36:36+02:00
New Revision: b3c0918fb4809ebb681daaf1910e171b204b93ea

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

LOG: [AST] Make header self-contained

There's a dependency in AbstractTypeReader.inc that becomes an error
after D127231.

Added: 


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

Removed: 




diff  --git a/clang/include/clang/AST/AbstractTypeReader.h 
b/clang/include/clang/AST/AbstractTypeReader.h
index c9162b1779bc6..e44bbf61c0ed0 100644
--- a/clang/include/clang/AST/AbstractTypeReader.h
+++ b/clang/include/clang/AST/AbstractTypeReader.h
@@ -9,8 +9,9 @@
 #ifndef LLVM_CLANG_AST_ABSTRACTTYPEREADER_H
 #define LLVM_CLANG_AST_ABSTRACTTYPEREADER_H
 
-#include "clang/AST/Type.h"
 #include "clang/AST/AbstractBasicReader.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/AST/Type.h"
 
 namespace clang {
 namespace serialization {



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

[PATCH] D127287: clang: Introduce -fexperimental-max-bitint-width

[Matthias Gehre via Phabricator via cfe-commits]

mgehre-amd created this revision.
mgehre-amd added a reviewer: aaron.ballman.
Herald added a project: All.
mgehre-amd requested review of this revision.
Herald added a project: clang.

This splits of the introduction of -fexperimental-max-bitint-width 
from https://reviews.llvm.org/D122234
because that PR is still blocked on discussions on the backend side.

I was asked [0] to upstream at least the flag.

[0] 
https://github.com/llvm/llvm-project/commit/09854f2af3b914b616f29cb640bede3a27cf7c4e#commitcomment-75116619


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127287

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Sema/large-bit-int.c

Index: clang/test/Sema/large-bit-int.c
===
--- /dev/null
+++ clang/test/Sema/large-bit-int.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fexperimental-max-bitint-width=1024 -fsyntax-only -verify %s -Wno-unused -triple x86_64-gnu-linux
+
+void f() {
+  _Static_assert(__BITINT_MAXWIDTH__ == 1024, "Macro value is unexpected.");
+
+  _BitInt(1024) a;
+  unsigned _BitInt(1024) b;
+
+  _BitInt(8388609) c;// expected-error {{signed _BitInt of bit sizes greater than 1024 not supported}}
+  unsigned _BitInt(0xFF) d; // expected-error {{unsigned _BitInt of bit sizes greater than 1024 not supported}}
+}
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -314,7 +314,7 @@
 
 #define BENIGN_LANGOPT(Name, Bits, Default, Description)
 #define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
-#define BENIGN_VALUE_LANGOPT(Name, Type, Bits, Default, Description)
+#define BENIGN_VALUE_LANGOPT(Name, Bits, Default, Description)
 #include "clang/Basic/LangOptions.def"
 
   if (ExistingLangOpts.ModuleFeatures != LangOpts.ModuleFeatures) {
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -150,6 +150,9 @@
   PlatformMinVersion = VersionTuple();
 
   MaxOpenCLWorkGroupSize = 1024;
+
+  MaxBitIntWidth = 0;
+
   ProgramAddrSpace = 0;
 }
 
@@ -478,6 +481,9 @@
 Diags.Report(diag::err_opt_not_valid_on_target) << "-fprotect-parens";
 Opts.ProtectParens = false;
   }
+
+  if (Opts.MaxBitIntWidth)
+MaxBitIntWidth = Opts.MaxBitIntWidth;
 }
 
 bool TargetInfo::initFeatureMap(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6082,6 +6082,12 @@
 def fobjc_gc : Flag<["-"], "fobjc-gc">, Group,
   HelpText<"Enable Objective-C garbage collection">;
 
+def fexperimental_max_bitint_width_EQ:
+  Joined<["-"], "fexperimental-max-bitint-width=">, Group,
+  MetaVarName<"">,
+  HelpText<"Set the maximum bitwidth for _BitInt (experimental)">,
+  MarshallingInfoInt, "0">;
+
 } // let Flags = [CC1Option, NoDriverOption]
 
 //===--===//
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -31,6 +31,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Frontend/OpenMP/OMPGridValues.h"
+#include "llvm/IR/DerivedTypes.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/VersionTuple.h"
@@ -235,6 +236,8 @@
 
   unsigned MaxOpenCLWorkGroupSize;
 
+  unsigned MaxBitIntWidth;
+
   Optional DarwinTargetVariantTriple;
 
   // TargetInfo Constructor.  Default initializes all fields.
@@ -595,11 +598,16 @@
   // Different targets may support a different maximum width for the _BitInt
   // type, depending on what operations are supported.
   virtual size_t getMaxBitIntWidth() const {
+// Consider -fexperimental-max-bitint-width= first.
+if (MaxBitIntWidth)
+  return std::min(MaxBitIntWidth, llvm::IntegerType::MAX_INT_BITS);
+
 // FIXME: this value should be llvm::IntegerType::MAX_INT_BITS, which is
 // maximum bit width that LLVM claims its IR can support. However, most
-// backends currently have a bug where they only support division
-// operations on types that are <= 128 bits and crash otherwise. We're
-// setting the max supported value to 128 to be conservative.
+// backends currently have a bug where they only support float to int
+// conversion (and vice versa) on types that are <= 128 bits and crash
+// otherwise. We're setting the max supported value to 128 to be
+// conservative.
 

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt

[Matthias Gehre via Phabricator via cfe-commits]

mgehre-amd marked an inline comment as done.
mgehre-amd added a comment.

I split the introduction of -fexperimental-max-bitint-width into 
https://reviews.llvm.org/D127287 because there is still discussion about the 
bitint libraries in the backend PRs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122234

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

[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 commandeered this revision.
jansvoboda11 added a reviewer: jkorous.
jansvoboda11 added a comment.

Jan has asked me to take over.


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

https://reviews.llvm.org/D125349

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

[PATCH] D127283: [pseudo] Simplify the glrReduce implementation.

[Sam McCall via Phabricator via cfe-commits]

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

Very nice! Not sure why I didn't see this before :-)




Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:266
   using Sequence = llvm::SmallVector;
-  KeyedQueue Sequences;
+  struct SeqNodeSpec {
+// The base nodes are the heads after popping the GSS nodes we are 
reducing.

This name is a bit vague/abstract.

Maybe RootedSequence, or PushSpec?



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:267
+  struct SeqNodeSpec {
+// The base nodes are the heads after popping the GSS nodes we are 
reducing.
+// We don't care which rule yielded each base. If Family.Symbol is S, the

nit: A base node is the head...
(no reason to talk about them collectively in this context)



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:268
+// The base nodes are the heads after popping the GSS nodes we are 
reducing.
+// We don't care which rule yielded each base. If Family.Symbol is S, the
+// base includes an item X := ... • S ... and since the grammar is

the second part of this comment belongs below where we decorrelate the two 
(under "grab the sequences and bases for each family"), not here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127283

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

[PATCH] D125095: [Clang][AIX] Add .ref in frontend for AIX XCOFF to support `-bcdtors:csect` linker option

[Ting Wang via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:688
+updateAssociatedFunc(VFInitTermAssoc, LocalCXXGlobalInits, GetElem, 
Fn);
+updateAssociatedFunc(FFDtorTermAssoc, LocalCXXGlobalInits, GetElem, 
Fn);
+  }

shchenz wrote:
> `FFDtorTermAssoc` should store the mapping between dtor and term functions? 
> So why we need to update this container when we generate wrapper function for 
> init function? I think in the init function there should only be ctors 
> related functions?
> 
> And why we don't need to update for `VarsWithInitTerm`, in that container 
> there should be some static variables reply on the wrapper init function?
> `FFDtorTermAssoc` should store the mapping between dtor and term functions? 
> So why we need to update this container when we generate wrapper function for 
> init function? I think in the init function there should only be ctors 
> related functions?

Thank you for pointing out! This is redundant.

> And why we don't need to update for `VarsWithInitTerm`, in that container 
> there should be some static variables reply on the wrapper init function?

VarsWithInitTerm keeps track of mapping between variables in clang (Decl*) and 
the corresponding data structure in llvm (Constant *). To me it's stable, and 
not like functions which could be wrapped in new functions.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4799
+if (getTriple().isOSAIX())
+  addVarWithInitTerm(D, GV);
+  }

shchenz wrote:
> Why do we need to add mapping between a variable and its address? We already 
> map the global and its init function in above `EmitCXXGlobalVarDeclInitFunc`?
It seems to me that clang most of the time operates on Decl*. However to 
generate metadata, we refer to llvm::Constant*. I did not find how to get 
llvm::Constant* from Decl* in clang, so I'm tracking that information. I will 
check again to see if there is any official way to do that but I'm not aware of.



Comment at: clang/lib/CodeGen/CodeGenModule.h:465
+  /// between dtor and term functions.
+  llvm::SmallVector, 8>
+  VFInitTermAssoc;

shchenz wrote:
> Is there any reason why we need `vector` here instead of `map`? Can you give 
> an example that shows one global variable will be connected with more than 
> one init functions?
One variable can have two functions associated: one init and one term, thus 
used vector for VFInitTermAssoc. Also it is better to use variable as key for 
the benefit of the inner for loop inside AddMeta.

Dtor-to-Term (FFDtorTermAssoc) could use map, however it shares similar update 
logic as VFInitTermAssoc (for example the code snippet AddMeta in 
CodeGenModule::genAssocMeta() is used on both data structure), so I prefer to 
use vector for both of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125095

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

[clang] bf21cda - Add the 2022 papers to the C status tracking page

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-06-08T07:42:39-04:00
New Revision: bf21cda7f260d2643ff7fa4e9520ba63114b59fc

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

LOG: Add the 2022 papers to the C status tracking page

This adds the papers from Feb 2022 (parts 1 and 2) and May 2022.

Added: 


Modified: 
clang/www/c_status.html

Removed: 




diff  --git a/clang/www/c_status.html b/clang/www/c_status.html
index c0d2a3878c85..7e96dd27218d 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -866,7 +866,7 @@ C2x implementation status
 
   Adding a Fundamental Type for N-bit integers
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2763.pdf";>N2763
-  Clang 14
+  Clang 15
 
 
 
@@ -955,10 +955,127 @@ C2x implementation status
   https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2872.htm";>N2872
   Yes
 
+
+
+  @, $, and ‘ in the source/execution character set
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2701.htm";>N2701
+  Yes
+
+
+  The noreturn attribute
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2764.pdf";>N2764
+  Clang 15
+
+
+  Literal suffixes for bit-precise integers
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2775.pdf";>N2775
+  Clang 15
+
+
+  *_HAS_SUBNORM==0 implies what?
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2797.htm";>N2797
+  Yes
+
+
+  Disambiguate the storage class of some compound literals
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2819.pdf";>N2819
+  Unknown
+
+
+  Add annotations for unreachable control flow v2
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2826.pdf";>N2826
+  No
+
+
+  Unicode Sequences More Than 21 Bits are a Constraint Violation 
r0
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2828.htm";>N2828
+  Clang 3.6
+
+
+  Make assert() macro user friendly for C and C++ v2
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2829.htm";>N2829
+  No
+
+
+  Identifier Syntax using Unicode Standard Annex 31
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2836.pdf";>N2836
+  No
+
+
+  No function declarators without prototypes
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2841.htm";>N2841
+  Clang 15
+
+
+  Remove default argument promotions for _FloatN types
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2844.pdf";>N2844
+  Unknown
+
+
+  5.2.4.2.2 Cleanup, Again Again (N2806 update)
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2879.pdf";>N2879
+  Yes
+
+
+  Clarification for max exponent macros-update
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2882.pdf";>N2882
+  Unknown
+
+
+  Consistent, Warningless, and Intuitive Initialization with {}, 
revision 2
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2900.htm";>N2900
+  Unknown
+
+
+  Not-so-magic: typeof, r5
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2927.htm";>N2927
+  
+Partial
+  Clang supports typeof in GNU standards mode, but its
+  compatibility with this proposal is unknown. Also, Clang does not yet
+  support remove_quals.
+
+  
+
+
+  Revise spelling of keywords v7
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2934.pdf";>N2934
+  No
+
+
+  Make false and true first-class language features v8
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2935.pdf";>N2935
+  Clang 15
+
+
+  Properly define blocks as part of the grammar v3
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2937.pdf";>N2937
+  Yes
+
+
+
+  Indeterminate Values and Trap Representations
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2861.pdf";>N2861
+  Yes
+
+
+  Remove ATOMIC_VAR_INIT v2
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2886.htm";>N2886
+  No
+
+
+  Require exact-width integer type interfaces v2
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2888.htm";>N2888
+  Yes
+
+
+  Wording Change for Variably-Modified Types
+  https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2992.pdf";>N2992
+  Yes
+
 
 
 
 
 
 
-



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

[PATCH] D125677: [pseudo] Remove the explicit Accept actions.

[Haojian Wu via Phabricator via cfe-commits]

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

pull out the logic of detecting acceptable state out of glrReduce callback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125677

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/LRTable.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTable.cpp
  clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
  clang-tools-extra/pseudo/test/lr-build-basic.test
  clang-tools-extra/pseudo/test/lr-build-conflicts.test
  clang-tools-extra/pseudo/unittests/GLRTest.cpp
  clang-tools-extra/pseudo/unittests/LRTableTest.cpp

Index: clang-tools-extra/pseudo/unittests/LRTableTest.cpp
===
--- clang-tools-extra/pseudo/unittests/LRTableTest.cpp
+++ clang-tools-extra/pseudo/unittests/LRTableTest.cpp
@@ -33,7 +33,7 @@
   std::vector Entries = {
   {/* State */ 0, tokenSymbol(tok::semi), Action::shift(0)},
   {/* State */ 0, tokenSymbol(tok::semi), Action::reduce(0)},
-  {/* State */ 1, tokenSymbol(tok::eof), Action::accept(2)},
+  {/* State */ 1, tokenSymbol(tok::eof), Action::reduce(2)},
   {/* State */ 2, tokenSymbol(tok::semi), Action::reduce(1)}};
   GrammarTable GT;
   LRTable T = LRTable::buildForTests(GT, Entries);
@@ -41,7 +41,7 @@
   EXPECT_THAT(T.find(0, tokenSymbol(tok::semi)),
   UnorderedElementsAre(Action::shift(0), Action::reduce(0)));
   EXPECT_THAT(T.find(1, tokenSymbol(tok::eof)),
-  UnorderedElementsAre(Action::accept(2)));
+  UnorderedElementsAre(Action::reduce(2)));
   EXPECT_THAT(T.find(1, tokenSymbol(tok::semi)), IsEmpty());
   EXPECT_THAT(T.find(2, tokenSymbol(tok::semi)),
   UnorderedElementsAre(Action::reduce(1)));
Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -393,6 +393,29 @@
 "[  0, end) └─IDENTIFIER := tok[0]\n");
 }
 
+TEST_F(GLRTest, NoExplicitAccept) {
+  build(R"bnf(
+_ := test
+
+test := IDENTIFIER test
+test := IDENTIFIER
+  )bnf");
+  clang::LangOptions LOptions;
+  // Given the following input, and the grammar above, we perform two reductions
+  // of the nonterminal `test` when the next token is `eof`, verify that the
+  // parser stops at the right state.
+  const TokenStream &Tokens = cook(lex("id id", LOptions), LOptions);
+  auto LRTable = LRTable::buildSLR(*G);
+
+  const ForestNode &Parsed =
+  glrParse(Tokens, {*G, LRTable, Arena, GSStack}, id("test"));
+  EXPECT_EQ(Parsed.dumpRecursive(*G),
+"[  0, end) test := IDENTIFIER test\n"
+"[  0,   1) ├─IDENTIFIER := tok[0]\n"
+"[  1, end) └─test := IDENTIFIER\n"
+"[  1, end)   └─IDENTIFIER := tok[1]\n");
+}
+
 } // namespace
 } // namespace pseudo
 } // namespace clang
Index: clang-tools-extra/pseudo/test/lr-build-conflicts.test
===
--- clang-tools-extra/pseudo/test/lr-build-conflicts.test
+++ clang-tools-extra/pseudo/test/lr-build-conflicts.test
@@ -33,7 +33,6 @@
 # TABLE-NEXT: 'IDENTIFIER': shift state 2
 # TABLE-NEXT: 'expr': go to state 1
 # TABLE-NEXT: State 1
-# TABLE-NEXT: 'EOF': accept
 # TABLE-NEXT: '-': shift state 3
 # TABLE-NEXT: State 2
 # TABLE-NEXT: 'EOF': reduce by rule 1 'expr := IDENTIFIER'
Index: clang-tools-extra/pseudo/test/lr-build-basic.test
===
--- clang-tools-extra/pseudo/test/lr-build-basic.test
+++ clang-tools-extra/pseudo/test/lr-build-basic.test
@@ -22,7 +22,6 @@
 # TABLE-NEXT: 'expr': go to state 1
 # TABLE-NEXT: 'id': go to state 2
 # TABLE-NEXT: State 1
-# TABLE-NEXT: 'EOF': accept
 # TABLE-NEXT: State 2
 # TABLE-NEXT: 'EOF': reduce by rule 1 'expr := id'
 # TABLE-NEXT: State 3
Index: clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
===
--- clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
+++ clang-tools-extra/pseudo/lib/grammar/LRTableBuild.cpp
@@ -124,11 +124,11 @@
   auto FollowSets = followSets(G);
   for (StateID SID = 0; SID < Graph.states().size(); ++SID) {
 for (const Item &I : Graph.states()[SID].Items) {
-  // If we've just parsed the start symbol, we can accept the input.
-  if (G.lookupRule(I.rule()).Target == G.underscore() && !I.hasNext()) {
-Build.insert({SID, tokenSymbol(tok::eof), Action::accept(I.rule())});
+  // If we've just parsed the start symbol, this means we successfully parse
+  // the input. We don't add the reduce action of `_ := start_symbol` in the
+  // LRTable (the GLR parser handles it specifically).
+  if (G.lookup

[PATCH] D125677: [pseudo] Remove the explicit Accept actions.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:85
 
-  if (!PendingAccept.empty()) {
-LLVM_DEBUG({
-  llvm::dbgs() << llvm::formatv("Accept: {0} accepted result:\n",
- PendingAccept.size());
-  for (const auto &Accept : PendingAccept)
-llvm::dbgs() << "  - " << G.symbolName(Accept.Head->Payload->symbol())
- << "\n";
-});
-assert(PendingAccept.size() == 1);
-return *PendingAccept.front().Head->Payload;
-  }
+  const ForestNode *Result = nullptr;
+  StateID AcceptState = Params.Table.getGoToState(StartState, StartSymbol);

sammccall wrote:
> rather than mingling this with the glrReduce, I'd suggest collecting the set 
> of final heads first and then analyzing them afterwards.
> 
> This means looping a second time, but I think the logic around recognizing 
> patterns that look like `accept` might grow (e.g. if we want to incorporate 
> some error tolerance)
that sounds reasonable, but it requires some additional changes (to identify 
inactive heads in the callback of `glrReduce`):

- AddSteps now returns true/false whether there is any actions in 
LRTable[NewHead->state][nextTok];
- no available action in the acceptable state on the eof token in LRTable (see 
the change in lr-build-basic.test);


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125677

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

[PATCH] D125095: [Clang][AIX] Add .ref in frontend for AIX XCOFF to support `-bcdtors:csect` linker option

[Ting Wang via Phabricator via cfe-commits]

tingwang updated this revision to Diff 435107.
tingwang added a comment.
Herald added subscribers: llvm-commits, jdoerfert.

Update according to comments:
(1) Update docs
(2) Use `addVarTermAssoc`
(3) Remove redundant call to `updateAssociatedFunc(FFDtorTermAssoc...`
(4) Remove unnecessary `llvm::array_pod_sort` on `FFDtorTermAssoc`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125095

Files:
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGen/PowerPC/aix-init-ref-null.cpp
  clang/test/CodeGen/PowerPC/aix-ref-static-var.cpp
  clang/test/CodeGen/PowerPC/aix-ref-tls_init.cpp
  clang/test/CodeGenCXX/aix-static-init-debug-info.cpp
  clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
  clang/test/CodeGenCXX/aix-static-init.cpp
  llvm/docs/LangRef.rst

Index: llvm/docs/LangRef.rst
===
--- llvm/docs/LangRef.rst
+++ llvm/docs/LangRef.rst
@@ -7040,6 +7040,10 @@
 @b = internal global i32 2, comdat $a, section "abc", !associated !0
 !0 = !{i32* @a}
 
+On XCOFF target, the ``associated`` metadata indicates connection among static
+variables (static global variable, static class member etc.) and static init/
+term functions. This metadata lowers to ``.ref`` assembler pseudo-operation
+which prevents discarding of the functions in linker GC.
 
 '``prof``' Metadata
 ^^^
Index: clang/test/CodeGenCXX/aix-static-init.cpp
===
--- clang/test/CodeGenCXX/aix-static-init.cpp
+++ clang/test/CodeGenCXX/aix-static-init.cpp
@@ -38,6 +38,10 @@
   }
 } // namespace test4
 
+// CHECK: @_ZN5test12t1E = global %"struct.test1::Test1" zeroinitializer, align {{[0-9]+}}, !associated ![[ASSOC0:[0-9]+]]
+// CHECK: @_ZN5test12t2E = global %"struct.test1::Test1" zeroinitializer, align {{[0-9]+}}, !associated ![[ASSOC0:[0-9]+]]
+// CHECK: @_ZN5test21xE = global i32 0, align {{[0-9]+}}, !associated ![[ASSOC1:[0-9]+]]
+// CHECK: @_ZN5test31tE = global %"struct.test3::Test3" undef, align {{[0-9]+}}, !associated ![[ASSOC0:[0-9]+]]
 // CHECK: @_ZGVZN5test41fEvE11staticLocal = internal global i64 0, align 8
 // CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__sub_I__, i8* null }]
 // CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @_GLOBAL__D_a, i8* null }]
@@ -49,7 +53,7 @@
 // CHECK:   ret void
 // CHECK: }
 
-// CHECK: define internal void @__dtor__ZN5test12t1E() [[ATTR:#[0-9]+]] {
+// CHECK: define internal void @__dtor__ZN5test12t1E() [[ATTR:#[0-9]+]] !associated ![[ASSOC2:[0-9]+]] {
 // CHECK: entry:
 // CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t1E)
 // CHECK:   ret void
@@ -80,7 +84,7 @@
 // CHECK:   ret void
 // CHECK: }
 
-// CHECK: define internal void @__dtor__ZN5test12t2E() [[ATTR:#[0-9]+]] {
+// CHECK: define internal void @__dtor__ZN5test12t2E() [[ATTR:#[0-9]+]] !associated ![[ASSOC2:[0-9]+]] {
 // CHECK: entry:
 // CHECK:   call void @_ZN5test15Test1D1Ev(%"struct.test1::Test1"* @_ZN5test12t2E)
 // CHECK:   ret void
@@ -114,7 +118,7 @@
 // CHECK:   ret void
 // CHECK: }
 
-// CHECK: define internal void @__dtor__ZN5test31tE() [[ATTR:#[0-9]+]] {
+// CHECK: define internal void @__dtor__ZN5test31tE() [[ATTR:#[0-9]+]] !associated ![[ASSOC2:[0-9]+]] {
 // CHECK: entry:
 // CHECK:   call void @_ZN5test35Test3D1Ev(%"struct.test3::Test3"* @_ZN5test31tE)
 // CHECK:   ret void
@@ -155,7 +159,7 @@
 // CHECK:   ret void
 // CHECK: }
 
-// CHECK: define internal void @__dtor__ZZN5test41fEvE11staticLocal() [[ATTR:#[0-9]+]] {
+// CHECK: define internal void @__dtor__ZZN5test41fEvE11staticLocal() [[ATTR:#[0-9]+]] !associated ![[ASSOC2:[0-9]+]] {
 // CHECK: entry:
 // CHECK:   call void @_ZN5test45Test4D1Ev(%"struct.test4::Test4"* @_ZZN5test41fEvE11staticLocal)
 // CHECK:   ret void
@@ -192,3 +196,7 @@
 // CHECK:   call void @__finalize__ZN5test12t1E()
 // CHECK:   ret void
 // CHECK: }
+
+// CHECK: ![[ASSOC0]] = !{void ()* @{{_GLOBAL__sub_I__|_GLOBAL__D_a}}, void ()* @{{_GLOBAL__sub_I__|_GLOBAL__D_a}}}
+// CHECK: ![[ASSOC1]] = !{void ()* @_GLOBAL__sub_I__}
+// CHECK: ![[ASSOC2]] = !{void ()* @_GLOBAL__D_a}
Index: clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
===
--- clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
+++ clang/test/CodeGenCXX/aix-static-init-temp-spec-and-inline-var.cpp
@@ -44,8 +44,13 @@
 A A::instance = bar();
 } // namespace test2
 
+// CHECK: @_ZN5test12t0E = global %"struct.test1::Test1" zeroinitializer, align {{[0-9]+}}, !associated ![[ASSOC0:[0-9]+]]
+//

[PATCH] D127285: [analyzer] Fix assertion failure after getKnownValue call

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D127285#3566034 , @steakhal wrote:

> ... Try to mark named entities like `this`.

Could you please elaborate?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127285

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

[PATCH] D127270: [clang-format] Add space in placement new expression

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks added inline comments.



Comment at: clang/include/clang/Format/Format.h:3498
+/// \endcode
+bool AfterPlacementNew;
 /// If ``true``, put a space between operator overloading and opening

Please sort after `AfterOver...` here and all other occasions.



Comment at: clang/lib/Format/Format.cpp:1314
   LLVMStyle.SpaceBeforeParensOptions.AfterIfMacros = true;
+  LLVMStyle.SpaceBeforeParensOptions.AfterPlacementNew = true;
   LLVMStyle.SpaceBeforeRangeBasedForLoopColon = true;

This isn't needed, because the default CTor initializes it with true.
Or you change that, I don't know right now why all other attributes are 
initialized with false.



Comment at: clang/unittests/Format/FormatTest.cpp:15278
+  SpacePlacementNew.SpaceBeforeParens = FormatStyle::SBPO_Custom;
+  SpacePlacementNew.SpaceBeforeParensOptions.AfterPlacementNew = true;
+  verifyFormat("new (buf) T;", SpacePlacementNew);

Assert on that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127270

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

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

[gehry via Phabricator via cfe-commits]

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

If a union member is initialized, other members are still recorded in the 
container to be initialized.  This patch fixes this behavior.
Reference issue: https://github.com/llvm/llvm-project/issues/54748


https://reviews.llvm.org/D127293

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
@@ -552,3 +552,21 @@
   int A;
   // CHECK-FIXES-NOT: int A{};
 };
+
+struct S1 {
+  S1() {}
+  // CHECK-MESSAGES-NOT: warning:
+  union {
+int a = 0;
+int b;
+  };
+};
+
+struct S2 {
+  S2() : a{} {}
+  // CHECK-MESSAGES-NOT: warning:
+  union {
+int a;
+int b;
+  };
+};
Index: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
===
--- clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
+++ clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeMemberInitCheck.cpp
@@ -61,6 +61,17 @@
   }
 }
 
+void removeFieldInitialized(const FieldDecl *M,
+SmallPtrSetImpl &FieldDecls) {
+  const RecordDecl *R = M->getParent();
+  if (R && R->isUnion()) {
+// Erase all members in a union if any member of it is initialized.
+for (const auto *F : R->fields())
+  FieldDecls.erase(F);
+  } else
+FieldDecls.erase(M);
+}
+
 void removeFieldsInitializedInBody(
 const Stmt &Stmt, ASTContext &Context,
 SmallPtrSetImpl &FieldDecls) {
@@ -70,7 +81,7 @@
 hasLHS(memberExpr(member(fieldDecl().bind("fieldDecl")),
 Stmt, Context);
   for (const auto &Match : Matches)
-FieldDecls.erase(Match.getNodeAs("fieldDecl"));
+removeFieldInitialized(Match.getNodeAs("fieldDecl"), 
FieldDecls);
 }
 
 StringRef getName(const FieldDecl *Field) { return Field->getName(); }
@@ -418,13 +429,18 @@
 
   // Gather all fields (direct and indirect) that need to be initialized.
   SmallPtrSet FieldsToInit;
-  forEachField(ClassDecl, ClassDecl.fields(), [&](const FieldDecl *F) {
+  bool AnyMemberHasInitPerUnion = false;
+  forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
+ AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
 if (IgnoreArrays && F->getType()->isArrayType())
   return;
+if (F->hasInClassInitializer() && F->getParent()->isUnion())
+  AnyMemberHasInitPerUnion = true;
 if (!F->hasInClassInitializer() &&
 utils::type_traits::isTriviallyDefaultConstructible(F->getType(),
 Context) &&
-!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield())
+!isEmpty(Context, F->getType()) && !F->isUnnamedBitfield() &&
+!AnyMemberHasInitPerUnion)
   FieldsToInit.insert(F);
   });
   if (FieldsToInit.empty())
@@ -437,7 +453,7 @@
   if (Init->isAnyMemberInitializer() && Init->isWritten()) {
 if (IsUnion)
   return; // We can only initialize one member of a union.
-FieldsToInit.erase(Init->getAnyMember());
+removeFieldInitialized(Init->getAnyMember(), FieldsToInit);
   }
 }
 removeFieldsInitializedInBody(*Ctor->getBody(), Context, FieldsToInit);
@@ -478,7 +494,7 @@
   // Collect all fields but only suggest a fix for the first member of unions,
   // as initializing more than one union member is an error.
   SmallPtrSet FieldsToFix;
-  bool AnyMemberHasInitPerUnion = false;
+  AnyMemberHasInitPerUnion = false;
   forEachFieldWithFilter(ClassDecl, ClassDecl.fields(),
  AnyMemberHasInitPerUnion, [&](const FieldDecl *F) {
 if (!FieldsToInit.count(F))


Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-member-init.cpp
@@ -552,3 +552,21 @@
   int A;
   // CHECK-FIXES-NOT: int A{};
 };
+
+struct S1 {
+  S1() {}
+  // CHECK-MESSAGES-NOT: warning:
+  union {
+int a = 0;
+int b;
+  };
+};
+
+struct S2 {
+  S2() : a{} {}
+  // CHECK-MESSAGES-NOT: warning:
+  union {
+int a;
+int b;
+  };
+};
Index: clang-tools-extra/cl

[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 updated this revision to Diff 435108.
jansvoboda11 added a comment.
Herald added a project: clang.

Add operators for atomic types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125349

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp
  clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp

Index: clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/atomic-builtin-compound-assignment-overload.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -std=gnu++98 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+_Atomic unsigned an_atomic_uint;
+
+enum { an_enum_value = 1 };
+
+void enum1() { an_atomic_uint += an_enum_value; }
+
+void enum2() { an_atomic_uint |= an_enum_value; }
Index: clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/atomic-builtin-compound-assignment-overload.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -std=gnu++11 -emit-llvm -triple=x86_64-linux-gnu -o - %s | FileCheck %s
+
+_Atomic unsigned an_atomic_uint;
+
+enum { an_enum_value = 1 };
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum1v()
+void enum1() {
+  an_atomic_uint += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum2v()
+void enum2() {
+  an_atomic_uint |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum3RU7_Atomicj({{.*}})
+void enum3(_Atomic unsigned &an_atomic_uint_param) {
+  an_atomic_uint_param += an_enum_value;
+  // CHECK: atomicrmw add ptr
+}
+
+// CHECK-LABEL: define {{.*}}void @_Z5enum4RU7_Atomicj({{.*}})
+void enum4(_Atomic unsigned &an_atomic_uint_param) {
+  an_atomic_uint_param |= an_enum_value;
+  // CHECK: atomicrmw or ptr
+}
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -8202,6 +8202,7 @@
   Sema &S;
   ArrayRef Args;
   Qualifiers VisibleTypeConversionsQuals;
+  bool VisibleTypeIsAtomic;
   bool HasArithmeticOrEnumeralCandidateType;
   SmallVectorImpl &CandidateTypes;
   OverloadCandidateSet &CandidateSet;
@@ -8326,11 +8327,13 @@
   BuiltinOperatorOverloadBuilder(
 Sema &S, ArrayRef Args,
 Qualifiers VisibleTypeConversionsQuals,
+bool VisibleTypeIsAtomic,
 bool HasArithmeticOrEnumeralCandidateType,
 SmallVectorImpl &CandidateTypes,
 OverloadCandidateSet &CandidateSet)
 : S(S), Args(Args),
   VisibleTypeConversionsQuals(VisibleTypeConversionsQuals),
+  VisibleTypeIsAtomic(VisibleTypeIsAtomic),
   HasArithmeticOrEnumeralCandidateType(
 HasArithmeticOrEnumeralCandidateType),
   CandidateTypes(CandidateTypes),
@@ -8951,6 +8954,13 @@
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
   /*IsAssignmentOperator=*/isEqualOp);
 
+if (VisibleTypeIsAtomic) {
+  ParamTypes[0] = S.Context.getAtomicType(LeftBaseTy);
+  ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
+  S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
+/*IsAssignmentOperator=*/isEqualOp);
+}
+
 // Add this built-in operator as a candidate (VQ is 'volatile').
 if (VisibleTypeConversionsQuals.hasVolatile()) {
   ParamTypes[0] = S.Context.getVolatileType(LeftBaseTy);
@@ -9007,6 +9017,13 @@
 // Add this built-in operator as a candidate (VQ is empty).
 ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
+
+if (VisibleTypeIsAtomic) {
+  ParamTypes[0] = S.Context.getAtomicType(LeftBaseTy);
+  ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
+  S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
+}
+
 if (VisibleTypeConversionsQuals.hasVolatile()) {
   // Add this built-in operator as a candidate (VQ is 'volatile').
   ParamTypes[0] = LeftBaseTy;
@@ -9179,8 +9196,11 @@
   // candidate types or either arithmetic or enumeral candidate types.
   Qualifiers VisibleTypeConversionsQuals;
   VisibleTypeConversionsQuals.addConst();
-  for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx)
+  bool VisibleTypeIsAtomic = false;
+  for (unsigned ArgIdx = 0, N = Args.size(); ArgIdx != N; ++ArgIdx) {
 VisibleTypeConversionsQuals += CollectVRQualifiers(Context, Args[ArgIdx]);
+VisibleTypeIsAtomic |= Args[ArgIdx]->getType()->isAtomicType();
+  }
 
   bool HasNonRecordCandidateType = false;
   bool HasArithmeticOrEnumeralCandidateType = false;
@@ -9213,6 +9233,7 @@
  

[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

I tried the approach suggested by @ahatanak. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125349

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

[PATCH] D125402: [clang][diag] warn if function returns class type by-const-value

[Namgoo Lee via Phabricator via cfe-commits]

nlee updated this revision to Diff 435113.
nlee added a comment.

Updated diff to pass clang-format test.


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

https://reviews.llvm.org/D125402

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/Misc/warning-wall.c
  clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp

Index: clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/warn-pessimizing-return-by-const.cpp
@@ -0,0 +1,58 @@
+// RUN: %clang_cc1 -fsyntax-only -Wpessimizing-return-by-const -std=c++11 -verify %s
+
+struct S1 {};
+
+S1 f1() {
+  return S1{};
+}
+
+const S1 f1_const() { // expected-warning{{const qualifying the return value prevents move semantics}}
+  return S1{};
+}
+
+void run1() {
+  S1 s1;
+  s1 = f1();
+  s1 = f1_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'S1'}}
+}
+
+struct S2 {
+  S2() = default;
+  S2(const S2 &) = default;
+  S2 &operator=(const S2 &) = default;
+  S2 &operator=(S2 &&) = default;
+};
+
+S2 f2() {
+  return S2{};
+}
+
+const S2 f2_const() { // expected-warning{{const qualifying the return value prevents move semantics}}
+  return S2{};
+}
+
+void run2() {
+  S2 s2;
+  s2 = f2();
+  s2 = f2_const(); // expected-note{{copy assignment is invoked here even if move assignment is supported for type 'S2'}}
+}
+
+struct S3 {
+  S3() = default;
+  S3(const S3 &) = default;
+  S3 &operator=(const S3 &) = default;
+};
+
+S3 f3() {
+  return S3{};
+}
+
+const S3 f3_const() { // do not warn if move assignment operator is deleted
+  return S3{};
+}
+
+void run3() {
+  S3 s3;
+  s3 = f3();
+  s3 = f3_const();
+}
Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -33,6 +33,7 @@
 CHECK-NEXT:  -Wreturn-std-move
 CHECK-NEXT:  -Wself-move
 CHECK-NEXT:-Wmultichar
+CHECK-NEXT:-Wpessimizing-return-by-const
 CHECK-NEXT:-Wrange-loop-construct
 CHECK-NEXT:-Wreorder
 CHECK-NEXT:  -Wreorder-ctor
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -13810,9 +13810,10 @@
   ArgsArray = ArgsArray.slice(1);
 }
 
-// Check for a self move.
-if (Op == OO_Equal)
+if (Op == OO_Equal) {
   DiagnoseSelfMove(Args[0], Args[1], OpLoc);
+  DiagnosePessimizingConstReturn(Args[1], OpLoc);
+}
 
 if (ImplicitThis) {
   QualType ThisType = Context.getPointerType(ImplicitThis->getType());
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -16800,6 +16800,47 @@
 << RHSExpr->getSourceRange();
 }
 
+void Sema::DiagnosePessimizingConstReturn(const Expr *RHSExpr,
+  SourceLocation OpLoc) {
+  if (!getLangOpts().CPlusPlus11 || inTemplateInstantiation())
+return;
+
+  RHSExpr = RHSExpr->IgnoreParenImpCasts();
+
+  // Is RHS a function call expression?
+  const CallExpr *CE = dyn_cast(RHSExpr);
+  if (!CE)
+return;
+
+  const Decl *CD = CE->getCalleeDecl();
+  if (!CD)
+return;
+
+  const FunctionDecl *FD = dyn_cast(CD);
+  if (!FD)
+return;
+
+  // Is the return type by-const-value of a struct or class type?
+  const QualType RT = FD->getReturnType();
+  if (!RT->isStructureOrClassType() || !RT.isConstQualified())
+return;
+
+  // Is the move assignment operator deleted?
+  bool MoveAssignmentIsDeleted = false;
+  for (const CXXMethodDecl *iter : RT->getAsCXXRecordDecl()->methods())
+if (iter->isMoveAssignmentOperator() && iter->isDeleted())
+  MoveAssignmentIsDeleted = true;
+
+  if (RT->getAsCXXRecordDecl()->hasDefinition() &&
+  RT->getAsCXXRecordDecl()->hasMoveAssignment() &&
+  !MoveAssignmentIsDeleted) {
+const SourceRange ReturnTypeLoc = FD->getReturnTypeSourceRange();
+Diag(ReturnTypeLoc.getBegin(), diag::warn_pessimizing_return_by_const);
+Diag(OpLoc, diag::note_pessimizing_return_by_const)
+<< RT.getUnqualifiedType();
+  }
+}
+
 //===--- Layout compatibility --//
 
 static bool isLayoutCompatible(ASTContext &C, QualType T1, QualType T2);
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5070,6 +5070,9 @@
   void DiagnoseSelfMove(const Expr *LHSExpr, const Expr *R

[PATCH] D126944: [Clang] Fix memory leak due to TemplateArgumentListInfo used in AST node.

[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; I think it's reasonable to not have test coverage specific to the changes 
here (the existing test coverage already uncovered this issue). It might be 
worth mentioning that the leak was fixed in a release note, however. Also, I 
did find one naming convention issue that should be fixed before landing.

Thanks for fixing this up!




Comment at: clang/include/clang/AST/DeclTemplate.h:2778-2779
 
-  const TemplateArgumentListInfo &getTemplateArgsInfo() const {
+  const ASTTemplateArgumentListInfo *getTemplateArgsInfo() const {
 return TemplateArgsInfo;
   }

browneee wrote:
> aaron.ballman wrote:
> > Do we want to assert that `TemplateArgsInfo` is nonnull and return a const 
> > reference instead?
> > 
> > Basically, I'm wondering if we can remove a bunch of the pointers and go 
> > with references as much as possible; it seems to me that a null pointer is 
> > a sign of an error, same as with `TemplateArgs`.
> This may be why https://reviews.llvm.org/D126937 is failing tests.
> 
> My goals is to fix the memory leak, so the sanitizer buildbots are clean. I 
> would agree there is probably more cleanup to do for this member variable 
> after https://reviews.llvm.org/D126944 - but fixing the memory leak is the 
> urgent thing for me.
Okay, I'm fine not handling the extra safety measures at the moment.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:5563-5565
+  for (const TemplateArgumentLoc& arg : ArgInfo->arguments()) {
+TemplateArgInfo.addArgument(arg);
+  }

aaron.ballman wrote:
> More coding standard changes -- btw, it looks like you need to run the patch 
> through clang-format 
> (https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting).
It looks like you may have missed the rename suggestion of switching `arg` to 
`Arg` per our usual coding conventions. You can fix that when landing though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126944

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

[PATCH] D126937: Fix memleak in VarTemplateSpecializationDecl

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D126937#3555731 , @aaron.ballman 
wrote:

> In D126937#3555243 , @browneee 
> wrote:
>
>> I was also looking into fixing this: https://reviews.llvm.org/D126944
>>
>> I'm not yet sure if my changes are correct.
>
> I did a review on https://reviews.llvm.org/D126944, but I'll let the two of 
> you decide which review to move forward with rather than review them both in 
> tandem. :-)

FYI, I've accepted that other review (we needed to get the leak fixed ASAP).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126937

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

[PATCH] D127207: [flang][driver] Fix support for `-x`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/lib/Frontend/CompilerInvocation.cpp:268
+ // pre-processed inputs.
+.Case("f95", Language::Fortran)
+.Case("f95-cpp-input", Language::Fortran)

rovka wrote:
> ekieri wrote:
> > Is there a reason to change from "f90" to "f95"? In my understanding, "f90" 
> > is more idiomatic for free-form fortran of any standard >= 90.
> At least for `gfortran`, `f90` doesn't seem to be supported, only `f77`, 
> `f77-cpp-input`, `f95`,  `f95-cpp-input` are.
> https://raw.githubusercontent.com/gcc-mirror/gcc/master/gcc/doc/invoke.texi#:~:text=f77%20%20f77%2Dcpp%2Dinput%20f95%20%20f95%2Dcpp%2Dinput
> 
> Note that these are not file extensions, but values for the `-x` option.
> Note that these are not file extensions, but values for the -x option.
Indeed, thanks Diana! For `clangDriver,` the available values for Fortran are 
defined  [[ 
https://github.com/llvm/llvm-project/blob/5fee1799f4d8da59c251e2d04172fc2f387cbe54/clang/include/clang/Driver/Types.def#L80-L81
 | here ]].



Comment at: flang/test/Driver/parse-ir-error.f95:16
+!
+! CHECK: error: expected integer
+! CHECK: error: Could not parse IR

rovka wrote:
> Nit: I think checking just the "Could not parse IR" message should be enough.
👍🏻 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127207

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

[PATCH] D127207: [flang][driver] Fix support for `-x`

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 435115.
awarzynski added a comment.

Remove redundant `CHECK` line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127207

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/input-from-stdin-llvm.ll
  flang/test/Driver/input-from-stdin.f90
  flang/test/Driver/linker-flags.f90
  flang/test/Driver/parse-error.f95
  flang/test/Driver/parse-error.ll
  flang/test/Driver/parse-ir-error.f95

Index: flang/test/Driver/parse-ir-error.f95
===
--- /dev/null
+++ flang/test/Driver/parse-ir-error.f95
@@ -0,0 +1,18 @@
+! This file is a valid Fortran file, but we force the driver to treat it as an
+! LLVM file (with the `-x` flag). This way we verify that the driver correctly
+! rejects invalid LLVM IR input.
+
+!--
+! RUN LINES
+!--
+! Input type is implicit (correctly assumed to be Fortran)
+! RUN: %flang_fc1 -S %s
+! Input type is explicitly set as LLVM IR
+! RUN: not %flang -S -x ir %s 2>&1 | FileCheck %s
+
+!
+! EXPECTED OUTPUT
+!
+! CHECK: error: Could not parse IR
+
+end program
Index: flang/test/Driver/parse-error.ll
===
--- /dev/null
+++ flang/test/Driver/parse-error.ll
@@ -0,0 +1,23 @@
+; This file is a valid LLVM IR file, but we force the driver to treat it as
+; Fortran (with the `-x` flag). This way we verify that the driver
+; correctly rejects invalid Fortran input.
+
+;--
+; RUN LINES
+;--
+; Input type is implicit (correctly assumed to be LLVM IR)
+; RUN: %flang_fc1 -S %s -o -
+
+; Input type is explicitly set as Fortran
+; Verify that parsing errors are correctly reported by the driver
+; Focuses on actions inheriting from the following:
+; * PrescanAndSemaAction (-fsyntax-only)
+; * PrescanAndParseAction (-fdebug-unparse-no-sema)
+; RUN: not %flang_fc1 -fdebug-unparse-no-sema -x f95 %s 2>&1 | FileCheck %s --check-prefix=ERROR
+; RUN: not %flang_fc1 -fsyntax-only %s -x f95 2>&1 | FileCheck %s --check-prefix=ERROR
+
+; ERROR: Could not parse {{.*}}parse-error.f95
+
+define void @foo() {
+  ret void
+}
Index: flang/test/Driver/parse-error.f95
===
--- flang/test/Driver/parse-error.f95
+++ /dev/null
@@ -1,11 +0,0 @@
-! Verify that parsing errors are correctly reported by the driver
-! Focuses on actions inheriting from the following:
-! * PrescanAndSemaAction (-fsyntax-only)
-! * PrescanAndParseAction (-fdebug-unparse-no-sema)
-
-! RUN: not %flang_fc1 -fdebug-unparse-no-sema %s 2>&1 | FileCheck %s --check-prefix=ERROR
-! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=ERROR
-
-! ERROR: Could not parse {{.*}}parse-error.f95
-
-"This file will not parse"
Index: flang/test/Driver/linker-flags.f90
===
--- flang/test/Driver/linker-flags.f90
+++ flang/test/Driver/linker-flags.f90
@@ -20,7 +20,7 @@
 !
 ! Compiler invocation to generate the object file
 ! CHECK-LABEL: {{.*}} "-emit-obj"
-! CHECK-SAME:  "-o" "[[object_file:.*]]" {{.*}}Inputs/hello.f90
+! CHECK-SAME:  "-o" "[[object_file:.*\.o]]" {{.*}}Inputs/hello.f90
 
 ! Linker invocation to generate the executable
 ! CHECK-LABEL:  "/usr/bin/ld"
Index: flang/test/Driver/input-from-stdin.f90
===
--- flang/test/Driver/input-from-stdin.f90
+++ flang/test/Driver/input-from-stdin.f90
@@ -1,4 +1,4 @@
-! Verify that reading from stdin works as expected
+! Verify that reading from stdin works as expected - Fortran input
 
 !--
 ! FLANG DRIVER (flang)
Index: flang/test/Driver/input-from-stdin-llvm.ll
===
--- /dev/null
+++ flang/test/Driver/input-from-stdin-llvm.ll
@@ -0,0 +1,26 @@
+; Verify that reading from stdin works as expected - LLVM input
+
+;--
+; RUN LINES
+;--
+; Input type is implicit - assumed to be Fortran. As the input is provided via
+; stdin, the file extension is not relevant here.
+; RUN: cat %s | not %flang -S - -o -
+; RUN: cat %s | not %flang_fc1 -S - -o -
+
+; Input type is explicit
+; RUN: cat %s | %flang -x ir -S - -o - | FileCheck %s
+; RUN: cat %s | %flang_fc1 -x ir -S - -o - | FileCheck %s
+
+;
+; EXPECTED OUTPUT
+;
+; CHECK-LABEL: foo:
+; CHECK: ret
+
+;--
+; INPUT
+;--
+define void @foo() {
+  ret void
+}
Index: flang/lib/Frontend/FrontendActions.cpp
===
--- flang/lib/Frontend/FrontendActions.cpp
+++ flang/lib/Frontend/FrontendActions.cpp
@@ -42,6 +42,7 @@
 #includ

[clang] 98d4f06 - Correcting some links in the C status page

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-06-08T08:52:31-04:00
New Revision: 98d4f0651a7f90df161167362e2060e0b4de7969

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

LOG: Correcting some links in the C status page

The paper titles were correct, but the document number and links were
incorrect (typo'ed numbers).

Added: 


Modified: 
clang/www/c_status.html

Removed: 




diff  --git a/clang/www/c_status.html b/clang/www/c_status.html
index 7e96dd27218d..3ba814c757cb 100644
--- a/clang/www/c_status.html
+++ b/clang/www/c_status.html
@@ -650,12 +650,12 @@ C2x implementation status
 
 
   nodiscard attribute
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2667.pdf";>N2667
+  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2267.pdf";>N2267
   Clang 9
 
 
   maybe_unused attribute
-  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2670.pdf";>N2670
+  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2270.pdf";>N2270
   Clang 9
 
 



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

[PATCH] D127246: [LinkerWrapper] Rework the linker wrapper and use owning binaries

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 435118.
jhuber6 added a comment.

There was a problem where the Triple and Arch data would be deallocated when 
the LTO pass took ownership of every single file. Add a UniqueStringSaver to 
make sure they are still accessible after linking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127246

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ArchiveWriter.h"
 #include "llvm/Object/Binary.h"
+#include "llvm/Object/IRObjectFile.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/OffloadBinary.h"
 #include "llvm/Support/CommandLine.h"
@@ -68,8 +69,7 @@
cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt
-TargetFeatures("target-feature",
-   cl::desc("Target features for triple"),
+TargetFeatures("target-feature", cl::desc("Target features for triple"),
cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt OptLevel("opt-level",
@@ -114,9 +114,8 @@
cl::value_desc(" or ="),
cl::cat(ClangLinkerWrapperCategory));
 
-static cl::opt Verbose("v",
- cl::desc("Verbose output from tools"),
- 
+static cl::opt Verbose("v", cl::desc("Verbose output from tools"),
+
  cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt DebugInfo(
@@ -165,63 +164,43 @@
 /// different but it should work for what is passed here.
 static constexpr unsigned FatbinaryOffset = 0x50;
 
-/// Information for a device offloading file extracted from the host.
-struct DeviceFile {
-  DeviceFile(OffloadKind Kind, StringRef TheTriple, StringRef Arch,
- StringRef Filename)
-  : Kind(Kind), TheTriple(TheTriple), Arch(Arch), Filename(Filename) {}
+using OffloadingImage = OffloadBinary::OffloadingImage;
+
+/// A class to contain the binary information for a single OffloadBinary.
+class OffloadFile : public OwningBinary {
+public:
+  using TargetID = std::pair;
+
+  OffloadFile(std::unique_ptr Binary,
+  std::unique_ptr Buffer)
+  : OwningBinary(std::move(Binary), std::move(Buffer)) {}
 
-  OffloadKind Kind;
-  std::string TheTriple;
-  std::string Arch;
-  std::string Filename;
+  /// We use the Triple and Architecture pair to group linker inputs together.
+  /// This conversion function lets us use these files in a hash-map.
+  operator TargetID() const {
+return std::make_pair(getBinary()->getTriple(), getBinary()->getArch());
+  }
 };
 
 namespace llvm {
-/// Helper that allows DeviceFile to be used as a key in a DenseMap. For now we
-/// assume device files with matching architectures and triples but different
-/// offloading kinds should be handlded together, this may not be true in the
-/// future.
-
-// Provide DenseMapInfo for OffloadKind.
+// Provide DenseMapInfo so that OffloadKind can be used in a DenseMap.
 template <> struct DenseMapInfo {
   static inline OffloadKind getEmptyKey() { return OFK_LAST; }
   static inline OffloadKind getTombstoneKey() {
 return static_cast(OFK_LAST + 1);
   }
-  static unsigned getHashValue(const OffloadKind &Val) { return Val * 37U; }
+  static unsigned getHashValue(const OffloadKind &Val) { return Val; }
 
   static bool isEqual(const OffloadKind &LHS, const OffloadKind &RHS) {
 return LHS == RHS;
   }
 };
-template <> struct DenseMapInfo {
-  static DeviceFile getEmptyKey() {
-return {DenseMapInfo::getEmptyKey(),
-DenseMapInfo::getEmptyKey(),
-DenseMapInfo::getEmptyKey(),
-DenseMapInfo::getEmptyKey()};
-  }
-  static DeviceFile getTombstoneKey() {
-return {DenseMapInfo::getTombstoneKey(),
-DenseMapInfo::getTombstoneKey(),
-DenseMapInfo::getTombstoneKey(),
-DenseMapInfo::getTombstoneKey()};
-  }
-  static unsigned getHashValue(const DeviceFile &I) {
-return DenseMapInfo::getHashValue(I.TheTriple) ^
-   DenseMapInfo::getHashValue(I.Arch);
-  }
-  static bool isEqual(const DeviceFile &LHS, const DeviceFile &RHS) {
-return LHS.TheTriple == RHS.TheTriple && LHS.Arch == RHS.Arch;
-  }
-};
 } // namespace llvm
 
 namespace {
 
 Error extractFromBuffer(std::unique_ptr Buffer,
-SmallVectorImpl &DeviceFiles);
+SmallVectorImpl &DeviceFiles);
 
 void printCommands(ArrayRef CmdArgs) {
   if (CmdArgs.empty())
@@ -232,7 +211,7 @@
 llvm::errs() << *IC << (std::next(IC) != IE ? " " : "\n");
 }
 
-// Forward user requested arguments to the device linking job.
+//

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D126907#3565491 , @ChuanqiXu wrote:

> Oh, the diff part is small really. The change looks good to me and I've 
> tested it for libcxx.
>
> But it crashed at "clang/lib/Sema/SemaTemplateInstantiate.cpp:1852: 
> clang::QualType 
> {anonymous}::TemplateInstantiator::TransformTemplateTypeParmType(clang::TypeLocBuilder&,
>  clang::TemplateTypeParmTypeLoc): Assertion `Arg.getKind() == 
> TemplateArgument::Type && "Template argument kind mismatch"' failed" in an 
> internal library. I am trying to get a reduced example. I would suggest to do 
> more testing with other workloads (with libstdc++ >= 10)

Thanks so much for the feedback, and for running it!   I also checked out the 
basics of libcxx, and it worked fine (as well as SOME libstdc++ repros, but not 
extensively).  Any reproducer you could give me would be great!


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] D127196: [clang][dataflow] Enable use of synthetic properties on all Value instances.

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 435121.
wyt added a comment.

Remove cast to StructValues since get/setProperty can be used directly on 
Values now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127196

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -312,7 +312,7 @@
 if (const auto *E = selectFirst(
 "call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
   getASTContext( {
-  auto &ConstructorVal = *cast(Env.createValue(E->getType()));
+  auto &ConstructorVal = *Env.createValue(E->getType());
   ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(false));
   Env.setValue(*Env.getStorageLocation(*E, SkipPast::None), ConstructorVal);
 } else if (const auto *E = selectFirst(
@@ -327,8 +327,7 @@
   Env.getStorageLocation(*Object, SkipPast::ReferenceThenPointer);
   assert(ObjectLoc != nullptr);
 
-  auto &ConstructorVal =
-  *cast(Env.createValue(Object->getType()));
+  auto &ConstructorVal = *Env.createValue(Object->getType());
   ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(true));
   Env.setValue(*ObjectLoc, ConstructorVal);
 }
@@ -342,13 +341,11 @@
 Decl->getName() != "SpecialBool")
   return false;
 
-auto *IsSet1 = cast_or_null(
-cast(&Val1)->getProperty("is_set"));
+auto *IsSet1 = cast_or_null(Val1.getProperty("is_set"));
 if (IsSet1 == nullptr)
   return true;
 
-auto *IsSet2 = cast_or_null(
-cast(&Val2)->getProperty("is_set"));
+auto *IsSet2 = cast_or_null(Val2.getProperty("is_set"));
 if (IsSet2 == nullptr)
   return false;
 
@@ -365,18 +362,16 @@
 Decl->getName() != "SpecialBool")
   return true;
 
-auto *IsSet1 = cast_or_null(
-cast(&Val1)->getProperty("is_set"));
+auto *IsSet1 = cast_or_null(Val1.getProperty("is_set"));
 if (IsSet1 == nullptr)
   return true;
 
-auto *IsSet2 = cast_or_null(
-cast(&Val2)->getProperty("is_set"));
+auto *IsSet2 = cast_or_null(Val2.getProperty("is_set"));
 if (IsSet2 == nullptr)
   return true;
 
 auto &IsSet = MergedEnv.makeAtomicBoolValue();
-cast(&MergedVal)->setProperty("is_set", IsSet);
+MergedVal.setProperty("is_set", IsSet);
 if (Env1.flowConditionImplies(*IsSet1) &&
 Env2.flowConditionImplies(*IsSet2))
   MergedEnv.addToFlowCondition(IsSet);
@@ -426,32 +421,31 @@
   /*[[p4]]*/
 }
   )";
-  runDataflow(Code,
-  [](llvm::ArrayRef<
- std::pair>>
- Results,
- ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
- Pair("p2", _), Pair("p1", _)));
-const Environment &Env1 = Results[3].second.Env;
-const Environment &Env2 = Results[2].second.Env;
-const Environment &Env3 = Results[1].second.Env;
-const Environment &Env4 = Results[0].second.Env;
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p4", _), Pair("p3", _),
+ Pair("p2", _), Pair("p1", _)));
+const Environment &Env1 = Results[3].second.Env;
+const Environment &Env2 = Results[2].second.Env;
+const Environment &Env3 = Results[1].second.Env;
+const Environment &Env4 = Results[0].second.Env;
 
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
 
-auto GetFooValue = [FooDecl](const Environment &Env) {
-  return cast(
-  cast(Env.getValue(*FooDecl, SkipPast::None))
-  ->getProperty("is_set"));
-};
+auto GetFooValue = [FooDecl](const Environment &Env) {
+  return cast(
+  Env.getValue(*FooDecl, SkipPast::None)->getProperty("is_set"));
+};
 
-EXPECT_FALSE(Env1.flowConditionImplies(*GetFooValue(Env1)));
-EXPECT_TRUE(Env2.flowConditionImplies(*GetFooValue(Env2)));
-EXPECT_TRUE(Env3.flow

[PATCH] D127196: [clang][dataflow] Enable use of synthetic properties on all Value instances.

[weiyi via Phabricator via cfe-commits]

wyt marked 2 inline comments as done.
wyt added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/Value.h:29
 /// Base class for all values computed by abstract interpretation.
+/// All Value instances should be separately allocated and stored by pointer
+/// for pointer stability.

gribozavr2 wrote:
> sgatev wrote:
> > I'm not sure I understand what is meant by "separately allocated" and 
> > "stored by pointer". Could you please clarify?
> 
(See suggested fix by Dmitri)
When we create a new Value instance, we let DataflowAnalysisCtx take ownership 
of the value and refer to it by pointer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127196

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

[PATCH] D54943: [clang-tidy] implement new check 'misc-const-correctness' to add 'const' to unmodified variables

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.h:35
+TransformPointersAsValues(
+Options.get("TransformPointersAsValues", false)) {}
+

njames93 wrote:
> It may be worth adding some validation to these. If AnalyzeValues, 
> AnalyzeReferences and WarnPointersAsValues are all false, this whole check is 
> basically a no-op.
Whats the proper way to react? Short-circuit the `registerMatchers`, similar to 
language support?
I think thats the way I would go about this.


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

https://reviews.llvm.org/D54943

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

[PATCH] D126779: [analyzer] Fix assertion in simplifySymbolCast

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126779

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

[PATCH] D127246: [LinkerWrapper] Rework the linker wrapper and use owning binaries

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 435122.
jhuber6 added a comment.

Add use of bitcode libraries so this works on AMD.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127246

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ArchiveWriter.h"
 #include "llvm/Object/Binary.h"
+#include "llvm/Object/IRObjectFile.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/OffloadBinary.h"
 #include "llvm/Support/CommandLine.h"
@@ -68,8 +69,7 @@
cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt
-TargetFeatures("target-feature",
-   cl::desc("Target features for triple"),
+TargetFeatures("target-feature", cl::desc("Target features for triple"),
cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt OptLevel("opt-level",
@@ -114,9 +114,8 @@
cl::value_desc(" or ="),
cl::cat(ClangLinkerWrapperCategory));
 
-static cl::opt Verbose("v",
- cl::desc("Verbose output from tools"),
- 
+static cl::opt Verbose("v", cl::desc("Verbose output from tools"),
+
  cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt DebugInfo(
@@ -165,63 +164,43 @@
 /// different but it should work for what is passed here.
 static constexpr unsigned FatbinaryOffset = 0x50;
 
-/// Information for a device offloading file extracted from the host.
-struct DeviceFile {
-  DeviceFile(OffloadKind Kind, StringRef TheTriple, StringRef Arch,
- StringRef Filename)
-  : Kind(Kind), TheTriple(TheTriple), Arch(Arch), Filename(Filename) {}
+using OffloadingImage = OffloadBinary::OffloadingImage;
+
+/// A class to contain the binary information for a single OffloadBinary.
+class OffloadFile : public OwningBinary {
+public:
+  using TargetID = std::pair;
+
+  OffloadFile(std::unique_ptr Binary,
+  std::unique_ptr Buffer)
+  : OwningBinary(std::move(Binary), std::move(Buffer)) {}
 
-  OffloadKind Kind;
-  std::string TheTriple;
-  std::string Arch;
-  std::string Filename;
+  /// We use the Triple and Architecture pair to group linker inputs together.
+  /// This conversion function lets us use these files in a hash-map.
+  operator TargetID() const {
+return std::make_pair(getBinary()->getTriple(), getBinary()->getArch());
+  }
 };
 
 namespace llvm {
-/// Helper that allows DeviceFile to be used as a key in a DenseMap. For now we
-/// assume device files with matching architectures and triples but different
-/// offloading kinds should be handlded together, this may not be true in the
-/// future.
-
-// Provide DenseMapInfo for OffloadKind.
+// Provide DenseMapInfo so that OffloadKind can be used in a DenseMap.
 template <> struct DenseMapInfo {
   static inline OffloadKind getEmptyKey() { return OFK_LAST; }
   static inline OffloadKind getTombstoneKey() {
 return static_cast(OFK_LAST + 1);
   }
-  static unsigned getHashValue(const OffloadKind &Val) { return Val * 37U; }
+  static unsigned getHashValue(const OffloadKind &Val) { return Val; }
 
   static bool isEqual(const OffloadKind &LHS, const OffloadKind &RHS) {
 return LHS == RHS;
   }
 };
-template <> struct DenseMapInfo {
-  static DeviceFile getEmptyKey() {
-return {DenseMapInfo::getEmptyKey(),
-DenseMapInfo::getEmptyKey(),
-DenseMapInfo::getEmptyKey(),
-DenseMapInfo::getEmptyKey()};
-  }
-  static DeviceFile getTombstoneKey() {
-return {DenseMapInfo::getTombstoneKey(),
-DenseMapInfo::getTombstoneKey(),
-DenseMapInfo::getTombstoneKey(),
-DenseMapInfo::getTombstoneKey()};
-  }
-  static unsigned getHashValue(const DeviceFile &I) {
-return DenseMapInfo::getHashValue(I.TheTriple) ^
-   DenseMapInfo::getHashValue(I.Arch);
-  }
-  static bool isEqual(const DeviceFile &LHS, const DeviceFile &RHS) {
-return LHS.TheTriple == RHS.TheTriple && LHS.Arch == RHS.Arch;
-  }
-};
 } // namespace llvm
 
 namespace {
 
 Error extractFromBuffer(std::unique_ptr Buffer,
-SmallVectorImpl &DeviceFiles);
+SmallVectorImpl &DeviceFiles);
 
 void printCommands(ArrayRef CmdArgs) {
   if (CmdArgs.empty())
@@ -232,7 +211,7 @@
 llvm::errs() << *IC << (std::next(IC) != IE ? " " : "\n");
 }
 
-// Forward user requested arguments to the device linking job.
+/// Forward user requested arguments to the device linking job.
 void renderXLinkerArgs(SmallVectorImpl &Args, StringRef Triple) {
   for (StringRef Arg : Lin

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117229

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

[PATCH] D127287: clang: Introduce -fexperimental-max-bitint-width

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Given that this is a hidden option we expect to remove... do we add a release 
note for it? I think we probably should (for user awareness in case more users 
are in the same situation as the original reporter), and that gives a place to 
document that we will be removing the option in the future.




Comment at: clang/include/clang/Basic/LangOptions.def:448
 
+BENIGN_VALUE_LANGOPT(MaxBitIntWidth, 32, 128, "Maximum width of a _BitInt")
+

Should we add a comment about how this is expected to be removed in the future 
once the backend work is fixed?



Comment at: clang/include/clang/Basic/TargetInfo.h:239
 
+  unsigned MaxBitIntWidth;
+

Do we want to make this into an `Optional` so we don't have to use 0 
as a sentinel value? (I don't feel strongly, but we already use `Optional` in 
this interface anyways.)



Comment at: clang/include/clang/Driver/Options.td:6088
+  MetaVarName<"">,
+  HelpText<"Set the maximum bitwidth for _BitInt (experimental)">,
+  MarshallingInfoInt, "0">;

Maybe?



Comment at: clang/lib/Serialization/ASTReader.cpp:317
 #define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
-#define BENIGN_VALUE_LANGOPT(Name, Type, Bits, Default, Description)
+#define BENIGN_VALUE_LANGOPT(Name, Bits, Default, Description)
 #include "clang/Basic/LangOptions.def"

Good catch!



Comment at: clang/test/Sema/large-bit-int.c:1
+// RUN: %clang_cc1 -fexperimental-max-bitint-width=1024 -fsyntax-only -verify 
%s -Wno-unused -triple x86_64-gnu-linux
+

Thoughts on adding a frontend diagnostic if the user specifies a value larger 
than `llvm::IntegerType::MAX_INT_BITS`? I'm on the fence. OTOneH, this gives a 
good user experience in the event of mistypes in the command line, OTOtherH, 
it's a cc1-only option that we expect to remove in the (hopefully) near future.

Also, I don't think the triple is needed and I'm pretty sure `-Wno-unused` is 
the default already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127287

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

[PATCH] D127246: [LinkerWrapper] Rework the linker wrapper and use owning binaries

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 435124.
jhuber6 added a comment.

Sorry for this noise. This is a pretty large change but shouldn't affect any 
functionality and passes all the tests I know of, so this should be good to 
land. Let me know if you have any objections to how I've structured this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127246

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/ArchiveWriter.h"
 #include "llvm/Object/Binary.h"
+#include "llvm/Object/IRObjectFile.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/OffloadBinary.h"
 #include "llvm/Support/CommandLine.h"
@@ -68,8 +69,7 @@
cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt
-TargetFeatures("target-feature",
-   cl::desc("Target features for triple"),
+TargetFeatures("target-feature", cl::desc("Target features for triple"),
cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt OptLevel("opt-level",
@@ -114,9 +114,8 @@
cl::value_desc(" or ="),
cl::cat(ClangLinkerWrapperCategory));
 
-static cl::opt Verbose("v",
- cl::desc("Verbose output from tools"),
- 
+static cl::opt Verbose("v", cl::desc("Verbose output from tools"),
+
  cl::cat(ClangLinkerWrapperCategory));
 
 static cl::opt DebugInfo(
@@ -165,63 +164,43 @@
 /// different but it should work for what is passed here.
 static constexpr unsigned FatbinaryOffset = 0x50;
 
-/// Information for a device offloading file extracted from the host.
-struct DeviceFile {
-  DeviceFile(OffloadKind Kind, StringRef TheTriple, StringRef Arch,
- StringRef Filename)
-  : Kind(Kind), TheTriple(TheTriple), Arch(Arch), Filename(Filename) {}
+using OffloadingImage = OffloadBinary::OffloadingImage;
+
+/// A class to contain the binary information for a single OffloadBinary.
+class OffloadFile : public OwningBinary {
+public:
+  using TargetID = std::pair;
+
+  OffloadFile(std::unique_ptr Binary,
+  std::unique_ptr Buffer)
+  : OwningBinary(std::move(Binary), std::move(Buffer)) {}
 
-  OffloadKind Kind;
-  std::string TheTriple;
-  std::string Arch;
-  std::string Filename;
+  /// We use the Triple and Architecture pair to group linker inputs together.
+  /// This conversion function lets us use these files in a hash-map.
+  operator TargetID() const {
+return std::make_pair(getBinary()->getTriple(), getBinary()->getArch());
+  }
 };
 
 namespace llvm {
-/// Helper that allows DeviceFile to be used as a key in a DenseMap. For now we
-/// assume device files with matching architectures and triples but different
-/// offloading kinds should be handlded together, this may not be true in the
-/// future.
-
-// Provide DenseMapInfo for OffloadKind.
+// Provide DenseMapInfo so that OffloadKind can be used in a DenseMap.
 template <> struct DenseMapInfo {
   static inline OffloadKind getEmptyKey() { return OFK_LAST; }
   static inline OffloadKind getTombstoneKey() {
 return static_cast(OFK_LAST + 1);
   }
-  static unsigned getHashValue(const OffloadKind &Val) { return Val * 37U; }
+  static unsigned getHashValue(const OffloadKind &Val) { return Val; }
 
   static bool isEqual(const OffloadKind &LHS, const OffloadKind &RHS) {
 return LHS == RHS;
   }
 };
-template <> struct DenseMapInfo {
-  static DeviceFile getEmptyKey() {
-return {DenseMapInfo::getEmptyKey(),
-DenseMapInfo::getEmptyKey(),
-DenseMapInfo::getEmptyKey(),
-DenseMapInfo::getEmptyKey()};
-  }
-  static DeviceFile getTombstoneKey() {
-return {DenseMapInfo::getTombstoneKey(),
-DenseMapInfo::getTombstoneKey(),
-DenseMapInfo::getTombstoneKey(),
-DenseMapInfo::getTombstoneKey()};
-  }
-  static unsigned getHashValue(const DeviceFile &I) {
-return DenseMapInfo::getHashValue(I.TheTriple) ^
-   DenseMapInfo::getHashValue(I.Arch);
-  }
-  static bool isEqual(const DeviceFile &LHS, const DeviceFile &RHS) {
-return LHS.TheTriple == RHS.TheTriple && LHS.Arch == RHS.Arch;
-  }
-};
 } // namespace llvm
 
 namespace {
 
 Error extractFromBuffer(std::unique_ptr Buffer,
-SmallVectorImpl &DeviceFiles);
+SmallVectorImpl &DeviceFiles);
 
 void printCommands(ArrayRef CmdArgs) {
   if (CmdArgs.empty())
@@ -232,7 +211,7 @@
 llvm::errs() << *IC << (std::next(IC) != IE ? " " : "\n");
 }
 
-// Forward user requested arguments to the d

[PATCH] D127277: [clang][analyzer] Fix StdLibraryFunctionsChecker 'mkdir' return value.

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

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

Note in the summary that this patch affects:

- `mkdir`, `mkdirat`
- `mknod`, `mknodat`

> The checker modeled these functions with a >= 0 return value on success which 
> is incorrect.

Technically, it was correct previously. Now it's not only correct but also 
precise ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127277

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

[PATCH] D127285: [analyzer] Fix assertion failure after getKnownValue call

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

steakhal added a comment.

In D127285#3566198 , @martong wrote:

> In D127285#3566034 , @steakhal 
> wrote:
>
>> ... Try to mark named entities like `this`.
>
> Could you please elaborate?

Oh sure. The `SimpleSValBuilder` is not escaped like code or class entities 
should be in the summary. But it's really not that important.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127285

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

[PATCH] D127285: [analyzer] Fix assertion failure after getKnownValue call

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D127285#3566397 , @steakhal wrote:

> In D127285#3566198 , @martong wrote:
>
>> In D127285#3566034 , @steakhal 
>> wrote:
>>
>>> ... Try to mark named entities like `this`.
>>
>> Could you please elaborate?
>
> Oh sure. The `SimpleSValBuilder` is not escaped like code or class entities 
> should be in the summary. But it's really not that important.

Ok, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127285

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

[PATCH] D126779: [analyzer] Fix assertion in simplifySymbolCast

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

steakhal added a comment.

Oh I forgot to submit my review. My bad. Please investigate why we have two 
double casts there.




Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1082
+  // FIXME support cast from non-integers.
+  // E.g (char)(double)(double x) -> (char)(double x)
+  if (!RT->isIntegralOrEnumerationType())

Well, I would expect the same, but crosscheck this with the actual dump.



Comment at: clang/test/Analysis/produce-symbolcast_x86.cpp:16
+  double D = n / 30;
+  clang_analyzer_dump(D); // expected-warning{{(double) ((double) ((reg_$0) / 30))}}
+  char C = D;

Place here a FIXME that we should not have two double casts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126779

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

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet added a comment.

Pending D127279  before making sure that all 
`EmitTargetCodeForMemset` implementations are enforcing `AlwaysInline`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126903

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

[PATCH] D127304: [LinkerWrapper] Embed OffloadBinaries for OpenMP offloading images

[Joseph Huber via Phabricator via cfe-commits]

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

The OpenMP offloading runtine currently uses an array of linked
offloading images. One downside to this is that we cannot know the
architecture or triple associated with the given image. In this patch,
instead of embedding the image itself, we embed an offloading binary
instead. This binary is simply a binary format that wraps around the
original linked image to provide some additional metadata. This will
allow us to support offloading to multiple architecture, or performing
future JIT compilation inside of the runtime, more clearly.
Additionally, these can be placed at a special section such that the
supported architectures can be identified using objdump with the support
from D126904 . This needs to be stored in a 
new section name
`.llvm.offloading.images` because the `.llvm.offloading` section
implicitly uses the `SHF_EXCLUDE` flag and will always be stripped.

This patch does not contain the necessary code to parse these in
libomptarget.

Depends on D127246 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127304

Files:
  clang/test/Driver/linker-wrapper-image.c
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  clang/tools/clang-linker-wrapper/OffloadWrapper.cpp


Index: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/OffloadWrapper.cpp
+++ clang/tools/clang-linker-wrapper/OffloadWrapper.cpp
@@ -170,6 +170,7 @@
  GlobalVariable::InternalLinkage, Data,
  ".omp_offloading.device_image");
 Image->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
+Image->setSection(".llvm.offloading.images");
 
 auto *Size = ConstantInt::get(getSizeTTy(M), Buf.size());
 Constant *ZeroSize[] = {Zero, Size};
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -1119,8 +1119,7 @@
 bundleOpenMP(ArrayRef Images) {
   SmallVector> Buffers;
   for (const OffloadingImage &Image : Images)
-Buffers.emplace_back(
-MemoryBuffer::getMemBufferCopy(Image.Image->getBuffer()));
+Buffers.emplace_back(OffloadBinary::write(Image));
 
   return std::move(Buffers);
 }
Index: clang/test/Driver/linker-wrapper-image.c
===
--- clang/test/Driver/linker-wrapper-image.c
+++ clang/test/Driver/linker-wrapper-image.c
@@ -11,8 +11,8 @@
 // OPENMP: @__start_omp_offloading_entries = external hidden constant 
%__tgt_offload_entry
 // OPENMP-NEXT: @__stop_omp_offloading_entries = external hidden constant 
%__tgt_offload_entry
 // OPENMP-NEXT: @__dummy.omp_offloading.entry = hidden constant [0 x 
%__tgt_offload_entry] zeroinitializer, section "omp_offloading_entries"
-// OPENMP-NEXT: @.omp_offloading.device_image = internal unnamed_addr constant 
[0 x i8] zeroinitializer
-// OPENMP-NEXT: @.omp_offloading.device_images = internal unnamed_addr 
constant [1 x %__tgt_device_image] [%__tgt_device_image { ptr 
@.omp_offloading.device_image, ptr @.omp_offloading.device_image, ptr 
@__start_omp_offloading_entries, ptr @__stop_omp_offloading_entries }]
+// OPENMP-NEXT: @.omp_offloading.device_image = internal unnamed_addr constant 
[[[SIZE:[0-9]+]] x i8] c"\10\FF\10\AD{{.*}}"
+// OPENMP-NEXT: @.omp_offloading.device_images = internal unnamed_addr 
constant [1 x %__tgt_device_image] [%__tgt_device_image { ptr 
@.omp_offloading.device_image, ptr getelementptr inbounds ([[[SIZE]] x i8], ptr 
@.omp_offloading.device_image, i64 1, i64 0), ptr 
@__start_omp_offloading_entries, ptr @__stop_omp_offloading_entries }]
 // OPENMP-NEXT: @.omp_offloading.descriptor = internal constant 
%__tgt_bin_desc { i32 1, ptr @.omp_offloading.device_images, ptr 
@__start_omp_offloading_entries, ptr @__stop_omp_offloading_entries }
 // OPENMP-NEXT: @llvm.global_ctors = appending global [1 x { i32, ptr, ptr }] 
[{ i32, ptr, ptr } { i32 1, ptr @.omp_offloading.descriptor_reg, ptr null }]
 // OPENMP-NEXT: @llvm.global_dtors = appending global [1 x { i32, ptr, ptr }] 
[{ i32, ptr, ptr } { i32 1, ptr @.omp_offloading.descriptor_unreg, ptr null }]


Index: clang/tools/clang-linker-wrapper/OffloadWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/OffloadWrapper.cpp
+++ clang/tools/clang-linker-wrapper/OffloadWrapper.cpp
@@ -170,6 +170,7 @@
   

[PATCH] D126984: [clang] Add initial support for gcc's optimize function attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think this is getting close -- mostly just nits at this point.




Comment at: clang/include/clang/Basic/AttrDocs.td:3463
+The ``optimize`` attribute, when attached to a function, indicates that the
+function be compiled with a different optimization level than specified on the
+command line. See the Function Attributes documentation on GCC's docs for more





Comment at: clang/include/clang/Basic/AttrDocs.td:3465-3467
+information. Currently, the attribute differs from GCC's in that we only 
support
+one argument and we don't support "-f" arguments. We also don't support
+expressions and integers as arguments, unlike GCC.





Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1932-1934
+  if (const auto *OA = D->getAttr()) {
+HasOptimizeAttrO0 = OA->getOptLevel() == OptimizeAttr::O0;
+  }

Coding style nit.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4851
+
+  std::unordered_map StrToKind = {
+  {"", OptimizeAttr::O0},  {"s", OptimizeAttr::Os},

Given that most of the strings we're dealing with are either literals or a 
`StringRef`, I think you should use `llvm:: StringMap` instead of 
`unordered_map`.  (We tend to avoid STL containers because their performance 
characteristics are often different than what we need as a compiler.)



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4859
+
+  DEBUG_WITH_TYPE("foo", llvm::dbgs() << "Level: " << Level << "\n");
+

Probably meant to remove this?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4869
+  if (!Level.getAsInteger(10, Num)) {
+// Limit level to -O4 if higher
+std::string Level = std::to_string(Num.getLimitedValue(4));





Comment at: clang/lib/Sema/SemaDeclAttr.cpp:4841-4850
+  StringRef Level;
+  // Check if argument is prefixed with "-O" or "O"
+  if (Arg.str().rfind("-O", 0) == 0)
+Level = Arg.substr(2);
+  else if (Arg.str().rfind("O", 0) == 0)
+Level = Arg.substr(1);
+  else

steplong wrote:
> aaron.ballman wrote:
> > Then, in here, you can parse the `-O` the user passed as a 
> > string, and convert it to an `OptimizeAttr::OptLevelKind` enumerator and 
> > store that in the semantic attribute.
> > 
> > This allows you to map things like `-Og` to whatever -O level that actually 
> > represents, or do any other kind of mapping that works for you.
> > 
> > One question we should probably figure out is whether we also want to 
> > support clang-cl optimization strings or not. e.g., 
> > `__attribute__((optimize("/O0")))` with a slash instead of a dash. Since 
> > we're already going to be doing parsing from here anyway, I feel like it 
> > might not be a bad idea to also support those. FWIW, here's the list from 
> > the user's manual:
> > ```
> >   /O0 Disable optimization
> >   /O1 Optimize for size  (same as /Og /Os /Oy /Ob2 
> > /GF /Gy)
> >   /O2 Optimize for speed (same as /Og /Oi /Ot /Oy /Ob2 
> > /GF /Gy)
> >   /Ob0Disable function inlining
> >   /Ob1Only inline functions which are (explicitly or 
> > implicitly) marked inline
> >   /Ob2Inline functions as deemed beneficial by the 
> > compiler
> >   /Od Disable optimization
> >   /Og No effect
> >   /Oi-Disable use of builtin functions
> >   /Oi Enable use of builtin functions
> >   /Os Optimize for size
> >   /Ot Optimize for speed
> >   /Ox Deprecated (same as /Og /Oi /Ot /Oy /Ob2); use 
> > /O2 instead
> >   /Oy-Disable frame pointer omission (x86 only, default)
> >   /Oy Enable frame pointer omission (x86 only)
> >   /O   Set multiple /O flags at once; e.g. '/O2y-' for 
> > '/O2 /Oy-'
> > ```
> > (Not all of these would be supported, like enable use of builtin functions, 
> > etc.) WDYT?
> Hmm I don't think it's necessary to get pragma optimize to work, but it 
> shouldn't be hard to add the parsing logic for some of these strings
Definitely agreed on the MSVC command line switches, so if those are a burden, 
feel free to skip them. For the other flags, those seem more important to 
support because they're also flags present in GCC so users are more likely to 
expect them to work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126984

___
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 marked 11 inline comments as done.
erichkeane added inline comments.



Comment at: clang/include/clang/Sema/Template.h:221-226
+ArgListsIterator begin() { return TemplateArgumentLists.rbegin(); }
+ConstArgListsIterator begin() const {
+  return TemplateArgumentLists.rbegin();
+}
+ArgListsIterator end() { return TemplateArgumentLists.rend(); }
+ConstArgListsIterator end() const { return TemplateArgumentLists.rend(); }

ChuanqiXu wrote:
> The `begin()` and `end()` wrapper use `rbegin` and `rend` here. It is not 
> straight forward. Is it matter?
I was originally doing it for a good reason, though I don't recall it.  Going 
from 'rbegin' to 'rend' meant we'd be looking at the 'outermost' first (that is:

template struct S{
   template  void foo(){}
};

We'd be looking at the list containing 'T' first.  

However, for my use, it doesn't really matter (I just need to flatten the 
entire list for profile purposes), so I'll switch this to begin/end to be less 
surprising.



Comment at: clang/lib/Sema/SemaConcept.cpp:442-444
+}
+  } else if (FD->getTemplatedKind() == FunctionDecl::TK_MemberSpecialization ||
+ FD->getTemplatedKind() == FunctionDecl::TK_DependentNonTemplate) {

ChuanqiXu wrote:
> nit: This is not important but I prefer the style. It makes each logical 
> section shorter and indentation shorter.
I THINK i got this suggestion right?  Phab makes it not particularly clear.


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] D127306: [analyzer] Treat system globals as mutable if they are not const

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

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

Previously, system globals were treated as immutable regions, unless it
was the `errno` which is known to be frequently modified.

D124244  wants to add a check for stores to 
immutable regions.
It would basically turn all stores to system globals into an error even
though we have no reason to believe that those mutable sys globals
should be treated as if they were immutable. And this leads to
false-positives if we apply D124244 .

In this patch, I'm proposing to treat mutable sys globals actually
mutable, hence allocate them into the `GlobalSystemSpaceRegion`, UNLESS
they were declared as `const` (and a primitive arithmetic type), in
which case, we should use `GlobalImmutableSpaceRegion`.

In any other cases, I'm using the `GlobalInternalSpaceRegion`, which is
no different than the previous behavior.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127306

Files:
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/Analysis/Inputs/some_system_globals.h
  clang/test/Analysis/Inputs/some_user_globals.h
  clang/test/Analysis/global-region-invalidation.c
  clang/test/Analysis/globals-are-not-always-immutable.c

Index: clang/test/Analysis/globals-are-not-always-immutable.c
===
--- /dev/null
+++ clang/test/Analysis/globals-are-not-always-immutable.c
@@ -0,0 +1,56 @@
+// RUN: %clang_analyze_cc1 -verify %s \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-checker=core,debug.ExprInspection
+
+#include "Inputs/errno_var.h"
+#include "Inputs/some_system_globals.h"
+#include "Inputs/some_user_globals.h"
+
+void invalidate_globals(void);
+void clang_analyzer_eval(int x);
+
+/// Test the special system 'errno'
+void test_errno() {
+  errno = 2;
+  clang_analyzer_eval(errno == 2); // expected-warning {{TRUE}}
+  invalidate_globals();
+  clang_analyzer_eval(errno == 2); // expected-warning {{UNKNOWN}}
+}
+
+/// Test user global variables
+void test_my_const_user_global() {
+  if (my_const_user_global != 2)
+return;
+
+  clang_analyzer_eval(my_const_user_global == 2); // expected-warning {{TRUE}}
+  invalidate_globals();
+  clang_analyzer_eval(my_const_user_global == 2); // expected-warning {{TRUE}}
+}
+
+void test_my_mutable_user_global() {
+  if (my_mutable_user_global != 2)
+return;
+
+  clang_analyzer_eval(my_mutable_user_global == 2); // expected-warning {{TRUE}}
+  invalidate_globals();
+  clang_analyzer_eval(my_mutable_user_global == 2); // expected-warning {{UNKNOWN}}
+}
+
+/// Test system global variables
+void test_my_const_system_global() {
+  if (my_const_system_global != 2)
+return;
+
+  clang_analyzer_eval(my_const_system_global == 2); // expected-warning {{TRUE}}
+  invalidate_globals();
+  clang_analyzer_eval(my_const_system_global == 2); // expected-warning {{TRUE}}
+}
+
+void test_my_mutable_system_global() {
+  if (my_mutable_system_global != 2)
+return;
+
+  clang_analyzer_eval(my_mutable_system_global == 2); // expected-warning {{TRUE}}
+  invalidate_globals();
+  clang_analyzer_eval(my_mutable_system_global == 2); // expected-warning {{UNKNOWN}} It was previously TRUE.
+}
Index: clang/test/Analysis/global-region-invalidation.c
===
--- clang/test/Analysis/global-region-invalidation.c
+++ clang/test/Analysis/global-region-invalidation.c
@@ -28,11 +28,17 @@
 void foo(void);
 int stdinTest(void) {
   int i = 0;
-  fscanf(stdin, "%d", &i);
+  fscanf(stdin, "%d", &i); // (1)
   foo();
   int m = i; // expected-warning + {{tainted}}
-  fscanf(stdin, "%d", &i);
-  int j = i; // expected-warning + {{tainted}}
+  fscanf(stdin, "%d", &i); // (2)
+
+  // The 'stdin' mutable system global variable gets invalidated by the first
+  // (1) opaque 'fscanf()' call. Consequently, the subsequent
+  // (2) 'fscanf(stdin, ...)' call won't propagate taint.
+  // FIXME: The analyzer engine should propagate taint by default on
+  // invalidation.
+  int j = i; // FIXME: should have 'tainted' warning here.
   return m + j; // expected-warning + {{tainted}}
 }
 
Index: clang/test/Analysis/Inputs/some_user_globals.h
===
--- /dev/null
+++ clang/test/Analysis/Inputs/some_user_globals.h
@@ -0,0 +1,2 @@
+extern const int my_const_user_global;
+extern int my_mutable_user_global;
Index: clang/test/Analysis/Inputs/some_system_globals.h
===
--- /dev/null
+++ clang/te

[PATCH] D126461: [RISCV] Extract and store new vl of vleff iff destination isn't null

[Zakk Chen via Phabricator via cfe-commits]

khchen added a comment.

IMO, if I'm an user, I would not expected intrinsic function will generate the 
condition code to impact the performance, maybe we need to raise a issue in 
rvv-intrinsic-doc.

maybe another way is adding a note in intrinsic document to address that the vl 
could not be a null pointer.

How about the segment load? Does it make sense to add null pointer checking for 
all argument v0~vN?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126461

___
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 updated this revision to Diff 435148.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.

All of the comments from @ChuanqiXu done, thank you so much!

As far as the crash you're seeing: That is typically a 'depth mismatch' kinda 
thing.  They sadly only show up when the types of the template arguments are 
different (that is, pack vs integral vs type), but I thought I got all of them 
with examples.

I'll keep hunting for more during the day today, but any amount of reproducer 
you can provide (even if not minimized) would be incredibly helpful!


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

https://reviews.llvm.org/D126907

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CXX/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
  clang/test/SemaTemplate/concepts-friends.cpp
  clang/test/SemaTemplate/concepts.cpp
  clang/test/SemaTemplate/deferred-concept-inst.cpp
  clang/test/SemaTemplate/instantiate-requires-clause.cpp
  clang/test/SemaTemplate/trailing-return-short-circuit.cpp

Index: clang/test/SemaTemplate/trailing-return-short-circuit.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/trailing-return-short-circuit.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+template 
+  requires(sizeof(T) > 2) || T::value // #FOO_REQ
+void Foo(T){};// #FOO
+
+template 
+void TrailingReturn(T)   // #TRAILING
+  requires(sizeof(T) > 2) || // #TRAILING_REQ
+  T::value   // #TRAILING_REQ_VAL
+{};
+template 
+struct HasValue {
+  static constexpr bool value = B;
+};
+static_assert(sizeof(HasValue) <= 2);
+
+template 
+struct HasValueLarge {
+  static constexpr bool value = B;
+  int I;
+};
+static_assert(sizeof(HasValueLarge) > 2);
+
+void usage() {
+  // Passes the 1st check, short-circuit so the 2nd ::value is not evaluated.
+  Foo(1.0);
+  TrailingReturn(1.0);
+
+  // Fails the 1st check, but has a ::value, so the check happens correctly.
+  Foo(HasValue{});
+  TrailingReturn(HasValue{});
+
+  // Passes the 1st check, but would have passed the 2nd one.
+  Foo(HasValueLarge{});
+  TrailingReturn(HasValueLarge{});
+
+  // Fails the 1st check, fails 2nd because there is no ::value.
+  Foo(true);
+  // expected-error@-1{{no matching function for call to 'Foo'}}
+  // expected-note@#FOO{{candidate template ignored: constraints not satisfied [with T = bool]}}
+  // expected-note@#FOO_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#FOO_REQ{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
+
+  TrailingReturn(true);
+  // expected-error@-1{{no matching function for call to 'TrailingReturn'}}
+  // expected-note@#TRAILING{{candidate template ignored: constraints not satisfied [with T = bool]}}
+  // expected-note@#TRAILING_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#TRAILING_REQ_VAL{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
+
+  // Fails the 1st check, fails 2nd because ::value is false.
+  Foo(HasValue{});
+  // expected-error@-1 {{no matching function for call to 'Foo'}}
+  // expected-note@#FOO{{candidate template ignored: constraints not satisfied [with T = HasValue]}}
+  // expected-note@#FOO_REQ{{because 'sizeof(HasValue) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#FOO_REQ{{and 'HasValue::value' evaluated to false}}
+  TrailingReturn(HasValue{});
+  // expected-error@-1 {{no matching function for call to 'TrailingReturn'}}
+  // expected-note@#TRAILING{{candidate template ignored: constraints not satisfied [with T = HasValue]}}
+  // expected-note@#TRAILING_REQ{{because 'sizeof(HasValue) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#TRAILING_REQ_VAL{{and 'HasValue::value' evaluated to false}}
+}
Index: clang/test/SemaTemplate/instantiate-requires-clause.cpp
===
--- clang/test/SemaTemplate/instantiate-requires-clause.cpp
+++ clang/test/SemaTemplate/instantiate-requires-clause.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -x c++ %s -verify
+// RUN: %clang_cc1 -std=c++2a -x c++ %s -Wno-unused-

[PATCH] D124244: [analyzer] add StoreToImmutable and ModelConstQualifiedReturn checkers

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

steakhal added a comment.

In D124244#3521728 , @steakhal wrote:

> I found this report at `vim/src/term.c` (https://github.com/vim/vim.git at 
> `v8.2.1920`):
> F23102904: image.png 
>
> I'm not sure what the exact type of `BC` global variable is but I think it's 
> declared as either `extern char *BC;` or just `char *BC;`.
> `empty_option` is declared as either `extern unsigned char *empty_option;` or 
> `unsigned char *empty_option = (unsigned char *)"";`
>
> Could you please have a look at this FP @zukatsinadze?

I've checked why we report this. It turns out mutable global variables were 
considered //immutable// (except the `errno`), which caused FPs in this 
proposed checker.
I made D127306  to keep mutable (system) 
globals as //mutable// memory.


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

https://reviews.llvm.org/D124244

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

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:616
+setOperationAction(ISD::FROUNDEVEN, MVT::f16, Promote);
+setOperationAction(ISD::FP_ROUND, MVT::f16, Expand);
+setOperationAction(ISD::FP_EXTEND, MVT::f32, Expand);

LuoYuanke wrote:
> Just confused how to expand it. Will the expand fail and finally turns to 
> libcall?
Yeah, we can use `LibCall` instead.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:763
+if (isTypeLegal(MVT::f16)) {
+  setOperationAction(ISD::FP_EXTEND, MVT::f80, Custom);
+  setOperationAction(ISD::STRICT_FP_EXTEND, MVT::f80, Custom);

LuoYuanke wrote:
> Why f16 emulation affect f80 type? Are we checking isTypeLegal(MVT::f80)?
It's in the scope of `if (UseX87)`. And we need to lower `fpext half %0 to 
x86_fp80`.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22448
+  if (SrcVT == MVT::f16)
+return SDValue();
+

LuoYuanke wrote:
> Why we don't extent to f32 here?
Return `SDValue()` will extent later. This can save the code.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22522
+  if (!isScalarFPTypeInSSEReg(SrcVT) ||
+  (SrcVT == MVT::f16 && !Subtarget.hasFP16()))
 return SDValue();

LuoYuanke wrote:
> Why we don't extent to f32 here? Will it be promoted finally?
Yes.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22765
+DAG.getIntPtrConstant(0, DL));
+  Res = DAG.getNode(X86ISD::STRICT_CVTPS2PH, DL, {MVT::v8i16, MVT::Other},
+{Chain, Res, DAG.getTargetConstant(4, DL, MVT::i32)});

LuoYuanke wrote:
> Should MVT::v8i16 be MVT::v8f16?
No. We use `MVT::v8i16` when we enabled F16C instructions.



Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:22775
+
+Res = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, MVT::i16, Res,
+  DAG.getIntPtrConstant(0, DL));

LuoYuanke wrote:
> MVT::f16 and delete the bitcast?
I don't think we have pattern to extract `f16` from `v8i16`. Besides, I think 
keeping the bitcast makes the flow clear.



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:1476
 }
-let Predicates = [HasFP16] in {
+let Predicates = [HasBWI] in {
   def : Pat<(v32f16 (X86VBroadcastld16 addr:$src)),

LuoYuanke wrote:
> If target don't have avx512bw feature. There is some other pattern to lower 
> the node or fp16 broadcast node is invalid?
Good catch. Added in X86InstrSSE.td



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:4107
  _.ExeDomain>, EVEX_4V, Sched<[SchedWriteFShuffle.XMM]>;
+  let Predicates = [prd] in {
   def rrkz : AVX512PI<0x10, MRMSrcReg, (outs _.RC:$dst),

LuoYuanke wrote:
> Previous prd only apply to "def rr"? Is it a bug for previous code?
No. previous code works well because no mask variants before AVX512 and no f16 
before FP16. The latter is not true now.



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:4352
+defm : avx512_move_scalar_lowering<"VMOVSHZ", X86Movsh, fp16imm0, v8f16x_info>;
+defm : avx512_store_scalar_lowering<"VMOVSHZ", avx512vl_f16_info,
+   (v32i1 (bitconvert (and GR32:$mask, (i32 1, GR32>;

LuoYuanke wrote:
> Why previous code don't have predicates?
Because no legal `f16` previously.



Comment at: llvm/lib/Target/X86/X86InstrAVX512.td:11657
 
+let Predicates = [HasBWI], AddedComplexity = -10 in {
+  def : Pat<(f16 (load addr:$src)), (COPY_TO_REGCLASS (VPINSRWZrm (v8i16 
(IMPLICIT_DEF)), addr:$src, 0), FR16X)>;

LuoYuanke wrote:
> Why set AddedComplexity to -10? There no such addtional complexity in 
> previous code. Add comments for it? 
We used it before, but very little. We need to make sure select FP16 
instructions first if available.



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:3970
 
+let Predicates = [UseSSE2], AddedComplexity = -10 in {
+  def : Pat<(f16 (load addr:$src)), (COPY_TO_REGCLASS (PINSRWrm (v8i16 
(IMPLICIT_DEF)), addr:$src, 0), FR16)>;

LuoYuanke wrote:
> Why  AddedComplexity = -10? Add comments for it?
This is to avoid FP16 instructions been overridden.



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:3978
+let Predicates = [HasAVX, NoBWI], AddedComplexity = -10 in {
+  def : Pat<(f16 (load addr:$src)), (COPY_TO_REGCLASS (VPINSRWrm (v8i16 
(IMPLICIT_DEF)), addr:$src, 0), FR16)>;
+  def : Pat<(i16 (bitconvert f16:$src)), (EXTRACT_SUBREG (VPEXTRWrr (v8i16 
(COPY_TO_REGCLASS FR16:$src, VR128)), 0), sub_16bit)>;

LuoYuanke wrote:
> Miss pattern for store?
It's in line 5214.



Comment at: llvm/lib/Target/X86/X86InstrSSE.td:5214
+
+let Predicates = [HasAVX, NoBWI] in
+  def : Pat<(store f16:$src, addr:$dst), (VPEXTRWmr ad

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 435151.
pengfei marked 3 inline comments as done.
pengfei added a comment.

Address Yuanke's comments. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107082

Files:
  llvm/docs/ReleaseNotes.rst
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/lib/Target/X86/X86ISelLowering.cpp
  llvm/lib/Target/X86/X86ISelLowering.h
  llvm/lib/Target/X86/X86InstrAVX512.td
  llvm/lib/Target/X86/X86InstrCompiler.td
  llvm/lib/Target/X86/X86InstrInfo.cpp
  llvm/lib/Target/X86/X86InstrSSE.td
  llvm/lib/Target/X86/X86InstrVecCompiler.td
  llvm/lib/Target/X86/X86InstructionSelector.cpp
  llvm/lib/Target/X86/X86RegisterInfo.td
  llvm/test/Analysis/CostModel/X86/fptoi_sat.ll
  llvm/test/CodeGen/MIR/X86/inline-asm-registers.mir
  llvm/test/CodeGen/X86/atomic-non-integer.ll
  llvm/test/CodeGen/X86/avx512-insert-extract.ll
  llvm/test/CodeGen/X86/avx512-masked_memop-16-8.ll
  llvm/test/CodeGen/X86/avx512fp16-fp-logic.ll
  llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll
  llvm/test/CodeGen/X86/cvt16-2.ll
  llvm/test/CodeGen/X86/cvt16.ll
  llvm/test/CodeGen/X86/fastmath-float-half-conversion.ll
  llvm/test/CodeGen/X86/fmf-flags.ll
  llvm/test/CodeGen/X86/fp-round.ll
  llvm/test/CodeGen/X86/fp-roundeven.ll
  llvm/test/CodeGen/X86/fp128-cast-strict.ll
  llvm/test/CodeGen/X86/fpclamptosat.ll
  llvm/test/CodeGen/X86/fpclamptosat_vec.ll
  llvm/test/CodeGen/X86/fptosi-sat-scalar.ll
  llvm/test/CodeGen/X86/fptosi-sat-vector-128.ll
  llvm/test/CodeGen/X86/fptoui-sat-scalar.ll
  llvm/test/CodeGen/X86/fptoui-sat-vector-128.ll
  llvm/test/CodeGen/X86/freeze.ll
  llvm/test/CodeGen/X86/half-constrained.ll
  llvm/test/CodeGen/X86/half.ll
  llvm/test/CodeGen/X86/pr31088.ll
  llvm/test/CodeGen/X86/pr38533.ll
  llvm/test/CodeGen/X86/pr47000.ll
  llvm/test/CodeGen/X86/scheduler-asm-moves.mir
  llvm/test/CodeGen/X86/shuffle-extract-subvector.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16-fma.ll
  llvm/test/CodeGen/X86/stack-folding-fp-avx512fp16.ll
  llvm/test/CodeGen/X86/statepoint-invoke-ra-enter-at-end.mir
  llvm/test/CodeGen/X86/vec_fp_to_int.ll
  llvm/test/CodeGen/X86/vector-half-conversions.ll
  llvm/test/CodeGen/X86/vector-reduce-fmax-nnan.ll
  llvm/test/CodeGen/X86/vector-reduce-fmin-nnan.ll
  llvm/test/MC/X86/x86_64-asm-match.s

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

[PATCH] D125677: [pseudo] Remove the explicit Accept actions.

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:85
 
-  if (!PendingAccept.empty()) {
-LLVM_DEBUG({
-  llvm::dbgs() << llvm::formatv("Accept: {0} accepted result:\n",
- PendingAccept.size());
-  for (const auto &Accept : PendingAccept)
-llvm::dbgs() << "  - " << G.symbolName(Accept.Head->Payload->symbol())
- << "\n";
-});
-assert(PendingAccept.size() == 1);
-return *PendingAccept.front().Head->Payload;
-  }
+  const ForestNode *Result = nullptr;
+  StateID AcceptState = Params.Table.getGoToState(StartState, StartSymbol);

hokein wrote:
> sammccall wrote:
> > rather than mingling this with the glrReduce, I'd suggest collecting the 
> > set of final heads first and then analyzing them afterwards.
> > 
> > This means looping a second time, but I think the logic around recognizing 
> > patterns that look like `accept` might grow (e.g. if we want to incorporate 
> > some error tolerance)
> that sounds reasonable, but it requires some additional changes (to identify 
> inactive heads in the callback of `glrReduce`):
> 
> - AddSteps now returns true/false whether there is any actions in 
> LRTable[NewHead->state][nextTok];
> - no available action in the acceptable state on the eof token in LRTable 
> (see the change in lr-build-basic.test);
Hmm, I'm not sure whether the conditional or unconditional version is better, 
and we're really borrowing problems from the future here.

Let's go back to this simplest version, and update the opaque forest node 
comment below to a FIXME.
(We will need to invoke our generic error-recovery handlers when we reach EOF 
without reaching accept state, and simulating shifting an eof token may be the 
best way to reuse the code)



Comment at: clang-tools-extra/pseudo/lib/GLR.cpp:86
+  const ForestNode *Result = nullptr;
+  StateID AcceptState = Params.Table.getGoToState(StartState, StartSymbol);
+  glrReduce(PendingReduce, Params, [&](const GSS::Node *NewHead) {

this line introduces a requirement that the start symbol be a nonterminal - if 
this is new, can we doc it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125677

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

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h:29
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+

Please add `isLanguageVersionSupported`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-const-or-ref-data-members.rst:10
+
+The check implements
+`C.12 of C++ Core Guidelines 
`_.

Usually such links are placed at the end.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

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

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 435152.
martong added a comment.

- Rebase to llvm/main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117229

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/produce-ptr-to-integer-symbolcast.cpp
  clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp

Index: clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-config support-symbolic-integer-casts=true \
+// RUN:   -verify
+
+template 
+void clang_analyzer_dump(T);
+
+void clang_analyzer_eval(bool);
+
+void test_memory_region_to_integer_cast_and_symbol_simplification1(int *p) {
+  long p_as_integer = (long)p;
+  clang_analyzer_dump(p_as_integer);  // expected-warning{{(long) (reg_$0)}}
+  if (42 - p_as_integer < 42)
+return;
+  // 42 - p_as_integer >= 42
+  // p_as_integer <= 0
+
+  if (p)
+return;
+  clang_analyzer_eval(p == 0);// expected-warning{{TRUE}}
+  clang_analyzer_eval(p_as_integer == 0); // expected-warning{{TRUE}}
+
+  (void)p_as_integer;
+  (void)p;
+}
+
+void test_memory_region_to_integer_cast_and_symbol_simplification2(int *p) {
+  long p_as_integer = (long)p;
+  if (42 - p_as_integer < 42)
+return;
+  // 42 - p_as_integer >= 42
+  // p_as_integer <= 0
+
+  if (p_as_integer)
+return;
+  clang_analyzer_eval(p == 0);// expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(p_as_integer == 0); // expected-warning{{TRUE}}
+
+  (void)p_as_integer;
+  (void)p;
+}
Index: clang/test/Analysis/produce-ptr-to-integer-symbolcast.cpp
===
--- /dev/null
+++ clang/test/Analysis/produce-ptr-to-integer-symbolcast.cpp
@@ -0,0 +1,31 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-config support-symbolic-integer-casts=true \
+// RUN:   -triple x86_64-pc-linux-gnu \
+// RUN:   -verify
+
+template 
+void clang_analyzer_dump(T);
+
+void test_memory_region_to_integer_cast(int *p) {
+  long p_int = (long)p;
+  clang_analyzer_dump(p); // expected-warning{{&SymRegion{reg_$0}}}
+  clang_analyzer_dump(p_int); // expected-warning{{(long) (reg_$0)}}
+}
+
+void test_label_to_integer_cast(bool coin) {
+  int x = 0;
+p: ++x;
+  if (coin)
+goto p;
+
+  // FIXME, below we should produce a SymbolCast instead of the LocAsInteger.
+  // However, for that we'd need to be able to store a loc::GotoLabel SVal
+  // inside the SymbolCast. But currently we can put only another SymExpr
+  // inside a SymbolCast.
+
+  // Use of GNU address-of-label extension.
+  clang_analyzer_dump((long)&&p); // expected-warning{{&&p [as 64 bit integer]}}
+}
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -795,6 +795,16 @@
   //QualType elemTy = cast(originalTy)->getElementType();
   //QualType pointerTy = C.getPointerType(elemTy);
 }
+
+AnalyzerOptions &Opts =
+StateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions();
+if (Opts.ShouldSupportSymbolicIntegerCasts) {
+  const MemRegion *R = V.getRegion();
+  if (R && !OriginalTy.isNull())
+if (const auto *SR = dyn_cast(R))
+  return simplifySymbolCast(SR->getSymbol(), CastTy);
+}
+
 const unsigned BitWidth = Context.getIntWidth(CastTy);
 return makeLocAsInteger(Val.castAs(), BitWidth);
   }
@@ -1031,8 +1041,12 @@
   return V;
 }
 
-nonloc::SymbolVal SValBuilder::simplifySymbolCast(nonloc::SymbolVal V,
+nonloc::SymbolVal SValBuilder::simplifySymbolCast(SymbolRef SE,
   QualType CastTy) {
+  QualType T = Context.getCanonicalType(SE->getType());
+  if (!isa(SE))
+return makeNonLoc(SE, T, CastTy);
+
   // We use seven conditions to recognize a simplification case.
   // For the clarity let `CastTy` be `C`, SE->getType() - `T`, root type - `R`,
   // prefix `u` for unsigned, `s` for signed, no prefix - any sign:
@@ -1066,15 +1080,6 @@
   //  (uint)(ushort)(ushort x) -> (uint)(ushort x)
   //  (llong)(ulong)(uint x) -> (llong)(uint x) (sizeof(ulong) == sizeof(uint))
 
-  SymbolRef SE = V.getSymbol();
-  QualType T = Context.getCanonicalType(SE->getType());
-
-  if (T == CastTy)
-return V

[PATCH] D126291: [flang][Driver] Update link job on windows

[Diana Picus via Phabricator via cfe-commits]

rovka updated this revision to Diff 435150.
rovka edited the summary of this revision.
rovka added a comment.

- Fixed test
- Unconditionally added the subsystem arg
- Incorporated the MinGW toolchain changes (Thanks again @mmuetzel, I'm adding 
you as co-author)

@awarzynski I agree that the other patch (replacing ) should be 
committed separately. @mmuetzel are you going to upload it to Phab? I gave it a 
try on my machine and I can't compile the runtime with it, but I'll try to fix 
it on my end and we can continue the discussion in a new revision.


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

https://reviews.llvm.org/D126291

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  flang/test/Driver/linker-flags-windows.f90
  flang/test/Driver/linker-flags.f90

Index: flang/test/Driver/linker-flags.f90
===
--- flang/test/Driver/linker-flags.f90
+++ flang/test/Driver/linker-flags.f90
@@ -5,8 +5,8 @@
 ! NOTE: The additional linker flags tested here are currently only specified for
 ! GNU and Darwin. The following line will make sure that this test is skipped on
 ! Windows. If you are running this test on a yet another platform and it is
-! failing for you, please either update the following or (preferably) update the
-! linker invocation accordingly.
+! failing for you, please either update the following or (preferably)
+! create a similar test for your platform.
 ! UNSUPPORTED: system-windows
 
 !
Index: flang/test/Driver/linker-flags-windows.f90
===
--- /dev/null
+++ flang/test/Driver/linker-flags-windows.f90
@@ -0,0 +1,25 @@
+! Verify that the Fortran runtime libraries are present in the linker
+! invocation. These libraries are added on top of other standard runtime
+! libraries that the Clang driver will include.
+
+! NOTE: The additional linker flags tested here are currently specified in
+! clang/lib/Driver/Toolchains/MSVC.cpp.
+! REQUIRES: windows-msvc
+
+! RUN: %flang -### %S/Inputs/hello.f90 2>&1 | FileCheck %s
+
+! Compiler invocation to generate the object file
+! CHECK-LABEL: {{.*}} "-emit-obj"
+! CHECK-SAME:  "-o" "[[object_file:.*]]" {{.*}}Inputs/hello.f90
+
+! Linker invocation to generate the executable
+! NOTE: This check should also match if the default linker is lld-link.exe
+! CHECK-LABEL: link.exe
+! CHECK-NOT: libcmt
+! CHECK-NOT: oldnames
+! CHECK-SAME: Fortran_main.lib
+! CHECK-SAME: FortranRuntime.lib
+! CHECK-SAME: FortranDecimal.lib
+! CHECK-SAME: /subsystem:console
+! CHECK-SAME: "[[object_file]]"
+
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -218,6 +218,11 @@
 
   AddLinkerInputs(TC, Inputs, Args, CmdArgs, JA);
 
+  if (C.getDriver().IsFlangMode()) {
+tools::addFortranRuntimeLibraryPath(TC, Args, CmdArgs);
+tools::addFortranRuntimeLibs(TC, CmdArgs);
+  }
+
   // TODO: Add profile stuff here
 
   if (TC.ShouldLinkCXXStdlib(Args)) {
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -81,7 +81,7 @@
 Args.MakeArgString(std::string("-out:") + Output.getFilename()));
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles) &&
-  !C.getDriver().IsCLMode()) {
+  !C.getDriver().IsCLMode() && !C.getDriver().IsFlangMode()) {
 CmdArgs.push_back("-defaultlib:libcmt");
 CmdArgs.push_back("-defaultlib:oldnames");
   }
@@ -130,6 +130,16 @@
   Args.MakeArgString(std::string("-libpath:") + WindowsSdkLibPath));
   }
 
+  if (C.getDriver().IsFlangMode()) {
+tools::addFortranRuntimeLibraryPath(TC, Args, CmdArgs);
+tools::addFortranRuntimeLibs(TC, CmdArgs);
+
+// Inform the MSVC linker that we're generating a console application, i.e.
+// one with `main` as the "user-defined" entry point. The `main` function is
+// defined in flang's runtime libraries.
+CmdArgs.push_back("/subsystem:console");
+  }
+
   // Add the compiler-rt library directories to libpath if they exist to help
   // the linker find the various sanitizer, builtin, and profiling runtimes.
   for (const auto &LibPath : TC.getLibraryPaths()) {
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -599,7 +599,7 @@
   // TODO: Make this work unconditionally once Flang is mature enough.
   if (D.IsFlangMode() && Args.hasArg(options::OPT_flang_experimental_exec)) {
 a

[PATCH] D126796: [clang][driver] adds `-print-diagnostics`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thanks for this! Just some fairly minor nits from me. FWIW, the changes should 
also come with a release note so users know about the new command line option.




Comment at: clang/include/clang/Driver/Options.td:829
 def Xoffload_linker : JoinedAndSeparate<["-"], "Xoffload-linker">,
-  HelpText<"Pass  to the offload linkers or the ones idenfied by 
-">, 
+  HelpText<"Pass  to the offload linkers or the ones idenfied by 
-">,
   MetaVarName<" ">, Group;

Unintended whitespace change?



Comment at: clang/lib/Basic/DiagnosticIDs.cpp:656
 std::vector DiagnosticIDs::getDiagnosticFlags() {
-  std::vector Res;
+  std::vector Res{"-W", "-Wno-"};
   for (size_t I = 1; DiagGroupNames[I] != '\0';) {

I presume this change doesn't have any negative effects in 
`Driver::HandleAutocompletions()`? (That's currently the only consumer of this 
API.)



Comment at: clang/lib/Driver/Driver.cpp:2010
+  if (C.getArgs().hasArg(options::OPT_print_diagnostic_options)) {
+std::vector Flags = DiagnosticIDs::getDiagnosticFlags();
+for (std::size_t I = 0; I != Flags.size(); I += 2)

Declare the type as a const reference?

(Btw, when uploading patches, you should give them plenty of patch context -- 
that's what allows reviewers to suggest changes directly in Phab, but it also 
gives more information to reviewers so they don't have to leave the review to 
go to a code editor. FWIW, I usually use `-U999` to add context.)



Comment at: clang/test/Driver/print-diagnostic-options.c:1
+// Test that -print-diagnostic-options prints all warning groups
+

"All" warning groups? ;-) Might want to update the comment somewhat.


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

https://reviews.llvm.org/D126796

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

[PATCH] D127285: [analyzer] Fix assertion failure after getKnownValue call

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 435154.
martong marked an inline comment as done.
martong added a comment.

- Add one more sentence in the comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127285

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/svalbuilder-simplify-no-crash.c

Index: clang/test/Analysis/svalbuilder-simplify-no-crash.c
===
--- /dev/null
+++ clang/test/Analysis/svalbuilder-simplify-no-crash.c
@@ -0,0 +1,13 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -verify
+
+// Here, we test that svalbuilder simplification does not produce any
+// assertion failure.
+
+void crashing(long a, _Bool b) {
+  (void)(a & 1 && 0);
+  b = a & 1;
+  (void)(b << 1); // expected-warning{{core.UndefinedBinaryOperatorResult}}
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -22,6 +22,13 @@
 namespace {
 class SimpleSValBuilder : public SValBuilder {
 
+  // Query the constraint manager whether the SVal has only one possible
+  // (integer) value. If that is the case, the value is returned. Otherwise,
+  // returns NULL.
+  // This is an implementation detail. Checkers should use `getKnownValue()`
+  // instead.
+  const llvm::APSInt *getConstValue(ProgramStateRef state, SVal V);
+
   // With one `simplifySValOnce` call, a compound symbols might collapse to
   // simpler symbol tree that is still possible to further simplify. Thus, we
   // do the simplification on a new symbol tree until we reach the simplest
@@ -45,7 +52,7 @@
   SVal simplifyUntilFixpoint(ProgramStateRef State, SVal Val);
 
   // Recursively descends into symbolic expressions and replaces symbols
-  // with their known values (in the sense of the getKnownValue() method).
+  // with their known values (in the sense of the getConstValue() method).
   // We traverse the symbol tree and query the constraint values for the
   // sub-trees and if a value is a constant we do the constant folding.
   SVal simplifySValOnce(ProgramStateRef State, SVal V);
@@ -65,8 +72,9 @@
   SVal evalBinOpLN(ProgramStateRef state, BinaryOperator::Opcode op,
Loc lhs, NonLoc rhs, QualType resultTy) override;
 
-  /// getKnownValue - evaluates a given SVal. If the SVal has only one possible
-  ///  (integer) value, that value is returned. Otherwise, returns NULL.
+  /// Evaluates a given SVal by recursively evaluating and
+  /// simplifying the children SVals. If the SVal has only one possible
+  /// (integer) value, that value is returned. Otherwise, returns NULL.
   const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal V) override;
 
   SVal simplifySVal(ProgramStateRef State, SVal V) override;
@@ -532,7 +540,7 @@
   llvm::APSInt LHSValue = lhs.castAs().getValue();
 
   // If we're dealing with two known constants, just perform the operation.
-  if (const llvm::APSInt *KnownRHSValue = getKnownValue(state, rhs)) {
+  if (const llvm::APSInt *KnownRHSValue = getConstValue(state, rhs)) {
 llvm::APSInt RHSValue = *KnownRHSValue;
 if (BinaryOperator::isComparisonOp(op)) {
   // We're looking for a type big enough to compare the two values.
@@ -652,7 +660,7 @@
 }
 
 // For now, only handle expressions whose RHS is a constant.
-if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs)) {
+if (const llvm::APSInt *RHSValue = getConstValue(state, rhs)) {
   // If both the LHS and the current expression are additive,
   // fold their constants and try again.
   if (BinaryOperator::isAdditiveOp(op)) {
@@ -699,7 +707,7 @@
   }
 
   // Is the RHS a constant?
-  if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs))
+  if (const llvm::APSInt *RHSValue = getConstValue(state, rhs))
 return MakeSymIntVal(Sym, op, *RHSValue, resultTy);
 
   if (Optional V = tryRearrange(state, op, lhs, rhs, resultTy))
@@ -1187,8 +1195,8 @@
   return UnknownVal();
 }
 
-const llvm::APSInt *SimpleSValBuilder::getKnownValue(ProgramStateRef state,
-   SVal V) {
+const llvm::APSInt *SimpleSValBuilder::getConstValue(ProgramStateRef state,
+ SVal V) {
   if (V.isUnknownOrUndef())
 return nullptr;
 
@@ -1204,6 +1212,11 @@
   return nullptr;
 }
 
+const llvm::APSInt *SimpleSValBuilder::getKnownValue(ProgramStateRef state,
+ SVal V) {
+  return getConstValue(state, simplifySVal(state, V));
+}
+
 SVal SimpleSValBuilder::simplifyUntilFix

[PATCH] D127285: [analyzer] Fix assertion failure after getKnownValue call

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:25-28
+  // Query the constraint manager whether the SVal has only one possible
+  // (integer) value. If that is the case, the value is returned. Otherwise,
+  // returns NULL.
+  const llvm::APSInt *getConstValue(ProgramStateRef state, SVal V);

steakhal wrote:
> I'm also wondering if this should be at the top. I would rather put it below 
> `simplifySValOnce()`.
It is a good idea to draw the checker writer's attention to use 
`getKnownValue`, however, this is a `private` function deliberately for this 
exact reason.

> I'm also wondering if this should be at the top. I would rather put it below 
> simplifySValOnce().
I've put it here because simplifySValOnce references this even in its header 
comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127285

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

[PATCH] D127285: [analyzer] Fix assertion failure after getKnownValue call

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

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



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:25-28
+  // Query the constraint manager whether the SVal has only one possible
+  // (integer) value. If that is the case, the value is returned. Otherwise,
+  // returns NULL.
+  const llvm::APSInt *getConstValue(ProgramStateRef state, SVal V);

martong wrote:
> steakhal wrote:
> > I'm also wondering if this should be at the top. I would rather put it 
> > below `simplifySValOnce()`.
> It is a good idea to draw the checker writer's attention to use 
> `getKnownValue`, however, this is a `private` function deliberately for this 
> exact reason.
> 
> > I'm also wondering if this should be at the top. I would rather put it 
> > below simplifySValOnce().
> I've put it here because simplifySValOnce references this even in its header 
> comment.
> It is a good idea to draw the checker writer's attention to use 
> getKnownValue, however, this is a private function deliberately for this 
> exact reason.
I know. My point is that the name of those two functions won't express the 
difference; thus I wanted to have a more verbose way of expressing that the 
private impl. fn. should not be exposed. I won't insist though.

> I've put it here because simplifySValOnce references this even in its header 
> comment.
Well, it also refers to `simplifySValOnce()`, yet that's below that comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127285

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

[PATCH] D107082: [X86][RFC] Enable `_Float16` type support on X86 following the psABI

[LuoYuanke via Phabricator via cfe-commits]

LuoYuanke added inline comments.



Comment at: llvm/test/Analysis/CostModel/X86/fptoi_sat.ll:852
+; SSE2-NEXT:  Cost Model: Found an estimated cost of 3 for instruction: %f16u1 
= call i1 @llvm.fptoui.sat.i1.f16(half undef)
+; SSE2-NEXT:  Cost Model: Found an estimated cost of 5 for instruction: %f16s8 
= call i8 @llvm.fptosi.sat.i8.f16(half undef)
+; SSE2-NEXT:  Cost Model: Found an estimated cost of 3 for instruction: %f16u8 
= call i8 @llvm.fptoui.sat.i8.f16(half undef)

It seems the cost is reduced in general. Is it because we pass/return f16 by 
xmm register?



Comment at: llvm/test/CodeGen/MIR/X86/inline-asm-registers.mir:31
   ; CHECK-LABEL: name: test
-  ; CHECK: INLINEASM &foo, 0 /* attdialect */, 4390922 /* regdef:GR64 */, def 
$rsi, 4390922 /* regdef:GR64 */, def dead $rdi,
-INLINEASM &foo, 0, 4390922, def $rsi, 4390922, def dead $rdi, 2147549193, 
killed $rdi, 2147483657, killed $rsi, 12, implicit-def dead early-clobber 
$eflags
+  ; CHECK: INLINEASM &foo, 0 /* attdialect */, 4456458 /* regdef:GR64 */, def 
$rsi, 4456458 /* regdef:GR64 */, def dead $rdi,
+INLINEASM &foo, 0, 4456458, def $rsi, 4456458, def dead $rdi, 2147549193, 
killed $rdi, 2147483657, killed $rsi, 12, implicit-def dead early-clobber 
$eflags

Why f16 patch affect this test case? There is no fp instruction in this test 
case.



Comment at: llvm/test/CodeGen/X86/atomic-non-integer.ll:253
+; X64-SSE-NEXT:movzwl (%rdi), %eax
+; X64-SSE-NEXT:pinsrw $0, %eax, %xmm0
+; X64-SSE-NEXT:retq

I notice X86-SSE1 return by GPR. Should we also return by GPR for X64-SSE?



Comment at: llvm/test/CodeGen/X86/avx512-insert-extract.ll:2307
+; SKX-NEXT:vmovd %ecx, %xmm0
+; SKX-NEXT:vcvtph2ps %xmm0, %xmm0
+; SKX-NEXT:vmovss %xmm0, %xmm0, %xmm0 {%k2} {z}

Is code less efficient than previous code? Why previous code still works 
without convert half to float?



Comment at: llvm/test/CodeGen/X86/avx512-masked_memop-16-8.ll:156
 ; Make sure we scalarize masked loads of f16.
 define <16 x half> @test_mask_load_16xf16(<16 x i1> %mask, <16 x half>* %addr, 
<16 x half> %val) {
 ; CHECK-LABEL: test_mask_load_16xf16:

It seems parameter %val is useless.



Comment at: llvm/test/CodeGen/X86/callbr-asm-bb-exports.ll:20
 ; CHECK-NEXT: t22: ch,glue = CopyToReg t17, Register:i32 %5, t8
-; CHECK-NEXT: t30: ch,glue = inlineasm_br t22, TargetExternalSymbol:i64'xorl 
$0, $0; jmp ${1:l}', MDNode:ch, TargetConstant:i64<8>, 
TargetConstant:i32<2293769>, Register:i32 %5, TargetConstant:i64<13>, 
TargetBlockAddress:i64<@test, %fail> 0, TargetConstant:i32<12>, Register:i32 
$df, TargetConstant:i32<12>, Register:i16 $fpsw, TargetConstant:i32<12>, 
Register:i32 $eflags, t22:1
+; CHECK-NEXT: t30: ch,glue = inlineasm_br t22, TargetExternalSymbol:i64'xorl 
$0, $0; jmp ${1:l}', MDNode:ch, TargetConstant:i64<8>, 
TargetConstant:i32<2359305>, Register:i32 %5, TargetConstant:i64<13>, 
TargetBlockAddress:i64<@test, %fail> 0, TargetConstant:i32<12>, Register:i32 
$df, TargetConstant:i32<12>, Register:i16 $fpsw, TargetConstant:i32<12>, 
Register:i32 $eflags, t22:1
 

Why this test is affacted? Is it caused by calling convention change?



Comment at: llvm/test/CodeGen/X86/fmf-flags.ll:115
-; X64-NEXT:movzwl %di, %edi
-; X64-NEXT:callq __gnu_h2f_ieee@PLT
 ; X64-NEXT:mulss {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0

Does __gnu_h2f_ieee retrun from xmm?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107082

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

[PATCH] D125094: [ARM][Thumb] Command-line option to ensure AAPCS compliant Frame Records

[Lucas Prates via Phabricator via cfe-commits]

pratlucas updated this revision to Diff 435157.
pratlucas added a comment.

Avoid scavenging extra register when accessing const pool.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125094

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Arch/ARM.cpp
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMBaseRegisterInfo.cpp
  llvm/lib/Target/ARM/ARMCallingConv.td
  llvm/lib/Target/ARM/ARMFrameLowering.cpp
  llvm/lib/Target/ARM/ARMFrameLowering.h
  llvm/lib/Target/ARM/ARMMachineFunctionInfo.h
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/Thumb1FrameLowering.cpp
  llvm/lib/Target/ARM/ThumbRegisterInfo.cpp
  llvm/test/CodeGen/ARM/frame-chain-reserved-fp.ll
  llvm/test/CodeGen/ARM/frame-chain.ll
  llvm/test/CodeGen/Thumb/frame-access.ll
  llvm/test/CodeGen/Thumb/frame-chain-reserved-fp.ll
  llvm/test/CodeGen/Thumb/frame-chain.ll

Index: llvm/test/CodeGen/Thumb/frame-chain.ll
===
--- /dev/null
+++ llvm/test/CodeGen/Thumb/frame-chain.ll
@@ -0,0 +1,288 @@
+; RUN: llc -mtriple thumb-arm-none-eabi -filetype asm -o - %s -frame-pointer=all | FileCheck %s --check-prefixes=FP,LEAF-FP
+; RUN: llc -mtriple thumb-arm-none-eabi -filetype asm -o - %s -frame-pointer=all -mattr=+aapcs-frame-chain | FileCheck %s --check-prefixes=FP-AAPCS,LEAF-FP
+; RUN: llc -mtriple thumb-arm-none-eabi -filetype asm -o - %s -frame-pointer=all -mattr=+aapcs-frame-chain-leaf | FileCheck %s --check-prefixes=FP-AAPCS,LEAF-FP-AAPCS
+; RUN: llc -mtriple thumb-arm-none-eabi -filetype asm -o - %s -frame-pointer=non-leaf | FileCheck %s --check-prefixes=FP,LEAF-NOFP
+; RUN: llc -mtriple thumb-arm-none-eabi -filetype asm -o - %s -frame-pointer=non-leaf -mattr=+aapcs-frame-chain | FileCheck %s --check-prefixes=FP-AAPCS,LEAF-NOFP
+; RUN: llc -mtriple thumb-arm-none-eabi -filetype asm -o - %s -frame-pointer=non-leaf -mattr=+aapcs-frame-chain-leaf | FileCheck %s --check-prefixes=FP-AAPCS,LEAF-NOFP-AAPCS
+; RUN: llc -mtriple thumb-arm-none-eabi -filetype asm -o - %s -frame-pointer=none | FileCheck %s --check-prefixes=NOFP,LEAF-NOFP
+; RUN: llc -mtriple thumb-arm-none-eabi -filetype asm -o - %s -frame-pointer=none -mattr=+aapcs-frame-chain | FileCheck %s --check-prefixes=NOFP-AAPCS,LEAF-NOFP
+; RUN: llc -mtriple thumb-arm-none-eabi -filetype asm -o - %s -frame-pointer=none -mattr=+aapcs-frame-chain-leaf | FileCheck %s --check-prefixes=NOFP-AAPCS,LEAF-NOFP-AAPCS
+
+define dso_local noundef i32 @leaf(i32 noundef %0) {
+; LEAF-FP-LABEL: leaf:
+; LEAF-FP:   @ %bb.0:
+; LEAF-FP-NEXT:.pad #4
+; LEAF-FP-NEXT:sub sp, #4
+; LEAF-FP-NEXT:str r0, [sp]
+; LEAF-FP-NEXT:adds r0, r0, #4
+; LEAF-FP-NEXT:add sp, #4
+; LEAF-FP-NEXT:bx lr
+;
+; LEAF-FP-AAPCS-LABEL: leaf:
+; LEAF-FP-AAPCS:   @ %bb.0:
+; LEAF-FP-AAPCS-NEXT:.save {lr}
+; LEAF-FP-AAPCS-NEXT:push {lr}
+; LEAF-FP-AAPCS-NEXT:mov lr, r11
+; LEAF-FP-AAPCS-NEXT:.save {r11}
+; LEAF-FP-AAPCS-NEXT:push {lr}
+; LEAF-FP-AAPCS-NEXT:.setfp r11, sp
+; LEAF-FP-AAPCS-NEXT:mov r11, sp
+; LEAF-FP-AAPCS-NEXT:.pad #4
+; LEAF-FP-AAPCS-NEXT:sub sp, #4
+; LEAF-FP-AAPCS-NEXT:str r0, [sp]
+; LEAF-FP-AAPCS-NEXT:adds r0, r0, #4
+; LEAF-FP-AAPCS-NEXT:add sp, #4
+; LEAF-FP-AAPCS-NEXT:pop {r1}
+; LEAF-FP-AAPCS-NEXT:mov r11, r1
+; LEAF-FP-AAPCS-NEXT:pop {r1}
+; LEAF-FP-AAPCS-NEXT:bx r1
+;
+; LEAF-NOFP-LABEL: leaf:
+; LEAF-NOFP:   @ %bb.0:
+; LEAF-NOFP-NEXT:.pad #4
+; LEAF-NOFP-NEXT:sub sp, #4
+; LEAF-NOFP-NEXT:str r0, [sp]
+; LEAF-NOFP-NEXT:adds r0, r0, #4
+; LEAF-NOFP-NEXT:add sp, #4
+; LEAF-NOFP-NEXT:bx lr
+;
+; LEAF-NOFP-AAPCS-LABEL: leaf:
+; LEAF-NOFP-AAPCS:   @ %bb.0:
+; LEAF-NOFP-AAPCS-NEXT:.pad #4
+; LEAF-NOFP-AAPCS-NEXT:sub sp, #4
+; LEAF-NOFP-AAPCS-NEXT:str r0, [sp]
+; LEAF-NOFP-AAPCS-NEXT:adds r0, r0, #4
+; LEAF-NOFP-AAPCS-NEXT:add sp, #4
+; LEAF-NOFP-AAPCS-NEXT:bx lr
+  %2 = alloca i32, align 4
+  store i32 %0, i32* %2, align 4
+  %3 = load i32, i32* %2, align 4
+  %4 = add nsw i32 %3, 4
+  ret i32 %4
+}
+
+define dso_local noundef i32 @non_leaf(i32 noundef %0) {
+; FP-LABEL: non_leaf:
+; FP:   @ %bb.0:
+; FP-NEXT:.save {r7, lr}
+; FP-NEXT:push {r7, lr}
+; FP-NEXT:.setfp r7, sp
+; FP-NEXT:add r7, sp, #0
+; FP-NEXT:.pad #8
+; FP-NEXT:sub sp, #8
+; FP-NEXT:str r0, [sp, #4]
+; FP-NEXT:bl leaf
+; FP-NEXT:adds r0, r0, #1
+; FP-NEXT:add sp, #8
+; FP-NEXT:pop {r7}
+; FP-NEXT:pop {r1}
+; FP-NEXT:bx r1
+;
+; FP-AAPCS-LABEL: non_leaf:
+; FP-AAPCS:   @ %bb.0:
+; FP-AAPCS-NEXT:.save {lr}
+; FP-AAPCS-NEXT:push {lr}
+; FP-AAPCS-NEXT:mov lr, r11
+; FP-AAPCS-NEXT:.save {r11}
+; FP-AAPCS-NEXT:push {lr}
+; FP-AAPCS-NEXT:.setfp r11, sp
+; FP-AAPCS-NEXT:mov r11, sp
+; FP-AAPCS-NEXT:.pad #8
+; FP-AAPCS-NEXT:sub sp, #8
+; FP

[PATCH] D125094: [ARM][Thumb] Command-line option to ensure AAPCS compliant Frame Records

[Lucas Prates via Phabricator via cfe-commits]

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



Comment at: llvm/test/CodeGen/Thumb/frame-access.ll:335
+; CHECK-FP-AAPCS: mov r1, r11
+; CHECK-FP-AAPCS: ldr r0, [r0, r1]
+; CHECK: bl i

efriedma wrote:
> This sequence requires, in general, scavenging two registers.  I'm not sure 
> we can do that in general?  I think we normally only have one emergency spill 
> slot.
> 
> Maybe we can save a register using add instead of mov; something like `ldr 
> r0, .LCPI0_0; add r0, r11; ldr r0, [r0]`.
That's true. Loads aren't affected as they reuse the destination register, but 
stores would indeed require the scaveging of two registers.
I've updaded the code to use the `add` approach instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125094

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

[PATCH] D117229: [Analyzer] Produce SymbolCast for pointer to integer cast

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

steakhal added a comment.

Looks correct to me.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:110-112
 
+  SVal simplifySymbolCast(SymbolRef SE, QualType CastTy);
+

You don't need to have gap between these two. They belong in the same overload 
set.



Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:776-785
+
+AnalyzerOptions &Opts =
+StateMgr.getOwningEngine().getAnalysisManager().getAnalyzerOptions();
+if (Opts.ShouldSupportSymbolicIntegerCasts) {
+  const MemRegion *R = V.getRegion();
+  if (R && !OriginalTy.isNull())
+if (const auto *SR = dyn_cast(R))





Comment at: clang/test/Analysis/produce-ptr-to-integer-symbolcast.cpp:24-27
+  // FIXME, below we should produce a SymbolCast instead of the LocAsInteger.
+  // However, for that we'd need to be able to store a loc::GotoLabel SVal
+  // inside the SymbolCast. But currently we can put only another SymExpr
+  // inside a SymbolCast.

Is this the only obstacle to eradicate `LocAsInteger`?



Comment at: 
clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp:1-6
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -analyzer-config support-symbolic-integer-casts=true \
+// RUN:   -verify

I think you need to pin the target. You assume a specific pointer bitwidth in 
the test.



Comment at: 
clang/test/Analysis/symbol-simplification-mem-region-to-int-cast.cpp:23
+  clang_analyzer_eval(p == 0);// expected-warning{{TRUE}}
+  clang_analyzer_eval(p_as_integer == 0); // expected-warning{{UNKNOWN}}
+

Why is this ``UNKNOWN`? I would expect `TRUE` here as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117229

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

[PATCH] D127306: [analyzer] Treat system globals as mutable if they are not const

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> In theory, the engine should propagate taint in default eval call. If that 
> would be the case, we would still have this report.

How complex would it be to add the taint propagation to the engine/evalCall? Do 
you see that feasible as a parent patch of this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127306

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

[PATCH] D127287: clang: Introduce -fexperimental-max-bitint-width

[Matthias Gehre via Phabricator via cfe-commits]

mgehre-amd updated this revision to Diff 435161.
mgehre-amd marked 4 inline comments as done.
mgehre-amd added a comment.

Used Optional
Added entry to release notes
Modifed help text according to suggestion
Added comment to LANGOPT that it will be removed in the future


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127287

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/Options.td
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Sema/large-bit-int.c

Index: clang/test/Sema/large-bit-int.c
===
--- /dev/null
+++ clang/test/Sema/large-bit-int.c
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -fexperimental-max-bitint-width=1024 -fsyntax-only -verify %s
+
+void f() {
+  _Static_assert(__BITINT_MAXWIDTH__ == 1024, "Macro value is unexpected.");
+
+  _BitInt(1024) a;
+  unsigned _BitInt(1024) b;
+
+  _BitInt(8388609) c;// expected-error {{signed _BitInt of bit sizes greater than 1024 not supported}}
+  unsigned _BitInt(0xFF) d; // expected-error {{unsigned _BitInt of bit sizes greater than 1024 not supported}}
+}
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -314,7 +314,7 @@
 
 #define BENIGN_LANGOPT(Name, Bits, Default, Description)
 #define BENIGN_ENUM_LANGOPT(Name, Type, Bits, Default, Description)
-#define BENIGN_VALUE_LANGOPT(Name, Type, Bits, Default, Description)
+#define BENIGN_VALUE_LANGOPT(Name, Bits, Default, Description)
 #include "clang/Basic/LangOptions.def"
 
   if (ExistingLangOpts.ModuleFeatures != LangOpts.ModuleFeatures) {
Index: clang/lib/Basic/TargetInfo.cpp
===
--- clang/lib/Basic/TargetInfo.cpp
+++ clang/lib/Basic/TargetInfo.cpp
@@ -150,6 +150,9 @@
   PlatformMinVersion = VersionTuple();
 
   MaxOpenCLWorkGroupSize = 1024;
+
+  MaxBitIntWidth.reset();
+
   ProgramAddrSpace = 0;
 }
 
@@ -478,6 +481,9 @@
 Diags.Report(diag::err_opt_not_valid_on_target) << "-fprotect-parens";
 Opts.ProtectParens = false;
   }
+
+  if (Opts.MaxBitIntWidth)
+MaxBitIntWidth = Opts.MaxBitIntWidth;
 }
 
 bool TargetInfo::initFeatureMap(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -6082,6 +6082,12 @@
 def fobjc_gc : Flag<["-"], "fobjc-gc">, Group,
   HelpText<"Enable Objective-C garbage collection">;
 
+def fexperimental_max_bitint_width_EQ:
+  Joined<["-"], "fexperimental-max-bitint-width=">, Group,
+  MetaVarName<"">,
+  HelpText<"Set the maximum bitwidth for _BitInt (this option is expected to be removed in the future)">,
+  MarshallingInfoInt>;
+
 } // let Flags = [CC1Option, NoDriverOption]
 
 //===--===//
Index: clang/include/clang/Basic/TargetInfo.h
===
--- clang/include/clang/Basic/TargetInfo.h
+++ clang/include/clang/Basic/TargetInfo.h
@@ -31,6 +31,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Frontend/OpenMP/OMPGridValues.h"
+#include "llvm/IR/DerivedTypes.h"
 #include "llvm/Support/DataTypes.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/VersionTuple.h"
@@ -235,6 +236,8 @@
 
   unsigned MaxOpenCLWorkGroupSize;
 
+  Optional MaxBitIntWidth;
+
   Optional DarwinTargetVariantTriple;
 
   // TargetInfo Constructor.  Default initializes all fields.
@@ -595,11 +598,16 @@
   // Different targets may support a different maximum width for the _BitInt
   // type, depending on what operations are supported.
   virtual size_t getMaxBitIntWidth() const {
+// Consider -fexperimental-max-bitint-width= first.
+if (MaxBitIntWidth)
+  return std::min(*MaxBitIntWidth, llvm::IntegerType::MAX_INT_BITS);
+
 // FIXME: this value should be llvm::IntegerType::MAX_INT_BITS, which is
 // maximum bit width that LLVM claims its IR can support. However, most
-// backends currently have a bug where they only support division
-// operations on types that are <= 128 bits and crash otherwise. We're
-// setting the max supported value to 128 to be conservative.
+// backends currently have a bug where they only support float to int
+// conversion (and vice versa) on types that are <= 128 bits and crash
+// otherwise. We're setting the max supported value to 128 to be
+// conservative.
 return 128;
   }
 
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/in

[PATCH] D127287: clang: Introduce -fexperimental-max-bitint-width

[Matthias Gehre via Phabricator via cfe-commits]

mgehre-amd added a comment.

Thanks for the quick review!




Comment at: clang/test/Sema/large-bit-int.c:1
+// RUN: %clang_cc1 -fexperimental-max-bitint-width=1024 -fsyntax-only -verify 
%s -Wno-unused -triple x86_64-gnu-linux
+

aaron.ballman wrote:
> Thoughts on adding a frontend diagnostic if the user specifies a value larger 
> than `llvm::IntegerType::MAX_INT_BITS`? I'm on the fence. OTOneH, this gives 
> a good user experience in the event of mistypes in the command line, 
> OTOtherH, it's a cc1-only option that we expect to remove in the (hopefully) 
> near future.
> 
> Also, I don't think the triple is needed and I'm pretty sure `-Wno-unused` is 
> the default already.
I was thinking that this is both only a cc1 option and at the same time 
MAX_INT_BITS is really big, so I don't imagine a practical case where one 
intends to have bigger _BitInts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127287

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

[PATCH] D127267: [NVPTX] Add setAuxTarget override rather than make a new TargetInfo

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

need a test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127267

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

[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

[Quinn Pham via Phabricator via cfe-commits]

quinnp created this revision.
Herald added a project: All.
quinnp requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

This patch correctly sets the devtoolset on RHEL.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127310

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


Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2149,7 +2149,7 @@
 
   if (ToolsetVersion > ChosenToolsetVersion) {
 ChosenToolsetVersion = ToolsetVersion;
-ChosenToolsetDir = "/opt/rh/" + ToolsetDir.str();
+ChosenToolsetDir = "/opt/rh/" + ToolsetDir.str() + "/root/usr";
   }
 }
 


Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -2149,7 +2149,7 @@
 
   if (ToolsetVersion > ChosenToolsetVersion) {
 ChosenToolsetVersion = ToolsetVersion;
-ChosenToolsetDir = "/opt/rh/" + ToolsetDir.str();
+ChosenToolsetDir = "/opt/rh/" + ToolsetDir.str() + "/root/usr";
   }
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4f8668d - [clang] co_return cleanup

[Nathan Sidwell via cfe-commits]

Author: Nathan Sidwell
Date: 2022-06-08T08:08:42-07:00
New Revision: 4f8668d9f73a0dc234ebf9938bcf24061991e9ae

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

LOG: [clang] co_return cleanup

There's no need for the CoreturnStmt getChildren member to deal with
the presence or absence of the operand member. All users already deal
with null children.

Reviewed By: MaskRay

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

Added: 


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

Removed: 




diff  --git a/clang/include/clang/AST/StmtCXX.h 
b/clang/include/clang/AST/StmtCXX.h
index 5ccf6904048e3..2c71f86768963 100644
--- a/clang/include/clang/AST/StmtCXX.h
+++ b/clang/include/clang/AST/StmtCXX.h
@@ -497,16 +497,10 @@ class CoreturnStmt : public Stmt {
   }
 
   child_range children() {
-if (!getOperand())
-  return child_range(SubStmts + SubStmt::PromiseCall,
- SubStmts + SubStmt::Count);
 return child_range(SubStmts, SubStmts + SubStmt::Count);
   }
 
   const_child_range children() const {
-if (!getOperand())
-  return const_child_range(SubStmts + SubStmt::PromiseCall,
-   SubStmts + SubStmt::Count);
 return const_child_range(SubStmts, SubStmts + SubStmt::Count);
   }
 



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

[PATCH] D125542: [clang] co_return cleanup

[Nathan Sidwell 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 rG4f8668d9f73a: [clang] co_return cleanup (authored by 
urnathan).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125542

Files:
  clang/include/clang/AST/StmtCXX.h


Index: clang/include/clang/AST/StmtCXX.h
===
--- clang/include/clang/AST/StmtCXX.h
+++ clang/include/clang/AST/StmtCXX.h
@@ -497,16 +497,10 @@
   }
 
   child_range children() {
-if (!getOperand())
-  return child_range(SubStmts + SubStmt::PromiseCall,
- SubStmts + SubStmt::Count);
 return child_range(SubStmts, SubStmts + SubStmt::Count);
   }
 
   const_child_range children() const {
-if (!getOperand())
-  return const_child_range(SubStmts + SubStmt::PromiseCall,
-   SubStmts + SubStmt::Count);
 return const_child_range(SubStmts, SubStmts + SubStmt::Count);
   }
 


Index: clang/include/clang/AST/StmtCXX.h
===
--- clang/include/clang/AST/StmtCXX.h
+++ clang/include/clang/AST/StmtCXX.h
@@ -497,16 +497,10 @@
   }
 
   child_range children() {
-if (!getOperand())
-  return child_range(SubStmts + SubStmt::PromiseCall,
- SubStmts + SubStmt::Count);
 return child_range(SubStmts, SubStmts + SubStmt::Count);
   }
 
   const_child_range children() const {
-if (!getOperand())
-  return const_child_range(SubStmts + SubStmt::PromiseCall,
-   SubStmts + SubStmt::Count);
 return const_child_range(SubStmts, SubStmts + SubStmt::Count);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127310: [clang][driver] fix to correctly set devtoolset on RHEL

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added inline comments.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2152
 ChosenToolsetVersion = ToolsetVersion;
-ChosenToolsetDir = "/opt/rh/" + ToolsetDir.str();
+ChosenToolsetDir = "/opt/rh/" + ToolsetDir.str() + "/root/usr";
   }

I believe this will cause a failure with the unit test added in the original 
patch.

And presumably since the unit test tests for paths that don't include 
`/root/usr`, there are real world toolsets that also don't use that suffix. So 
we should presumably add both `Prefixes`.



Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:2157
 if (ChosenToolsetVersion > 0)
   Prefixes.push_back(ChosenToolsetDir);
   }

Can we not just change this to something like:
```
if (ChosenToolsetVersion > 0) {
  Prefixes.push_back(ChosenToolsetDir);
  Prefixes.push_back(ChosenToolsetDir + "/root/usr");
}
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127310

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

[PATCH] D125862: [clang][driver] Add gcc-toolset/devtoolset 12 to prefixes

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added subscribers: quinnp, nemanjai.
nemanjai added a comment.

The original toolchain detection added `root/usr` to the paths which was 
certainly necessary on PowerPC. Since that suffix is removed in this patch, our 
Redhat buildbot is now broken 
(https://lab.llvm.org/buildbot/#/builders/57/builds/18577). @quinnp has posted 
an update to this at https://reviews.llvm.org/D127310.


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

https://reviews.llvm.org/D125862

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

[PATCH] D127312: [clang][dataflow] Move PointeeLoc field from IndirectionValue into PointerValue and ReferenceValue classes

[weiyi via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D127312

Files:
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.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
@@ -122,24 +122,24 @@
   // [[p]]
 }
   )";
-  runDataflow(
-  Code, [](llvm::ArrayRef<
-   std::pair>>
-   Results,
-   ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-const Environment &Env = Results[0].second.Env;
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
 
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const Value *FooVal = Env.getValue(*FooLoc);
-EXPECT_TRUE(isa_and_nonnull(FooVal));
-  });
+const Value *FooVal = Env.getValue(*FooLoc);
+EXPECT_TRUE(isa_and_nonnull(FooVal));
+  });
 }
 
 TEST_F(TransferTest, StructVarDecl) {
@@ -293,29 +293,29 @@
   // [[p]]
 }
   )";
-  runDataflow(
-  Code, [](llvm::ArrayRef<
-   std::pair>>
-   Results,
-   ASTContext &ASTCtx) {
-ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
-const Environment &Env = Results[0].second.Env;
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
 
-const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
-ASSERT_THAT(FooDecl, NotNull());
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
 
-const StorageLocation *FooLoc =
-Env.getStorageLocation(*FooDecl, SkipPast::None);
-ASSERT_TRUE(isa_and_nonnull(FooLoc));
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
 
-const ReferenceValue *FooVal =
-cast(Env.getValue(*FooLoc));
-const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
-EXPECT_TRUE(isa(&FooPointeeLoc));
+const ReferenceValue *FooVal =
+cast(Env.getValue(*FooLoc));
+const StorageLocation &FooPointeeLoc = FooVal->getPointeeLoc();
+EXPECT_TRUE(isa(&FooPointeeLoc));
 
-const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
-EXPECT_TRUE(isa_and_nonnull(FooPointeeVal));
-  });
+const Value *FooPointeeVal = Env.getValue(FooPointeeLoc);
+EXPECT_TRUE(isa_and_nonnull(FooPointeeVal));
+  });
 }
 
 TEST_F(TransferTest, SelfReferentialReferenceVarDecl) {
@@ -3327,23 +3327,23 @@
 ASSERT_THAT(LDecl, NotNull());
 
 // Inner.
-auto *LVal = dyn_cast(
-InnerEnv.getValue(*LDecl, SkipPast::None));
+auto *LVal =
+dyn_cast(InnerEnv.getValue(*LDecl, SkipPast::None));
 ASSERT_THAT(LVal, NotNull());
 
 EXPECT_EQ(&LVal->getPointeeLoc(),
   InnerEnv.getStorageLocation(*ValDecl, SkipPast::Reference));
 
 // Outer.
-LVal = dyn_cast(
-OuterEnv.getValue(*LDecl, SkipPast::None));
+LVal =
+dyn_cast(OuterEnv.getValue(*LDecl, SkipPast::None));
 ASSERT_THAT(LVal, NotNull());
 
 // The loop body may not have been executed, so we should not conclude
 // that `l` points to `val`.
 EXPECT_NE(&LVal->getPointeeLoc(),
   OuterEnv.getStorageLocation(*V

[clang] 9d6d069 - Add a parameter to LoadFromASTFile that accepts a file system and defaults to the real file-system.

[Yitzhak Mandelbaum via cfe-commits]

Author: Andy Soffer
Date: 2022-06-08T15:37:52Z
New Revision: 9d6d069f4e9a6dd6882aaf896f73f234b3dd7194

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

LOG: Add a parameter to LoadFromASTFile that accepts a file system and defaults 
to the real file-system.

Reviewed By: ymandel

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

Added: 


Modified: 
clang/include/clang/Frontend/ASTUnit.h
clang/lib/Frontend/ASTUnit.cpp

Removed: 




diff  --git a/clang/include/clang/Frontend/ASTUnit.h 
b/clang/include/clang/Frontend/ASTUnit.h
index 6cf9f3ff936f..420246278b47 100644
--- a/clang/include/clang/Frontend/ASTUnit.h
+++ b/clang/include/clang/Frontend/ASTUnit.h
@@ -696,7 +696,9 @@ class ASTUnit {
   bool UseDebugInfo = false, bool OnlyLocalDecls = false,
   CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::None,
   bool AllowASTWithCompilerErrors = false,
-  bool UserFilesAreVolatile = false);
+  bool UserFilesAreVolatile = false,
+  IntrusiveRefCntPtr VFS =
+  llvm::vfs::getRealFileSystem());
 
 private:
   /// Helper function for \c LoadFromCompilerInvocation() and

diff  --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index 1cf20a0b662a..91dde2d86d46 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -759,7 +759,8 @@ std::unique_ptr ASTUnit::LoadFromASTFile(
 WhatToLoad ToLoad, IntrusiveRefCntPtr Diags,
 const FileSystemOptions &FileSystemOpts, bool UseDebugInfo,
 bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics,
-bool AllowASTWithCompilerErrors, bool UserFilesAreVolatile) {
+bool AllowASTWithCompilerErrors, bool UserFilesAreVolatile,
+IntrusiveRefCntPtr VFS) {
   std::unique_ptr AST(new ASTUnit(true));
 
   // Recover resources if we crash before exiting this method.
@@ -775,8 +776,6 @@ std::unique_ptr ASTUnit::LoadFromASTFile(
   AST->OnlyLocalDecls = OnlyLocalDecls;
   AST->CaptureDiagnostics = CaptureDiagnostics;
   AST->Diagnostics = Diags;
-  IntrusiveRefCntPtr VFS =
-  llvm::vfs::getRealFileSystem();
   AST->FileMgr = new FileManager(FileSystemOpts, VFS);
   AST->UserFilesAreVolatile = UserFilesAreVolatile;
   AST->SourceMgr = new SourceManager(AST->getDiagnostics(),



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

[PATCH] D126888: Add a parameter to LoadFromASTFile that accepts a file system and defaults to the real file-system.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9d6d069f4e9a: Add a parameter to LoadFromASTFile that 
accepts a file system and defaults to… (authored by asoffer, committed by 
ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126888

Files:
  clang/include/clang/Frontend/ASTUnit.h
  clang/lib/Frontend/ASTUnit.cpp


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -759,7 +759,8 @@
 WhatToLoad ToLoad, IntrusiveRefCntPtr Diags,
 const FileSystemOptions &FileSystemOpts, bool UseDebugInfo,
 bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics,
-bool AllowASTWithCompilerErrors, bool UserFilesAreVolatile) {
+bool AllowASTWithCompilerErrors, bool UserFilesAreVolatile,
+IntrusiveRefCntPtr VFS) {
   std::unique_ptr AST(new ASTUnit(true));
 
   // Recover resources if we crash before exiting this method.
@@ -775,8 +776,6 @@
   AST->OnlyLocalDecls = OnlyLocalDecls;
   AST->CaptureDiagnostics = CaptureDiagnostics;
   AST->Diagnostics = Diags;
-  IntrusiveRefCntPtr VFS =
-  llvm::vfs::getRealFileSystem();
   AST->FileMgr = new FileManager(FileSystemOpts, VFS);
   AST->UserFilesAreVolatile = UserFilesAreVolatile;
   AST->SourceMgr = new SourceManager(AST->getDiagnostics(),
Index: clang/include/clang/Frontend/ASTUnit.h
===
--- clang/include/clang/Frontend/ASTUnit.h
+++ clang/include/clang/Frontend/ASTUnit.h
@@ -696,7 +696,9 @@
   bool UseDebugInfo = false, bool OnlyLocalDecls = false,
   CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::None,
   bool AllowASTWithCompilerErrors = false,
-  bool UserFilesAreVolatile = false);
+  bool UserFilesAreVolatile = false,
+  IntrusiveRefCntPtr VFS =
+  llvm::vfs::getRealFileSystem());
 
 private:
   /// Helper function for \c LoadFromCompilerInvocation() and


Index: clang/lib/Frontend/ASTUnit.cpp
===
--- clang/lib/Frontend/ASTUnit.cpp
+++ clang/lib/Frontend/ASTUnit.cpp
@@ -759,7 +759,8 @@
 WhatToLoad ToLoad, IntrusiveRefCntPtr Diags,
 const FileSystemOptions &FileSystemOpts, bool UseDebugInfo,
 bool OnlyLocalDecls, CaptureDiagsKind CaptureDiagnostics,
-bool AllowASTWithCompilerErrors, bool UserFilesAreVolatile) {
+bool AllowASTWithCompilerErrors, bool UserFilesAreVolatile,
+IntrusiveRefCntPtr VFS) {
   std::unique_ptr AST(new ASTUnit(true));
 
   // Recover resources if we crash before exiting this method.
@@ -775,8 +776,6 @@
   AST->OnlyLocalDecls = OnlyLocalDecls;
   AST->CaptureDiagnostics = CaptureDiagnostics;
   AST->Diagnostics = Diags;
-  IntrusiveRefCntPtr VFS =
-  llvm::vfs::getRealFileSystem();
   AST->FileMgr = new FileManager(FileSystemOpts, VFS);
   AST->UserFilesAreVolatile = UserFilesAreVolatile;
   AST->SourceMgr = new SourceManager(AST->getDiagnostics(),
Index: clang/include/clang/Frontend/ASTUnit.h
===
--- clang/include/clang/Frontend/ASTUnit.h
+++ clang/include/clang/Frontend/ASTUnit.h
@@ -696,7 +696,9 @@
   bool UseDebugInfo = false, bool OnlyLocalDecls = false,
   CaptureDiagsKind CaptureDiagnostics = CaptureDiagsKind::None,
   bool AllowASTWithCompilerErrors = false,
-  bool UserFilesAreVolatile = false);
+  bool UserFilesAreVolatile = false,
+  IntrusiveRefCntPtr VFS =
+  llvm::vfs::getRealFileSystem());
 
 private:
   /// Helper function for \c LoadFromCompilerInvocation() and
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D127243: [clang][deps] Make order of module dependencies deterministic

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:181
 std::vector PrebuiltModuleDeps;
-std::map ClangModuleDeps;
+llvm::MapVector,
+llvm::StringMap>

jansvoboda11 wrote:
> What's the reason for wrapping `ModuleDeps` in `unique_ptr`?
Oh hmm, my thinking was that DenseMap is optimized for small values, but I 
guess in this setup the map only stores integers and the values are in a 
vector.  I'll change this to values, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127243

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Markus Mützel via Phabricator via cfe-commits]

mmuetzel added a comment.

In D126291#3566692 , @rovka wrote:

> @mmuetzel are you going to upload it to Phab? I gave it a try on my machine 
> and I can't compile the runtime with it, but I'll try to fix it on my end and 
> we can continue the discussion in a new revision.

I can try. But I never worked with Phabricator before...


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

https://reviews.llvm.org/D126291

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

[PATCH] D127306: [analyzer] Treat system globals as mutable if they are not const

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

steakhal added a comment.

In D127306#3566761 , @martong wrote:

>> In theory, the engine should propagate taint in default eval call. If that 
>> would be the case, we would still have this report.
>
> How complex would it be to add the taint propagation to the engine/evalCall? 
> Do you see that feasible as a parent patch of this?

I see your point, but I don't think we should delay D124244 
 because of that. By landing this, we could 
also land that. And the change you propose might take significantly longer to 
make and verify.
I could make some prototypes, but I'm not making promises.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127306

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

[PATCH] D127277: [clang][analyzer] Fix StdLibraryFunctionsChecker 'mkdir' return value.

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

steakhal added a comment.

This looks like a standalone patch.
Feel free to rebase to `main` and land it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127277

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