[clang] abe997b - [CMake][Fuchsia] Switch to lld on Apple platforms

[Petr Hosek via cfe-commits]

Author: Petr Hosek
Date: 2022-03-22T01:06:30-07:00
New Revision: abe997bb2dd61188784954ae866352740629985d

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

LOG: [CMake][Fuchsia] Switch to lld on Apple platforms

lld Mach-O backend supports all our use cases now.

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

Added: 


Modified: 
clang/cmake/caches/Fuchsia-stage2.cmake
clang/cmake/caches/Fuchsia.cmake

Removed: 




diff  --git a/clang/cmake/caches/Fuchsia-stage2.cmake 
b/clang/cmake/caches/Fuchsia-stage2.cmake
index d64229bd572eb..df15aa65cc9d2 100644
--- a/clang/cmake/caches/Fuchsia-stage2.cmake
+++ b/clang/cmake/caches/Fuchsia-stage2.cmake
@@ -9,10 +9,7 @@ set(LLVM_ENABLE_RUNTIMES 
"compiler-rt;libcxx;libcxxabi;libunwind" CACHE STRING "
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(LLVM_ENABLE_LLD ON CACHE BOOL "")
-endif()
+set(LLVM_ENABLE_LLD ON CACHE BOOL "")
 set(LLVM_ENABLE_LTO ON CACHE BOOL "")
 set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBCXX ON CACHE BOOL "")
@@ -31,11 +28,8 @@ if(WIN32)
 endif()
 
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
-  set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
-endif()
+set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
+set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
 set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
 set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
 set(CLANG_ENABLE_STATIC_ANALYZER ON CACHE BOOL "")

diff  --git a/clang/cmake/caches/Fuchsia.cmake 
b/clang/cmake/caches/Fuchsia.cmake
index 8e9e44d5917ed..bf2ea1802963d 100644
--- a/clang/cmake/caches/Fuchsia.cmake
+++ b/clang/cmake/caches/Fuchsia.cmake
@@ -22,11 +22,8 @@ if(WIN32)
 endif()
 
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
-  set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
-endif()
+set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
+set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
 set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
 set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
 set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")
@@ -112,11 +109,8 @@ if(UNIX)
   set(BOOTSTRAP_CMAKE_EXE_LINKER_FLAGS "-ldl -lpthread" CACHE STRING "")
 endif()
 
+set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
 set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
-endif()
 
 set(CLANG_BOOTSTRAP_TARGETS
   check-all



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

[PATCH] D122047: [CMake][Fuchsia] Switch to lld on Apple platforms

[Petr Hosek via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGabe997bb2dd6: [CMake][Fuchsia] Switch to lld on Apple 
platforms (authored by phosek).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122047

Files:
  clang/cmake/caches/Fuchsia-stage2.cmake
  clang/cmake/caches/Fuchsia.cmake


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -22,11 +22,8 @@
 endif()
 
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
-  set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
-endif()
+set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
+set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
 set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
 set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
 set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")
@@ -112,11 +109,8 @@
   set(BOOTSTRAP_CMAKE_EXE_LINKER_FLAGS "-ldl -lpthread" CACHE STRING "")
 endif()
 
+set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
 set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
-endif()
 
 set(CLANG_BOOTSTRAP_TARGETS
   check-all
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -9,10 +9,7 @@
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(LLVM_ENABLE_LLD ON CACHE BOOL "")
-endif()
+set(LLVM_ENABLE_LLD ON CACHE BOOL "")
 set(LLVM_ENABLE_LTO ON CACHE BOOL "")
 set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBCXX ON CACHE BOOL "")
@@ -31,11 +28,8 @@
 endif()
 
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
-  set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
-endif()
+set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
+set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
 set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
 set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
 set(CLANG_ENABLE_STATIC_ANALYZER ON CACHE BOOL "")


Index: clang/cmake/caches/Fuchsia.cmake
===
--- clang/cmake/caches/Fuchsia.cmake
+++ clang/cmake/caches/Fuchsia.cmake
@@ -22,11 +22,8 @@
 endif()
 
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
-  set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
-endif()
+set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
+set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
 set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
 set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
 set(CLANG_ENABLE_STATIC_ANALYZER OFF CACHE BOOL "")
@@ -112,11 +109,8 @@
   set(BOOTSTRAP_CMAKE_EXE_LINKER_FLAGS "-ldl -lpthread" CACHE STRING "")
 endif()
 
+set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
 set(BOOTSTRAP_LLVM_ENABLE_LTO ON CACHE BOOL "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(BOOTSTRAP_LLVM_ENABLE_LLD ON CACHE BOOL "")
-endif()
 
 set(CLANG_BOOTSTRAP_TARGETS
   check-all
Index: clang/cmake/caches/Fuchsia-stage2.cmake
===
--- clang/cmake/caches/Fuchsia-stage2.cmake
+++ clang/cmake/caches/Fuchsia-stage2.cmake
@@ -9,10 +9,7 @@
 
 set(LLVM_ENABLE_BACKTRACES OFF CACHE BOOL "")
 set(LLVM_ENABLE_DIA_SDK OFF CACHE BOOL "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(LLVM_ENABLE_LLD ON CACHE BOOL "")
-endif()
+set(LLVM_ENABLE_LLD ON CACHE BOOL "")
 set(LLVM_ENABLE_LTO ON CACHE BOOL "")
 set(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR ON CACHE BOOL "")
 set(LLVM_ENABLE_LIBCXX ON CACHE BOOL "")
@@ -31,11 +28,8 @@
 endif()
 
 set(CLANG_DEFAULT_CXX_STDLIB libc++ CACHE STRING "")
-if(NOT APPLE)
-  # TODO: Remove this once we switch to ld64.lld.
-  set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
-  set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
-endif()
+set(CLANG_DEFAULT_LINKER lld CACHE STRING "")
+set(CLANG_DEFAULT_OBJCOPY llvm-objcopy CACHE STRING "")
 set(CLANG_DEFAULT_RTLIB compiler-rt CACHE STRING "")
 set(CLANG_ENABLE_ARCMT OFF CACHE BOOL "")
 set(CLANG_ENABLE_STATIC_ANALYZER ON CACHE BOOL "")
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118493: Set rpath on openmp executables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

It lets applications run with the libomp and libomptarget built with the 
toolchain. For users, they don't have to be root. For compiler devs, we test 
our work instead of whatever the distro had installed.

There's no info at that link. What's 'broken rpath'? If they mean runpath, then 
it's really on fedora to have Wl,--rpath set rpath instead of runpath. Setting 
the default in the linker to the one your own build tools rejects will break 
everything, not just llvm openmp.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118493

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

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 2 inline comments as done.
iains added subscribers: vsapsai, dblaikie.
iains added a comment.

So the adjustment to the error message is something I am 50/50 about (IMO it 
makes some messages more useful, but maybe not needed in others).

Without the change we get 
"cannot export redeclaration 'xxx' here since the previous declaration is not 
exported"

So, e.g in C++20 10.2 example 6. every case has the same error message (which 
was what prompted me to make the change).

With the change here we now get:
cannot export redeclaration 'f' here since the previous declaration has 
internal linkage
cannot export redeclaration 'S' here since the previous declaration has module 
linkage

which seems maybe to be more helpful to the user in telling them why.

I hope others can weigh in with an opinion here  .. @dblaikie @vsapsai  ?




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:7832-7833
 def err_redeclaration_non_exported : Error <
-  "cannot export redeclaration %0 here since the previous declaration is not "
-  "exported">;
+  "cannot export redeclaration %0 here since the previous declaration "
+  "%select{is not exported|has internal linkage|has module linkage}1">;
 def err_invalid_declarator_global_scope : Error<

ChuanqiXu wrote:
> I feel this change is not intended?
the change is intentional, but we should maybe decide if it is more or less 
helpful to the user.



Comment at: clang/lib/Sema/SemaDecl.cpp:1671-1677
+  auto Lk = Old->getFormalLinkage();
+  int S = 0;
+  if (Lk == Linkage::InternalLinkage)
+S = 1;
+  else if (Lk == Linkage::ModuleLinkage)
+S = 2;
+  Diag(New->getLocation(), diag::err_redeclaration_non_exported) << New << S;

ChuanqiXu wrote:
> It looks like that this change isn't reflected in the summary. Although this 
> is not bad, I feel it is not good. From the perspective of the user, I feel 
> the new message here is a little bit more confusing.
it is reflected in the summary:

"
This adjusts error messages for exports involving redeclarations in modules to
be more specific about the reason that the decl has been rejected.
"
Iet us discuss whether this is more/less useful outside of the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122119

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

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

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

balazske added inline comments.
Herald added a project: All.



Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.cpp:153
+diag(HandlerLambda->getBeginLoc(),
+ "lambda function is not allowed as signal handler (until C++17)")
+<< HandlerLambda->getSourceRange();

whisperity wrote:
> I am trying to find some source for this claim but so far hit a blank. 😦 
> Could you please elaborate where this information comes from? Most notably, 
> [[ https://en.cppreference.com/w/cpp/utility/program/signal | std::signal on 
> CppReference ]] makes no mention of this, which would be the first place I 
> would expect most people to look at.
> 
> Maybe this claim is derived from the rule that signal handlers **MUST** have 
> C linkage? (If so, is there a warning against people setting C++-linkage 
> functions as signal handlers in this check in general?)
This check is made to comply with a CERT rule [[ 
https://wiki.sei.cmu.edu/confluence/display/cplusplus/MSC54-CPP.+A+signal+handler+must+be+a+plain+old+function
 | MSC54-CPP ]]. According to this only the subset of C and C++ should be used 
in a signal handler, and it should have extern "C" linkage. This does not allow 
lambda functions because the linkage restriction (if not others). (From C++17 
on other rules apply, this check is not applicable for such code.)



Comment at: clang-tools-extra/clang-tidy/bugprone/SignalHandlerCheck.h:54
+ std::function ChainReporter);
+  /// Similar as `checkFunction` but only check for C++14 rules.
+  bool checkFunctionCPP14(const FunctionDecl *FD, const Expr *CallOrRef,

whisperity wrote:
> (Nit: to keep consistency with others and to ensure documentation renders 
> properly, use `@p checkFunction` instead of backtick.)
I see that the backtick character is used at other places. It should work with 
Doxygen. The `\c` is the normal Doxygen command to make code-style text format, 
`\p` is used for function parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118996

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

[clang] 2ddd57a - [clang][dataflow] Model the behavior of optional and std swap

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-03-22T08:35:34Z
New Revision: 2ddd57ae1ec42c4aad8e70645cff82c877a94e3f

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

LOG: [clang][dataflow] Model the behavior of optional and std swap

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

Reviewed-by: ymandel, xazax.hun

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 




diff  --git 
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp 
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 4d0b10435a094..bf325b04967c2 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -107,6 +107,12 @@ auto isOptionalNulloptAssignment() {
  hasArgument(1, hasNulloptType()));
 }
 
+auto isStdSwapCall() {
+  return callExpr(callee(functionDecl(hasName("std::swap"))),
+  argumentCountIs(2), hasArgument(0, hasOptionalType()),
+  hasArgument(1, hasOptionalType()));
+}
+
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
 /// symbolic value of its "has_value" property.
 StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
@@ -283,6 +289,50 @@ void transferNulloptAssignment(const CXXOperatorCallExpr 
*E,
   transferAssignment(E, State.Env.getBoolLiteralValue(false), State);
 }
 
+void transferSwap(const StorageLocation &OptionalLoc1,
+  const StorageLocation &OptionalLoc2,
+  LatticeTransferState &State) {
+  auto *OptionalVal1 = State.Env.getValue(OptionalLoc1);
+  assert(OptionalVal1 != nullptr);
+
+  auto *OptionalVal2 = State.Env.getValue(OptionalLoc2);
+  assert(OptionalVal2 != nullptr);
+
+  State.Env.setValue(OptionalLoc1, *OptionalVal2);
+  State.Env.setValue(OptionalLoc2, *OptionalVal1);
+}
+
+void transferSwapCall(const CXXMemberCallExpr *E,
+  const MatchFinder::MatchResult &,
+  LatticeTransferState &State) {
+  assert(E->getNumArgs() == 1);
+
+  auto *OptionalLoc1 = State.Env.getStorageLocation(
+  *E->getImplicitObjectArgument(), SkipPast::ReferenceThenPointer);
+  assert(OptionalLoc1 != nullptr);
+
+  auto *OptionalLoc2 =
+  State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference);
+  assert(OptionalLoc2 != nullptr);
+
+  transferSwap(*OptionalLoc1, *OptionalLoc2, State);
+}
+
+void transferStdSwapCall(const CallExpr *E, const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+  assert(E->getNumArgs() == 2);
+
+  auto *OptionalLoc1 =
+  State.Env.getStorageLocation(*E->getArg(0), SkipPast::Reference);
+  assert(OptionalLoc1 != nullptr);
+
+  auto *OptionalLoc2 =
+  State.Env.getStorageLocation(*E->getArg(1), SkipPast::Reference);
+  assert(OptionalLoc2 != nullptr);
+
+  transferSwap(*OptionalLoc1, *OptionalLoc2, State);
+}
+
 auto buildTransferMatchSwitch() {
   // FIXME: Evaluate the efficiency of matchers. If using matchers results in a
   // lot of duplicated work (e.g. string comparisons), consider providing APIs
@@ -361,6 +411,13 @@ auto buildTransferMatchSwitch() {
 State.Env.getBoolLiteralValue(false));
   })
 
+  // optional::swap
+  .CaseOf(isOptionalMemberCallWithName("swap"),
+ transferSwapCall)
+
+  // std::swap
+  .CaseOf(isStdSwapCall(), transferStdSwapCall)
+
   .Build();
 }
 

diff  --git 
a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index fbc17668447ff..76340aad5fd4d 100644
--- 
a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ 
b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -507,6 +507,9 @@ namespace std {
 template 
 constexpr remove_reference_t&& move(T&& x);
 
+template 
+void swap(T& a, T& b) noexcept;
+
 } // namespace std
 
 #endif // UTILITY_H
@@ -718,6 +721,8 @@ class optional : private __optional_storage_base<_Tp> {
 
   constexpr explicit operator bool() const noexcept;
   using __base::has_value;
+
+  constexpr void swap(optional& __opt) noexcept;
 };
 
 template 
@@ -938,6 +943,8 @@ class optional {
 
   constexpr explicit operator bool() const noexcept;
   constexpr bool has_value() const noexcept;
+
+  void swap(optional& rhs) noexcept;
 };
 
 template 
@@ -1129,6 +1136,8 @@ class Optional {
 
   constexpr explicit operator bool() const noexcept;
   constexpr b

[PATCH] D122129: [clang][dataflow] Model the behavior of optional and std swap

[Stanislav Gatev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2ddd57ae1ec4: [clang][dataflow] Model the behavior of 
optional and std swap (authored by sgatev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122129

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -507,6 +507,9 @@
 template 
 constexpr remove_reference_t&& move(T&& x);
 
+template 
+void swap(T& a, T& b) noexcept;
+
 } // namespace std
 
 #endif // UTILITY_H
@@ -718,6 +721,8 @@
 
   constexpr explicit operator bool() const noexcept;
   using __base::has_value;
+
+  constexpr void swap(optional& __opt) noexcept;
 };
 
 template 
@@ -938,6 +943,8 @@
 
   constexpr explicit operator bool() const noexcept;
   constexpr bool has_value() const noexcept;
+
+  void swap(optional& rhs) noexcept;
 };
 
 template 
@@ -1129,6 +1136,8 @@
 
   constexpr explicit operator bool() const noexcept;
   constexpr bool has_value() const noexcept;
+
+  void swap(Optional& other);
 };
 
 template 
@@ -1911,10 +1920,93 @@
   UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, OptionalSwap) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+void target() {
+  $ns::$optional opt1 = $ns::nullopt;
+  $ns::$optional opt2 = 3;
+
+  opt1.swap(opt2);
+
+  opt1.value();
+  /*[[check-1]]*/
+
+  opt2.value();
+  /*[[check-2]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-1", "safe"),
+   Pair("check-2", "unsafe: input.cc:13:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+void target() {
+  $ns::$optional opt1 = $ns::nullopt;
+  $ns::$optional opt2 = 3;
+
+  opt2.swap(opt1);
+
+  opt1.value();
+  /*[[check-3]]*/
+
+  opt2.value();
+  /*[[check-4]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-3", "safe"),
+   Pair("check-4", "unsafe: input.cc:13:7")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, StdSwap) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+void target() {
+  $ns::$optional opt1 = $ns::nullopt;
+  $ns::$optional opt2 = 3;
+
+  std::swap(opt1, opt2);
+
+  opt1.value();
+  /*[[check-1]]*/
+
+  opt2.value();
+  /*[[check-2]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-1", "safe"),
+   Pair("check-2", "unsafe: input.cc:13:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+void target() {
+  $ns::$optional opt1 = $ns::nullopt;
+  $ns::$optional opt2 = 3;
+
+  std::swap(opt2, opt1);
+
+  opt1.value();
+  /*[[check-3]]*/
+
+  opt2.value();
+  /*[[check-4]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-3", "safe"),
+   Pair("check-4", "unsafe: input.cc:13:7")));
+}
+
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
-// - swap
 // - invalidation (passing optional by non-const reference/pointer)
 // - `value_or(nullptr) != nullptr`, `value_or(0) != 0`, `value_or("").empty()`
 // - nested `optional` values
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -107,6 +107,12 @@
  hasArgument(1, hasNulloptType()));
 }
 
+auto isStdSwapCall() {
+  return callExpr(callee(functionDecl(hasName("std::swap"))),
+  argumentCountIs(2), hasArgument(0, hasOptionalType()),
+  hasArgument(1, hasOptionalType()));
+}
+
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
 /// symbolic value of its "has_value" property.
 StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
@@ -283,6 +289,50 @@
   transferAssignment(E, State.Env.getBoolLiteralValue(false), State);
 }
 
+void transferSwap(const StorageLocation &OptionalLoc1,
+  const StorageLocation &OptionalLoc2,
+  LatticeTransferState &State) {
+  auto *OptionalVal1 = State.Env.getValue(OptionalLoc1);
+  assert(OptionalVal1 != nullptr);
+
+  auto *OptionalVal2 = State.

[clang] a9656bd - [CodeGen][OpenMP] Make EmitLoadOfPointer() type consistent

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-03-22T09:37:48+01:00
New Revision: a9656bd1bc3792b81ad5a7277bdf7d8159b3b924

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

LOG: [CodeGen][OpenMP] Make EmitLoadOfPointer() type consistent

If necessary insert a bitcast beforehand, so the LLVM-level pointer
type and the Clang-level pointer type line up.

Added: 


Modified: 
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/test/OpenMP/target_data_use_device_addr_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 94e7e1a3f68bc..7ce3cd99c1b76 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -6949,16 +6949,13 @@ void CodeGenFunction::EmitOMPUseDeviceAddrClause(
 // For declrefs and variable length array need to load the pointer for
 // correct mapping, since the pointer to the data was passed to the 
runtime.
 if (isa(Ref->IgnoreParenImpCasts()) ||
-MatchingVD->getType()->isArrayType())
-  PrivAddr =
-  EmitLoadOfPointer(PrivAddr, getContext()
-  .getPointerType(OrigVD->getType())
-  ->castAs());
-llvm::Type *RealElTy =
-ConvertTypeForMem(OrigVD->getType().getNonReferenceType());
-llvm::Type *RealTy = RealElTy->getPointerTo();
-PrivAddr =
-Builder.CreatePointerBitCastOrAddrSpaceCast(PrivAddr, RealTy, 
RealElTy);
+MatchingVD->getType()->isArrayType()) {
+  QualType PtrTy = getContext().getPointerType(
+  OrigVD->getType().getNonReferenceType());
+  PrivAddr = EmitLoadOfPointer(
+  Builder.CreateElementBitCast(PrivAddr, ConvertTypeForMem(PtrTy)),
+  PtrTy->castAs());
+}
 
 (void)PrivateScope.addPrivate(OrigVD, PrivAddr);
   }

diff  --git a/clang/test/OpenMP/target_data_use_device_addr_codegen.cpp 
b/clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
index 9a3f7aa17b166..6bc1631c9ab86 100644
--- a/clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
+++ b/clang/test/OpenMP/target_data_use_device_addr_codegen.cpp
@@ -90,8 +90,8 @@ int main() {
 // CHECK: [[A_REF:%.+]] = load float*, float** [[BPTR0_A_ADDR]],
 // CHECK: [[REF_REF:%.+]] = load float*, float** [[BPTR2_REF_ADDR]],
 // CHECK: store float* [[REF_REF]], float** [[TMP_REF_ADDR:%.+]],
-// CHECK: [[ARR:%.+]] = load float*, float** [[BPTR3_ARR_ADDR]],
-// CHECK: [[ARR_REF:%.+]] = bitcast float* [[ARR]] to [4 x float]*
+// CHECK: [[BPTR3_ARR_ADDR_CAST:%.+]] = bitcast float** [[BPTR3_ARR_ADDR]] to 
[4 x float]**
+// CHECK: [[ARR_REF:%.+]] = load [4 x float]*, [4 x float]** 
[[BPTR3_ARR_ADDR_CAST]],
 // CHECK: [[VLA_REF:%.+]] = load float*, float** [[BPTR4_VLA_ADDR]],
 // CHECK: [[A:%.+]] = load float, float* [[A_REF]],
 // CHECK: [[INC:%.+]] = fadd float [[A]], 1.00e+00



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

[clang] 767ec88 - [CodeGen] Avoid deprecated Address ctor in EmitLoadOfPointer()

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-03-22T09:42:31+01:00
New Revision: 767ec883e37510a247ea5695921876ef67cf5b3f

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

LOG: [CodeGen] Avoid deprecated Address ctor in EmitLoadOfPointer()

This requires some adjustment in caller code, because there was
a confusion regarding the meaning of the PtrTy argument: This
argument is the type of the pointer being loaded, not the addresses
being loaded from.

Added: 


Modified: 
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
clang/lib/CodeGen/CodeGenFunction.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index b0eeddd98fce6..2ee62f97399ac 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -2547,10 +2547,10 @@ Address CodeGenFunction::EmitLoadOfPointer(Address Ptr,
LValueBaseInfo *BaseInfo,
TBAAAccessInfo *TBAAInfo) {
   llvm::Value *Addr = Builder.CreateLoad(Ptr);
-  return Address::deprecated(
-  Addr,
-  CGM.getNaturalTypeAlignment(PtrTy->getPointeeType(), BaseInfo, TBAAInfo,
-  /*forPointeeType=*/true));
+  return Address(Addr, ConvertTypeForMem(PtrTy->getPointeeType()),
+ CGM.getNaturalTypeAlignment(PtrTy->getPointeeType(), BaseInfo,
+ TBAAInfo,
+ /*forPointeeType=*/true));
 }
 
 LValue CodeGenFunction::EmitLoadOfPointerLValue(Address PtrAddr,

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index f1439f20f50cc..4e0c682723e50 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -4650,8 +4650,7 @@ CGOpenMPRuntime::getDepobjElements(CodeGenFunction &CGF, 
LValue DepobjLVal,
   RecordDecl *KmpDependInfoRD =
   cast(KmpDependInfoTy->getAsTagDecl());
   LValue Base = CGF.EmitLoadOfPointerLValue(
-  DepobjLVal.getAddress(CGF),
-  C.getPointerType(C.VoidPtrTy).castAs());
+  DepobjLVal.getAddress(CGF), C.VoidPtrTy.castAs());
   QualType KmpDependInfoPtrTy = C.getPointerType(KmpDependInfoTy);
   Address Addr = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
   Base.getAddress(CGF), CGF.ConvertTypeForMem(KmpDependInfoPtrTy),
@@ -4748,8 +4747,7 @@ emitDepobjElementsSizes(CodeGenFunction &CGF, QualType 
&KmpDependInfoTy,
 for (const Expr *E : Data.DepExprs) {
   LValue DepobjLVal = CGF.EmitLValue(E->IgnoreParenImpCasts());
   LValue Base = CGF.EmitLoadOfPointerLValue(
-  DepobjLVal.getAddress(CGF),
-  C.getPointerType(C.VoidPtrTy).castAs());
+  DepobjLVal.getAddress(CGF), C.VoidPtrTy.castAs());
   Address Addr = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
   Base.getAddress(CGF), KmpDependInfoPtrT,
   CGF.ConvertTypeForMem(KmpDependInfoTy));
@@ -4806,8 +4804,7 @@ static void emitDepobjElements(CodeGenFunction &CGF, 
QualType &KmpDependInfoTy,
   const Expr *E = Data.DepExprs[I];
   LValue DepobjLVal = CGF.EmitLValue(E->IgnoreParenImpCasts());
   LValue Base = CGF.EmitLoadOfPointerLValue(
-  DepobjLVal.getAddress(CGF),
-  C.getPointerType(C.VoidPtrTy).castAs());
+  DepobjLVal.getAddress(CGF), C.VoidPtrTy.castAs());
   Address Addr = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
   Base.getAddress(CGF), KmpDependInfoPtrT,
   CGF.ConvertTypeForMem(KmpDependInfoTy));
@@ -5055,8 +5052,7 @@ void CGOpenMPRuntime::emitDestroyClause(CodeGenFunction 
&CGF, LValue DepobjLVal,
   QualType FlagsTy;
   getDependTypes(C, KmpDependInfoTy, FlagsTy);
   LValue Base = CGF.EmitLoadOfPointerLValue(
-  DepobjLVal.getAddress(CGF),
-  C.getPointerType(C.VoidPtrTy).castAs());
+  DepobjLVal.getAddress(CGF), C.VoidPtrTy.castAs());
   QualType KmpDependInfoPtrTy = C.getPointerType(KmpDependInfoTy);
   Address Addr = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
   Base.getAddress(CGF), CGF.ConvertTypeForMem(KmpDependInfoPtrTy),
@@ -6038,8 +6034,7 @@ static llvm::Value *emitReduceFiniFunction(CodeGenModule 
&CGM,
   CodeGenFunction CGF(CGM);
   CGF.StartFunction(GlobalDecl(), C.VoidTy, Fn, FnInfo, Args, Loc, Loc);
   Address PrivateAddr = CGF.EmitLoadOfPointer(
-  CGF.GetAddrOfLocalVar(&Param),
-  C.getPointerType(C.VoidPtrTy).castAs());
+  CGF.GetAddrOfLocalVar(&Param), C.VoidPtrTy.castAs());
   llvm::Value *Size = nullptr;
   // If the size of the reduction item is non-constant, load it from global
   // threadprivate variable.

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp 
b/clang/lib/CodeGen/CG

[PATCH] D122199: [NFC][Clang][OpaquePtr] Remove calls to Address::deprecated in TargetInfo.cpp

[Nikita Popov via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122199

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

[PATCH] D122215: [WebAssembly] Initial support for reference types in clang

[Paulo Matos via Phabricator via cfe-commits]

pmatos created this revision.
pmatos added reviewers: tlively, asb.
Herald added subscribers: StephenFan, wingo, dexonsmith, ecnelises, arphaman, 
martong, sunfish, hiraditya, jgravelle-google, sbc100, dschuff.
Herald added a reviewer: shafik.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
pmatos requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, aheejin.
Herald added projects: clang, LLVM.

This patch defines a builtin type externref and represents a
funcref as a function pointer with the attribute funcref. As a test
it implements builtins __builtin_wasm_ref_null_func() and
__builtin_wasm_ref_null_extern(). Considerations about alloca address
spaces will come in a followup patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122215

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeProperties.td
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/include/clang/Basic/WebAssemblyReferenceTypes.def
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/module.modulemap
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/NSAPI.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/test/CodeGen/WebAssembly/wasm-externref.c
  clang/test/CodeGen/WebAssembly/wasm-funcref.c
  clang/test/CodeGen/builtins-wasm.c
  clang/test/CodeGenCXX/wasm-reftypes-mangle.cpp
  clang/test/CodeGenCXX/wasm-reftypes-typeinfo.cpp
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/IR/Type.h
  llvm/lib/CodeGen/ValueTypes.cpp
  llvm/lib/IR/Type.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
@@ -403,11 +403,17 @@
 }
 
 //===--===//
-// The following functions are called from lib/CodeGen/Passes.cpp to modify
-// the CodeGen pass sequence.
+// The following functions are called from lib/CodeGen/TargetPassConfig.cpp
+// to modify the CodeGen pass sequence.
 //===--===//
 
 void WebAssemblyPassConfig::addIRPasses() {
+  // Run mem2reg to remove alloca references - needed for reference types
+  // FIXME: this should only be added when the subtarget has reference types 
+  // enabled but the subtarget is dependent on the function being compiled to 
+  // which we don't have access atm.
+  addPass(createPromoteMemoryToRegisterPass());
+
   // Add signatures to prototype-less function declarations
   addPass(createWebAssemblyAddMissingPrototypes());
 
Index: llvm/lib/IR/Type.cpp
===
--- llvm/lib/IR/Type.cpp
+++ llvm/lib/IR/Type.cpp
@@ -304,6 +304,18 @@
   return getInt64Ty(C)->getPointerTo(AS);
 }
 
+Type *Type::getWasm_ExternrefTy(LLVMContext &C) {
+  // pointer to opaque struct in addrspace(10)
+  static PointerType *Ty = PointerType::get(StructType::get(C), 10);
+  return Ty;
+}
+
+Type *Type::getWasm_FuncrefTy(LLVMContext &C) {
+  // pointer to i8 addrspace(20)
+  static PointerType *Ty = PointerType::get(Type::getInt8Ty(C), 20);
+  return Ty;
+}
+
 //===--===//
 //   IntegerType Implementation
 //===--===//
Index: llvm/lib/CodeGen/ValueTypes.cpp
===
--- llvm/lib/CodeGen/ValueTypes.cpp
+++ llvm/lib/CodeGen/ValueTypes.cpp
@@ -200,12 +200,8 @@
   case MVT::x86mmx:  return Type::getX86_MMXTy(Context);
   case MVT::x86amx:  return Type::getX86_AMXTy(Context);
   case MVT::i64x8:   return IntegerType::get(Context, 512);
-  case MVT::externref:
-// pointer to opaque struct in addrspace(10)
-return PointerType::get(StructType::create(Context), 10);
-  case MVT::funcref:
-// pointer to i8 addrspace(20)
-return PointerType::get(Type::getInt8Ty(Context), 20);
+  case MVT::externref: return Type::getWasm_ExternrefTy(Context);
+  case M

[PATCH] D122215: [WebAssembly] Initial support for reference types in clang

[Paulo Matos via Phabricator via cfe-commits]

pmatos planned changes to this revision.
pmatos added a comment.

Test `wasm-funcref.c` still fails as type attribute `funcref` semantics are not 
yet implemented. Not ready to land yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122215

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

[PATCH] D118493: Set rpath on openmp executables

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a subscriber: estewart08.
JonChesterfield added a comment.

@estewart08 Fedora are rejecting some uses of rpath (and probably runpath). I 
can't find a corresponding page for redhat. This may become a problem for our 
aomp/rocm builds.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118493

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

[PATCH] D121824: [clang] Do not crash on arrow operator on dependent type.

[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, looks good!

please update the description of the patch accordingly (saying we invalidate 
the vardecl)




Comment at: clang/test/SemaCXX/arrow-operator.cpp:79
+void foo() {
+  // x is dependent.
+  A& x = blah[7];  // expected-error {{use of undeclared identifier 
'blah'}}

I think the comment `x is dependent` should associate to `x->call()` statement 
nor the vardecl.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121824

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

[clang] b8f0e12 - [CodeGen] Remove some uses of deprecated Address constructor

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-03-22T10:02:35+01:00
New Revision: b8f0e12847f5df0792c816c995bc67759aea669e

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

LOG: [CodeGen] Remove some uses of deprecated Address constructor

Remove two stray uses in CodeGenModule and CGCUDANV.

Added: 


Modified: 
clang/lib/CodeGen/CGCUDANV.cpp
clang/lib/CodeGen/CodeGenModule.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGCUDANV.cpp b/clang/lib/CodeGen/CGCUDANV.cpp
index b832c686b8b69..3ae152d743206 100644
--- a/clang/lib/CodeGen/CGCUDANV.cpp
+++ b/clang/lib/CodeGen/CGCUDANV.cpp
@@ -965,9 +965,9 @@ llvm::Function *CGNVCUDARuntime::makeModuleDtorFunction() {
   CGBuilderTy DtorBuilder(CGM, Context);
   DtorBuilder.SetInsertPoint(DtorEntryBB);
 
-  Address GpuBinaryAddr = Address::deprecated(
-  GpuBinaryHandle,
-  CharUnits::fromQuantity( GpuBinaryHandle->getAlignment()));
+  Address GpuBinaryAddr(
+  GpuBinaryHandle, GpuBinaryHandle->getValueType(),
+  CharUnits::fromQuantity(GpuBinaryHandle->getAlignment()));
   auto *HandleValue = DtorBuilder.CreateLoad(GpuBinaryAddr);
   // There is only one HIP fat binary per linked module, however there are
   // multiple destructor functions. Make sure the fat binary is unregistered

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index d07a4770a2715..c90e042759f08 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -6456,8 +6456,9 @@ void CodeGenModule::EmitOMPThreadPrivateDecl(const 
OMPThreadPrivateDecl *D) {
 !VD->getAnyInitializer()->isConstantInitializer(getContext(),
 /*ForRef=*/false);
 
-Address Addr = Address::deprecated(GetAddrOfGlobalVar(VD),
-   getContext().getDeclAlign(VD));
+Address Addr(GetAddrOfGlobalVar(VD),
+ getTypes().ConvertTypeForMem(VD->getType()),
+ getContext().getDeclAlign(VD));
 if (auto InitFunction = getOpenMPRuntime().emitThreadPrivateVarDefinition(
 VD, Addr, RefExpr->getBeginLoc(), PerformInit))
   CXXGlobalInits.push_back(InitFunction);



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

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D122119#3398823 , @iains wrote:

> So the adjustment to the error message is something I am 50/50 about (IMO it 
> makes some messages more useful, but maybe not needed in others).
>
> Without the change we get 
> "cannot export redeclaration 'xxx' here since the previous declaration is not 
> exported"
>
> So, e.g in C++20 10.2 example 6. every case has the same error message (which 
> was what prompted me to make the change).
>
> With the change here we now get:
> cannot export redeclaration 'f' here since the previous declaration has 
> internal linkage
> cannot export redeclaration 'S' here since the previous declaration has 
> module linkage
>
> which seems maybe to be more helpful to the user in telling them why.
>
> I hope others can weigh in with an opinion here  .. @dblaikie @vsapsai  ?

I am OK to wait for opinions from others. Let me talk it a little bit more.

The first feeling I saw the change is that not every C++ programmer knows about 
linkage. OK, it depends on the environment really and every one might has their 
own opinion.

Another thought is that 10.2.6 (http://eel.is/c++draft/module.interface#6) 
doesn't talk anything about linkage:

> A redeclaration of an entity X is implicitly exported if X was introduced by 
> an exported declaration; otherwise it shall not be exported.

So it looks like confusing to talk about linkage this time. In my imagination, 
there might be a such situation:

A programmer met the error when he tries to export a redeclaration which is 
internal linkage (maybe a simple const variable). Then the message told him the 
internal linkage is not allowed to re-export. Then he removes the const 
specifier. Now he meets the error again. It tells that we couldn't export 
redeclaration which is module linkage. I guess he would feel bad. Then he might 
try to export the first declaration to get passed. However, the `const` 
specifier is lost in the case. And in the current message, I guess he would add 
export to the first declaration directly after he reads the message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122119

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

[clang-tools-extra] 3e7a8aa - [clang-tidy] Don't try to build CTTestTidyModule for Windows with dylibs

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2022-03-22T11:08:32+02:00
New Revision: 3e7a8aab759a9bdf90f35a60ea6e96ec64e36cb9

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

LOG: [clang-tidy] Don't try to build CTTestTidyModule for Windows with dylibs

In MinGW mode, it's possible to build LLVM/Clang with
LLVM_LINK_LLVM_DYLIB (which implicitly enables plugins too). Other
existing ways of building plugins on Windows is to build with
LLVM_EXPORT_SYMBOLS_FOR_PLUGINS, where each executable exports its
symbols.

With LLVM_LINK_LLVM_DYLIB, we can't generally skip building plugins
even if they are set up with PLUGIN_TOOL, as some plugins (e.g.
under clang/examples) set up that way do build properly (as
they manually call clang_target_link_libraries, which links in the
libclang-cpp.dll dylib).

For CTTestTidyModule, there's no corresponding dylib that would
provide the same exports.

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

Added: 


Modified: 
clang-tools-extra/test/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/test/CMakeLists.txt 
b/clang-tools-extra/test/CMakeLists.txt
index 170e5f8bd197d..26aece9cf337e 100644
--- a/clang-tools-extra/test/CMakeLists.txt
+++ b/clang-tools-extra/test/CMakeLists.txt
@@ -81,11 +81,13 @@ foreach(dep ${LLVM_UTILS_DEPS})
 endforeach()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+  endif()
 
   if(CLANG_BUILT_STANDALONE)
 # LLVMHello library is needed below



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

[PATCH] D121687: [clang-tidy] Don't try to build CTTestTidyModule for Windows with dylibs

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e7a8aab759a: [clang-tidy] Don't try to build 
CTTestTidyModule for Windows with dylibs (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121687

Files:
  clang-tools-extra/test/CMakeLists.txt


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -81,11 +81,13 @@
 endforeach()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+  endif()
 
   if(CLANG_BUILT_STANDALONE)
 # LLVMHello library is needed below


Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -81,11 +81,13 @@
 endforeach()
 
 if (NOT LLVM_INSTALL_TOOLCHAIN_ONLY)
-  llvm_add_library(
-  CTTestTidyModule
-  MODULE clang-tidy/CTTestTidyModule.cpp
-  PLUGIN_TOOL clang-tidy
-  DEPENDS clang-tidy-headers)
+  if (NOT WIN32 AND NOT LLVM_LINK_LLVM_DYLIB)
+llvm_add_library(
+CTTestTidyModule
+MODULE clang-tidy/CTTestTidyModule.cpp
+PLUGIN_TOOL clang-tidy
+DEPENDS clang-tidy-headers)
+  endif()
 
   if(CLANG_BUILT_STANDALONE)
 # LLVMHello library is needed below
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 2 inline comments as done.
iains added a comment.

In D122119#3398949 , @ChuanqiXu wrote:

> In D122119#3398823 , @iains wrote:
>
>> So the adjustment to the error message is something I am 50/50 about (IMO it 
>> makes some messages more useful, but maybe not needed in others).
>>
>> Without the change we get 
>> "cannot export redeclaration 'xxx' here since the previous declaration is 
>> not exported"
>>
>> So, e.g in C++20 10.2 example 6. every case has the same error message 
>> (which was what prompted me to make the change).
>>
>> With the change here we now get:
>> cannot export redeclaration 'f' here since the previous declaration has 
>> internal linkage
>> cannot export redeclaration 'S' here since the previous declaration has 
>> module linkage
>>
>> which seems maybe to be more helpful to the user in telling them why.
>>
>> I hope others can weigh in with an opinion here  .. @dblaikie @vsapsai  ?
>
> I am OK to wait for opinions from others. Let me talk it a little bit more.
>
> The first feeling I saw the change is that not every C++ programmer knows 
> about linkage. OK, it depends on the environment really and every one might 
> has their own opinion.
>
> Another thought is that 10.2.6 (http://eel.is/c++draft/module.interface#6) 
> doesn't talk anything about linkage:
>
>> A redeclaration of an entity X is implicitly exported if X was introduced by 
>> an exported declaration; otherwise it shall not be exported.
>
> So it looks like confusing to talk about linkage this time. In my 
> imagination, there might be a such situation:
>
> A programmer met the error when he tries to export a redeclaration which is 
> internal linkage (maybe a simple const variable). Then the message told him 
> the internal linkage is not allowed to re-export. Then he removes the const 
> specifier. Now he meets the error again. It tells that we couldn't export 
> redeclaration which is module linkage. I guess he would feel bad. Then he 
> might try to export the first declaration to get passed. However, the `const` 
> specifier is lost in the case. And in the current message, I guess he would 
> add export to the first declaration directly after he reads the message.

yes, that seems like a reasonable counter argument, and perhaps talking about 
linkage in a user message is a bit "implementor speak".. 
...  as I said, I was kind of 50/50 about this - I'll wait 48h for any other 
comments and then remove the change if there are no votes in favour.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122119

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

[clang] f42b195 - [CodeGen][RISCV] Avoid deprecated address constructor

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-03-22T10:15:19+01:00
New Revision: f42b1954a0e85fadd421d5e345cb8343c410b34d

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

LOG: [CodeGen][RISCV] Avoid deprecated address constructor

Added: 


Modified: 
clang/include/clang/Basic/riscv_vector.td

Removed: 




diff  --git a/clang/include/clang/Basic/riscv_vector.td 
b/clang/include/clang/Basic/riscv_vector.td
index 7f0ad2ee20996..556f6c244cee5 100644
--- a/clang/include/clang/Basic/riscv_vector.td
+++ b/clang/include/clang/Basic/riscv_vector.td
@@ -641,8 +641,8 @@ multiclass RVVVLEFFBuiltin types> {
 // Store new_vl.
 clang::CharUnits Align =
 CGM.getNaturalPointeeTypeAlignment(E->getArg(1)->getType());
-Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {1}),
-Address::deprecated(NewVL, Align));
+llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {1});
+Builder.CreateStore(Val, Address(NewVL, Val->getType(), Align));
 return V;
   }
   }],
@@ -661,8 +661,8 @@ multiclass RVVVLEFFBuiltin types> {
 // Store new_vl.
 clang::CharUnits Align =
 CGM.getNaturalPointeeTypeAlignment(E->getArg(3)->getType());
-Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {1}),
-Address::deprecated(NewVL, Align));
+llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {1});
+Builder.CreateStore(Val, Address(NewVL, Val->getType(), Align));
 return V;
   }
   }] in {
@@ -879,8 +879,8 @@ multiclass RVVUnitStridedSegLoad {
   CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
   llvm::Value *V;
   for (unsigned I = 0; I < NF; ++I) {
-V = Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {I}),
-Address::deprecated(Ops[I], Align));
+llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {I});
+V = Builder.CreateStore(Val, Address(Ops[I], Val->getType(), Align));
   }
   return V;
 }
@@ -905,8 +905,8 @@ multiclass RVVUnitStridedSegLoad {
   CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
   llvm::Value *V;
   for (unsigned I = 0; I < NF; ++I) {
-V = Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {I}),
-Address::deprecated(Ops[I], Align));
+llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {I});
+V = Builder.CreateStore(Val, Address(Ops[I], Val->getType(), Align));
   }
   return V;
 }
@@ -950,12 +950,12 @@ multiclass RVVUnitStridedSegLoadFF {
   clang::CharUnits Align =
   CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
   for (unsigned I = 0; I < NF; ++I) {
-Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {I}),
-Address::deprecated(Ops[I], Align));
+llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {I});
+Builder.CreateStore(Val, Address(Ops[I], Val->getType(), Align));
   }
   // Store new_vl.
-  return Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {NF}),
- Address::deprecated(NewVL, Align));
+  llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {NF});
+  return Builder.CreateStore(Val, Address(NewVL, Val->getType(), Align));
 }
 }],
 ManualCodegenMask = [{
@@ -978,12 +978,12 @@ multiclass RVVUnitStridedSegLoadFF {
   clang::CharUnits Align =
   CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
   for (unsigned I = 0; I < NF; ++I) {
-Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {I}),
-Address::deprecated(Ops[I], Align));
+llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {I});
+Builder.CreateStore(Val, Address(Ops[I], Val->getType(), Align));
   }
   // Store new_vl.
-  return Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {NF}),
- Address::deprecated(NewVL, Align));
+  llvm::Value *Val = Builder.CreateExtractValue(LoadValue, {NF});
+  return Builder.CreateStore(Val, Address(NewVL, Val->getType(), Align));
 }
 }] in {
   defvar PV = PVString.S;
@@ -1025,8 +1025,8 @@ multiclass RVVStridedSegLoad {
   CGM.getNaturalPointeeTypeAlignment(E->getArg(0)->getType());
   llvm::Value *V;
   for (unsigned I = 0; I < NF; ++I) {
-V = Builder.CreateStore(Builder.CreateExtractValue(LoadValue, {I}),
-Address::deprecated(Ops[I], Align));
+llvm::Value *Val = Builder.CreateEx

[clang] 9ab18cc - [RISCV] Add policy operand for masked vid and viota IR intrinsics.

[Zakk Chen via cfe-commits]

Author: Zakk Chen
Date: 2022-03-22T02:32:31-07:00
New Revision: 9ab18cc535379c3442bf52e21fbe21c92eb0fd60

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

LOG: [RISCV] Add policy operand for masked vid and viota IR intrinsics.

Reviewed By: rogfer01

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

Added: 


Modified: 
clang/include/clang/Basic/riscv_vector.td
llvm/include/llvm/IR/IntrinsicsRISCV.td
llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
llvm/test/CodeGen/RISCV/rvv/masked-tama.ll
llvm/test/CodeGen/RISCV/rvv/masked-tamu.ll
llvm/test/CodeGen/RISCV/rvv/masked-tuma.ll
llvm/test/CodeGen/RISCV/rvv/masked-tumu.ll
llvm/test/CodeGen/RISCV/rvv/vid.ll
llvm/test/CodeGen/RISCV/rvv/viota.ll

Removed: 




diff  --git a/clang/include/clang/Basic/riscv_vector.td 
b/clang/include/clang/Basic/riscv_vector.td
index 556f6c244cee5..6e4dff801dafb 100644
--- a/clang/include/clang/Basic/riscv_vector.td
+++ b/clang/include/clang/Basic/riscv_vector.td
@@ -2007,6 +2007,7 @@ def vmsif : RVVMaskUnaryBuiltin;
 
 // 16.6. vmsof.m set-only-first mask bit
 def vmsof : RVVMaskUnaryBuiltin;
+}
 
 let NoMaskPolicy = HasPassthruOperand, HasNoMaskedOverloaded = false in {
   // 16.8. Vector Iota Instruction
@@ -2016,7 +2017,6 @@ let NoMaskPolicy = HasPassthruOperand, 
HasNoMaskedOverloaded = false in {
   defm vid : RVVOutBuiltinSet<"vid", "csil", [["v", "v", "v"],
   ["v", "Uv", "Uv"]]>;
 }
-}
 
 // 17. Vector Permutation Instructions
 // 17.1. Integer Scalar Move Instructions

diff  --git a/llvm/include/llvm/IR/IntrinsicsRISCV.td 
b/llvm/include/llvm/IR/IntrinsicsRISCV.td
index 9c8a320d11023..7ac157d48bb85 100644
--- a/llvm/include/llvm/IR/IntrinsicsRISCV.td
+++ b/llvm/include/llvm/IR/IntrinsicsRISCV.td
@@ -816,7 +816,7 @@ let TargetPrefix = "riscv" in {
   }
   // Output: (vector)
   // Input: (passthru, vl)
-  class RISCVNullaryIntrinsicTU
+  class RISCVID
 : Intrinsic<[llvm_anyvector_ty],
 [LLVMMatchType<0>, llvm_anyint_ty],
 [IntrNoMem]>, RISCVVIntrinsic {
@@ -1460,26 +1460,26 @@ let TargetPrefix = "riscv" in {
 let VLOperand = 2;
   }
   // Output: (vector)
-  // Input: (maskedoff, mask type vector_in, mask, vl)
+  // Input: (maskedoff, mask type vector_in, mask, vl, policy)
   def int_riscv_viota_mask : Intrinsic<[llvm_anyvector_ty],
[LLVMMatchType<0>,
 LLVMScalarOrSameVectorWidth<0, 
llvm_i1_ty>,
 LLVMScalarOrSameVectorWidth<0, 
llvm_i1_ty>,
-llvm_anyint_ty],
-   [IntrNoMem]>, RISCVVIntrinsic {
+llvm_anyint_ty, LLVMMatchType<1>],
+   [ImmArg>, IntrNoMem]>, 
RISCVVIntrinsic {
 let VLOperand = 3;
   }
   // Output: (vector)
   // Input: (passthru, vl)
-  def int_riscv_vid : RISCVNullaryIntrinsicTU;
+  def int_riscv_vid : RISCVID;
 
   // Output: (vector)
-  // Input: (maskedoff, mask, vl)
+  // Input: (maskedoff, mask, vl, policy)
   def int_riscv_vid_mask : Intrinsic<[llvm_anyvector_ty],
  [LLVMMatchType<0>,
   LLVMScalarOrSameVectorWidth<0, 
llvm_i1_ty>,
-  llvm_anyint_ty],
- [IntrNoMem]>, RISCVVIntrinsic {
+  llvm_anyint_ty, LLVMMatchType<1>],
+ [ImmArg>, IntrNoMem]>, 
RISCVVIntrinsic {
 let VLOperand = 2;
   }
 

diff  --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td 
b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
index 406d30ac73360..60a09db4efd5e 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
@@ -907,7 +907,7 @@ class VPseudoNullaryNoMaskTU:
 class VPseudoNullaryMask:
   Pseudo<(outs GetVRegNoV0.R:$rd),
  (ins GetVRegNoV0.R:$merge, VMaskOp:$vm, AVL:$vl,
-  ixlenimm:$sew), []>, RISCVVPseudo {
+  ixlenimm:$sew, ixlenimm:$policy), []>, RISCVVPseudo {
   let mayLoad = 0;
   let mayStore = 0;
   let hasSideEffects = 0;
@@ -916,6 +916,7 @@ class VPseudoNullaryMask:
   let HasSEWOp = 1;
   let HasMergeOp = 1;
   let UsesMaskPolicy = 1;
+  let HasVecPolicyOp = 1;
   let BaseInstr = !cast(PseudoToVInst.VInst);
 }
 
@@ -1730,7 +1731,7 @@ multiclass VPseudoVIOT_M {
Sched<[WriteVMIotV, ReadVMIotV, ReadVMask]>;
   def "_" # m.MX # "_TU" : VPseudoUnaryNoMaskTU,
Sched<[WriteVMIotV, ReadVMIotV, ReadVMask]>;
-  

[PATCH] D120227: [RISCV] Add policy operand for masked vid and viota IR intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9ab18cc53537: [RISCV] Add policy operand for masked vid and 
viota IR intrinsics. (authored by khchen).
Herald added subscribers: s, StephenFan, arichardson.
Herald added a project: All.

Changed prior to commit:
  https://reviews.llvm.org/D120227?vs=410209&id=417223#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120227

Files:
  clang/include/clang/Basic/riscv_vector.td
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/masked-tama.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tamu.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tuma.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tumu.ll
  llvm/test/CodeGen/RISCV/rvv/vid.ll
  llvm/test/CodeGen/RISCV/rvv/viota.ll

Index: llvm/test/CodeGen/RISCV/rvv/viota.ll
===
--- llvm/test/CodeGen/RISCV/rvv/viota.ll
+++ llvm/test/CodeGen/RISCV/rvv/viota.ll
@@ -27,7 +27,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv1i8_nxv1i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv1i8_nxv1i1:
@@ -40,7 +40,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -69,7 +69,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv2i8_nxv2i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv2i8_nxv2i1:
@@ -82,7 +82,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -111,7 +111,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv4i8_nxv4i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv4i8_nxv4i1:
@@ -124,7 +124,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -153,7 +153,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv8i8_nxv8i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv8i8_nxv8i1:
@@ -166,7 +166,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -195,7 +195,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv16i8_nxv16i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv16i8_nxv16i1:
@@ -208,7 +208,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -237,7 +237,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv32i8_nxv32i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv32i8_nxv32i1:
@@ -250,7 +250,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -279,7 +279,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv64i8_nxv64i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv64i8_nxv64i1:
@@ -292,7 +292,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -321,7 +321,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv1i16_nxv1i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv1i16_nxv1i1:
@@ -334,7 +334,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -363,7 +363,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv2i16_nxv2i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv2i16_nxv2i1:
@@ -376,7 +376,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -405,7 +405,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv4i16_nxv4i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv4i16_nxv4i1:
@@ -418,7 +418,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -447,7 +447,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv8i16_nxv8i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv8i16_nxv8i1:
@@ -460,7 +460,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -489,7 +489,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv16i16_nxv16i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv16i16_nxv16i1:
@@ -502,7 +502,7 @@
  %0,
  %1,
  %1,
-iXLen %2)
+iXLen %2, iXLen 0)
 
   ret  %a
 }
@@ -531,7 +531,7 @@
   ,
   ,
   ,
-  iXLen);
+  iXLen, iXLen);
 
 define  @intrinsic_viota_mask_m_nxv32i16_nxv32i1( %0,  %1, iXLen %2) nounwind {
 ; CHECK-LABEL: intrinsic_viota_mask_m_nxv

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

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

balazske added reviewers: martong, steakhal, NoQ.
balazske added a comment.
Herald added a subscriber: rnkovacs.

This checker is made to add a partial support for CERT rule ERR30-C 
 . 
One part of the rule is "check errno only after the function returns a value 
indicating failure".

To make this check possible the function (one that sets //errno// in some way) 
should be modeled by another checker that knows when a failure-indication value 
is returned from the function. In (but only in) that case the function sets 
value of //errno//. Return value of the function call should be constrained by 
the modeling checker to the failure-indicating values if the errno value is 
set, otherwise to some other values (a state split is needed).

The new API allows to set the //errno value// only together with an "errno 
check state". This state indicates how to handle the //errno value// by the 
ErrnoChecker. This information is available at the modeling of the 
errno-setting function. The CERT rule specifies classes of functions, including 
"functions that set errno and return an out-of-band error indicator" and "set 
errno and return an in-band error indicator". At the out-of-band case the errno 
value is not required to be checked, failure can be observed by check of the 
return value. At the in-band case the return value at failure is a valid return 
value too, here errno must be checked to observe if the function has failed. 
This case is modeled by the `Errno_MustBeChecked` //errno check state//. At 
many functions value of errno may be undefined after the function call if the 
function has not failed (the function is not required to not change errno), 
this is modeled by the `Errno_MustNotBeChecked` value.


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] D121756: [clang-format] Clean up code looking for if statements

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

> It turned out this patch does change behavior.

i.e. now your function is considering more things, the formatted code is 
different, do you think the new formatting is more correct? (I mean it feels 
like it should be right? assuming we want to consider whiles in the same way we 
consider if and for  (which seems fairly reasonable, i guess)

You have to capture that in the unit tests for sure, but also can you work out 
what it was not doing? what it that it wasn't considering the while? should it 
have?

This is kind of the problem about refactoring without a github issue to justify 
why you are making the changes in the first place. If you had/can find a bug 
saying "While loop doesn't format correctly" then it would be easier to justify.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[clang-tools-extra] 1579090 - Reland "[pseudo] Split greatergreater token."

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-03-22T10:27:52+01:00
New Revision: 1579090141c5cc061f3a0b62cd92bd93802ddcf7

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

LOG: Reland "[pseudo] Split greatergreater token."

It was reverted, because the test had a lift-time issue.
Reland f66d3758bda99e9f57bfdad168212feda18792ae with a fix.

Added: 


Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/Token.h
clang-tools-extra/pseudo/lib/Lex.cpp
clang-tools-extra/pseudo/lib/cxx.bnf
clang-tools-extra/pseudo/unittests/TokenTest.cpp

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/Token.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/Token.h
index 24b6729151e61..4563477b2c4fe 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/Token.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/Token.h
@@ -180,7 +180,8 @@ enum class LexFlags : uint8_t {
   NeedsCleaning = 1 << 1,
 };
 
-/// Derives a token stream by decoding escapes and interpreting 
raw_identifiers.
+/// Derives a token stream by decoding escapes, interpreting raw_identifiers 
and
+/// splitting the greatergreater token.
 ///
 /// Tokens containing UCNs, escaped newlines, trigraphs etc are decoded and
 /// their backing data is owned by the returned stream.

diff  --git a/clang-tools-extra/pseudo/lib/Lex.cpp 
b/clang-tools-extra/pseudo/lib/Lex.cpp
index f5a239533c532..e99bf3a63e5e1 100644
--- a/clang-tools-extra/pseudo/lib/Lex.cpp
+++ b/clang-tools-extra/pseudo/lib/Lex.cpp
@@ -98,9 +98,21 @@ TokenStream cook(const TokenStream &Code, const LangOptions 
&LangOpts) {
   Tok.Length = Text.size();
   Tok.Flags &= ~static_cast(LexFlags::NeedsCleaning);
 }
-// Cook raw_identifiers into identifier, keyword, etc.
-if (Tok.Kind == tok::raw_identifier)
+
+if (Tok.Kind == tok::raw_identifier) {
+  // Cook raw_identifiers into identifier, keyword, etc.
   Tok.Kind = Identifiers.get(Tok.text()).getTokenID();
+} else if (Tok.Kind == tok::greatergreater) {
+  // Split the greatergreater token.
+  // FIXME: split lessless token to support Cuda triple angle brackets <<<.
+  assert(Tok.text() == ">>");
+  Tok.Kind = tok::greater;
+  Tok.Length = 1;
+  Result.push(Tok);
+  // Line is wrong if the first greater is followed by an escaped newline!
+  Tok.Data = Tok.text().data() + 1;
+}
+
 Result.push(std::move(Tok));
   }
 

diff  --git a/clang-tools-extra/pseudo/lib/cxx.bnf 
b/clang-tools-extra/pseudo/lib/cxx.bnf
index 48bf4621eefe5..cf664b8e13e55 100644
--- a/clang-tools-extra/pseudo/lib/cxx.bnf
+++ b/clang-tools-extra/pseudo/lib/cxx.bnf
@@ -13,6 +13,9 @@
 #  - the file merely describes the core C++ grammar. Preprocessor directives 
and
 #lexical conversions are omitted as we reuse clang's lexer and run a fake
 #preprocessor;
+#  - grammar rules with the >> token are adjusted, the greatergreater token is
+#split into two > tokens, to make the GLR parser aware of nested templates
+#and right shift operator;
 #
 # Guidelines:
 #   - nonterminals are lower_case; terminals (aka tokens) correspond to
@@ -96,7 +99,7 @@ fold-operator := %
 fold-operator := ^
 fold-operator := |
 fold-operator := <<
-fold-operator := >>
+fold-operator := greatergreater
 fold-operator := +=
 fold-operator := -=
 fold-operator := *=
@@ -202,7 +205,7 @@ additive-expression := additive-expression - 
multiplicative-expression
 # expr.shift
 shift-expression := additive-expression
 shift-expression := shift-expression << additive-expression
-shift-expression := shift-expression >> additive-expression
+shift-expression := shift-expression greatergreater additive-expression
 # expr.spaceship
 compare-expression := shift-expression
 compare-expression := compare-expression <=> shift-expression
@@ -615,7 +618,7 @@ operator-name := <=>
 operator-name := ^^
 operator-name := ||
 operator-name := <<
-operator-name := >>
+operator-name := greatergreater
 operator-name := <<=
 operator-name := >>=
 operator-name := ++
@@ -737,3 +740,8 @@ contextual-zero := NUMERIC_CONSTANT
 module-keyword := IDENTIFIER
 import-keyword := IDENTIFIER
 export-keyword := IDENTIFIER
+
+#! greatergreater token -- clang lexer always lexes it as a single token, we
+#! split it into two tokens to make the GLR parser aware of the nested-template
+#! case.
+greatergreater := > >

diff  --git a/clang-tools-extra/pseudo/unittests/TokenTest.cpp 
b/clang-tools-extra/pseudo/unittests/TokenTest.cpp
index 1357d23501193..8280a9b29341e 100644
--- a/clang-tools-extra/pseudo/unittests/TokenTest.cpp
+++ b/clang-tools-extra/pseudo/unittests/TokenTest.cpp
@@ -171,6 +171,26 @@ no_indent \
 }));
 }
 
+TEST(TokenTest, SplitGreaterGreater) {
+  LangOptions Opts;
+  std::st

[PATCH] D121756: [clang-format] Clean up code looking for if statements

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D121756#3398165 , @sstwcw wrote:

> It turned out this patch does change behavior.
>
>   -  while (
>   -  FormatTok->isOneOf(tok::identifier, tok::kw_requires, 
> tok::coloncolon)) {
>   +  while (FormatTok->isOneOf(tok::identifier, tok::kw_requires,
>   +tok::coloncolon)) {
>
> So what do I do?
>
> And what's the regression suite?

I like to run clang-format over all the files in LLVM that were previously 
clang-formatted, (now be careful because some people don't keep their files 
formatted), but you can run this over the ~8000 files in LLVM that are 
clang-format clean, this should give you a good idea of if you change is 
breaking stuff.

clang-format -verbose -n -files ./clang/docs/tools/clang-formatted-files.txt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[PATCH] D121370: [clang-format] SortIncludes should support "@import" lines in Objective-C

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk added a comment.

@MyDeveloperDay can you please have another look now that the patch has 
additional tests and most comments that still apply have been addressed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121370

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

[PATCH] D121756: [clang-format] Clean up code looking for if statements

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

FWIW I'm not a fan of the `while( \n` case, so assuming this fix, fixes that 
then that would be good I think.

  -  while (
  -  FormatTok->isOneOf(tok::identifier, tok::kw_requires, 
tok::coloncolon)) {
  +  while (FormatTok->isOneOf(tok::identifier, tok::kw_requires,
  +tok::coloncolon)) {


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[PATCH] D122139: [pseudo] Introduce parse forest.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 417225.
hokein marked 7 inline comments as done.
hokein added a comment.

address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122139

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
  clang-tools-extra/pseudo/lib/CMakeLists.txt
  clang-tools-extra/pseudo/lib/Forest.cpp
  clang-tools-extra/pseudo/unittests/CMakeLists.txt
  clang-tools-extra/pseudo/unittests/ForestTest.cpp

Index: clang-tools-extra/pseudo/unittests/ForestTest.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/unittests/ForestTest.cpp
@@ -0,0 +1,130 @@
+//===--- ForestTest.cpp - Test Forest dump --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang-pseudo/Forest.h"
+#include "clang-pseudo/Token.h"
+#include "clang/Basic/LangOptions.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace pseudo {
+namespace {
+
+// FIXME: extract to a TestGrammar class to allow code sharing among tests.
+class ForestTest : public ::testing::Test {
+public:
+  void build(llvm::StringRef BNF) {
+Diags.clear();
+G = Grammar::parseBNF(BNF, Diags);
+  }
+
+  SymbolID symbol(llvm::StringRef Name) const {
+for (unsigned I = 0; I < NumTerminals; ++I)
+  if (G->table().Terminals[I] == Name)
+return tokenSymbol(static_cast(I));
+for (SymbolID ID = 0; ID < G->table().Nonterminals.size(); ++ID)
+  if (G->table().Nonterminals[ID].Name == Name)
+return ID;
+ADD_FAILURE() << "No such symbol found: " << Name;
+return 0;
+  }
+
+  RuleID ruleFor(llvm::StringRef NonterminalName) const {
+auto RuleRange = G->table().Nonterminals[symbol(NonterminalName)].RuleRange;
+if (RuleRange.End - RuleRange.Start == 1)
+  return G->table().Nonterminals[symbol(NonterminalName)].RuleRange.Start;
+ADD_FAILURE() << "Expected a single rule for " << NonterminalName
+  << ", but it has " << RuleRange.End - RuleRange.Start
+  << " rule!\n";
+return 0;
+  }
+
+protected:
+  std::unique_ptr G;
+  std::vector Diags;
+};
+
+TEST_F(ForestTest, DumpBasic) {
+  build(R"cpp(
+_ := add-expression
+add-expression := id-expression + id-expression
+id-expression := IDENTIFIER
+  )cpp");
+  ASSERT_TRUE(Diags.empty());
+  ForestArena Arena;
+  const auto &TS =
+  cook(lex("a + b", clang::LangOptions()), clang::LangOptions());
+
+  auto T = Arena.createTerminals(TS);
+  ASSERT_EQ(T.size(), 3u);
+  const auto *Left = &Arena.createSequence(
+  symbol("id-expression"), ruleFor("id-expression"), {&T.front()});
+  const auto *Right = &Arena.createSequence(symbol("id-expression"),
+ruleFor("id-expression"), {&T[2]});
+
+  const auto *Add =
+  &Arena.createSequence(symbol("add-expression"), 1, {Left, &T[1], Right});
+  EXPECT_EQ(Add->dumpRecursive(*G, true),
+"[  0, end) add-expression := id-expression + id-expression\n"
+"[  0,   1)   id-expression~IDENTIFIER := tok[0]\n"
+"[  1,   2)   + := tok[1]\n"
+"[  2, end)   id-expression~IDENTIFIER := tok[2]\n");
+  EXPECT_EQ(Add->dumpRecursive(*G, false),
+"[  0, end) add-expression := id-expression + id-expression\n"
+"[  0,   1)   id-expression := IDENTIFIER\n"
+"[  0,   1) IDENTIFIER := tok[0]\n"
+"[  1,   2)   + := tok[1]\n"
+"[  2, end)   id-expression := IDENTIFIER\n"
+"[  2, end) IDENTIFIER := tok[2]\n");
+}
+
+TEST_F(ForestTest, DumpAmbiguousAndRefs) {
+  build(R"cpp(
+_ := type  # rule 0
+type := class-type # rule 1
+type := enum-type # rule 2
+class-type := shared-type
+enum-type := shared-type
+shared-type := IDENTIFIER)cpp");
+  ASSERT_TRUE(Diags.empty());
+  ForestArena Arena;
+  const auto &TS = cook(lex("abc", clang::LangOptions()), clang::LangOptions());
+
+  auto Terminals = Arena.createTerminals(TS);
+  ASSERT_EQ(Terminals.size(), 1u);
+
+  const auto *SharedType = &Arena.createSequence(
+  symbol("shared-type"), ruleFor("shared-type"), {Terminals.begin()});
+  const auto *ClassType = &Arena.createSequence(
+  symbol("class-type"), ruleFor("class-type"), {SharedType});
+  const auto *Enumtype = &Arena.createSequence(
+  symbol("enum-type"), ruleFor("enum-type"), {SharedType});
+  const auto *Alternative1 =
+  &Arena.createSequence(symbol("type"), /*RuleID=*/1, {ClassType});
+  const auto *Alternative2 =
+  &Arena.createSequence(symbol("type"

[PATCH] D122143: [clang][dataflow] Add support for disabling warnings on smart pointers.

[Stanislav Gatev via Phabricator via cfe-commits]

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



Comment at: 
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:31
+struct UncheckedOptionalAccessModelOptions {
+  /// Ignore optionals reachable by derefencing a smart pointer (other than
+  /// optional itself). The analysis does not track smart-pointer pointees, so

Can you clarify what "smart pointer" refers to? It's not only standard types 
like `unique_ptr`, `shared_ptr`, and `weak_ptr`, right?



Comment at: 
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:32
+  /// Ignore optionals reachable by derefencing a smart pointer (other than
+  /// optional itself). The analysis does not track smart-pointer pointees, so
+  /// it can't identify when optionals resulting from dereferencing such





Comment at: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:1923-1927
+template 
+struct unique_ptr {
+  T& operator*() &;
+  T* operator->();
+};

Let's add this to a `memory.h` header.



Comment at: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:1929
+
+struct Member {
+  $ns::$optional opt;

Why `Member`? I suggest either using one of the generic names that we use above 
or maybe something more descriptive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122143

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

[PATCH] D121756: [clang-format] Clean up code looking for if statements

[Owen Pan via Phabricator via cfe-commits]

owenpan requested changes to this revision.
owenpan added inline comments.



Comment at: clang/lib/Format/ContinuationIndenter.cpp:749
+  if (Current.isNot(tok::comment) &&
+  Previous.isConditionLParen(/*IncludeSpecial=*/true)) {
 // Treat the condition inside an if as if it was a second function

We only checked `for` and `if` before. Now you are also checking `while` and 
`switch`?



Comment at: clang/lib/Format/FormatToken.h:529
+  bool isConditionLParen(bool IncludeSpecial) const {
+if (!is(tok::l_paren))
+  return false;





Comment at: clang/lib/Format/FormatToken.h:534-539
+// `for` and `catch` special handling.
+return Prev &&
+   ((IncludeSpecial && Prev->isOneOf(TT_ForEachMacro, TT_ObjCForIn,
+ tok::kw_for, tok::kw_catch)) ||
+Prev->isOneOf(tok::kw_if, tok::kw_while, tok::kw_switch,
+  tok::kw_case, tok::kw_constexpr));

We prefer early returns and shorter conditionals.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2425
   nextToken();
-if (FormatTok->is(tok::l_paren))
+if (FormatTok->Tok.is(tok::l_paren)) {
+  FormatTok->setFinalizedType(TT_ConditionLParen);





Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2719-2721
+  // Those that begin with a for require special treatment because inside the
+  // parentheses is not an expression.
+  bool IsFor = FormatTok->is(tok::kw_for) || FormatTok->is(TT_ForEachMacro);

You can move the comment to above line 2729 below if you like.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:2729
+  if (FormatTok->is(tok::l_paren)) {
+if (!IsFor)
+  FormatTok->setFinalizedType(TT_ConditionLParen);




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121756

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

[clang] 51ba13b - [CGStmtOpenMP] Remove uses of deprecated Address constructor

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-03-22T11:00:08+01:00
New Revision: 51ba13b1aea3d6e04310b80b6bcfc641049b9890

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

LOG: [CGStmtOpenMP] Remove uses of deprecated Address constructor

Added: 


Modified: 
clang/lib/CodeGen/CGStmtOpenMP.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index 7ce3cd99c1b76..a5ee7abc655bd 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -153,13 +153,13 @@ class OMPLoopScope : public 
CodeGenFunction::RunCleanupsScope {
   const auto *OrigVD =
   cast(cast(IRef)->getDecl());
   if (EmittedAsPrivate.insert(OrigVD->getCanonicalDecl()).second) {
+QualType OrigVDTy = OrigVD->getType().getNonReferenceType();
 (void)PreCondVars.setVarAddr(
 CGF, OrigVD,
-Address::deprecated(
-llvm::UndefValue::get(
-CGF.ConvertTypeForMem(CGF.getContext().getPointerType(
-OrigVD->getType().getNonReferenceType(,
-CGF.getContext().getDeclAlign(OrigVD)));
+Address(llvm::UndefValue::get(CGF.ConvertTypeForMem(
+CGF.getContext().getPointerType(OrigVDTy))),
+CGF.ConvertTypeForMem(OrigVDTy),
+CGF.getContext().getDeclAlign(OrigVD)));
   }
 }
   }
@@ -991,10 +991,11 @@ bool CodeGenFunction::EmitOMPCopyinClause(const 
OMPExecutableDirective &D) {
   MasterAddr = EmitLValue(&DRE).getAddress(*this);
   LocalDeclMap.erase(VD);
 } else {
-  MasterAddr = Address::deprecated(
-  VD->isStaticLocal() ? CGM.getStaticLocalDeclAddress(VD)
-  : CGM.GetAddrOfGlobal(VD),
-  getContext().getDeclAlign(VD));
+  MasterAddr =
+  Address(VD->isStaticLocal() ? CGM.getStaticLocalDeclAddress(VD)
+  : CGM.GetAddrOfGlobal(VD),
+  CGM.getTypes().ConvertTypeForMem(VD->getType()),
+  getContext().getDeclAlign(VD));
 }
 // Get the address of the threadprivate variable.
 Address PrivateAddr = EmitLValue(*IRef).getAddress(*this);
@@ -1162,8 +1163,9 @@ void CodeGenFunction::EmitOMPLastprivateClauseFinal(
 // Get the address of the private variable.
 Address PrivateAddr = GetAddrOfLocalVar(PrivateVD);
 if (const auto *RefTy = PrivateVD->getType()->getAs())
-  PrivateAddr = Address::deprecated(
+  PrivateAddr = Address(
   Builder.CreateLoad(PrivateAddr),
+  CGM.getTypes().ConvertTypeForMem(RefTy->getPointeeType()),
   CGM.getNaturalTypeAlignment(RefTy->getPointeeType()));
 // Store the last value to the private copy in the last iteration.
 if (C->getKind() == OMPC_LASTPRIVATE_conditional)
@@ -1634,7 +1636,7 @@ Address 
CodeGenFunction::OMPBuilderCBHelpers::getAddressOfLocalVariable(
   Addr,
   CGF.ConvertTypeForMem(CGM.getContext().getPointerType(CVD->getType())),
   getNameWithSeparators({CVD->getName(), ".addr"}, ".", "."));
-  return Address::deprecated(Addr, Align);
+  return Address(Addr, CGF.ConvertTypeForMem(CVD->getType()), Align);
 }
 
 Address CodeGenFunction::OMPBuilderCBHelpers::getAddrOfThreadPrivate(
@@ -1657,7 +1659,7 @@ Address 
CodeGenFunction::OMPBuilderCBHelpers::getAddrOfThreadPrivate(
   llvm::CallInst *ThreadPrivateCacheCall =
   OMPBuilder.createCachedThreadPrivate(CGF.Builder, Data, Size, CacheName);
 
-  return Address::deprecated(ThreadPrivateCacheCall, VDAddr.getAlignment());
+  return Address(ThreadPrivateCacheCall, CGM.Int8Ty, VDAddr.getAlignment());
 }
 
 std::string CodeGenFunction::OMPBuilderCBHelpers::getNameWithSeparators(
@@ -4634,9 +4636,10 @@ void CodeGenFunction::EmitOMPTaskBasedDirective(
 Scope.addPrivate(Pair.first, CGF.EmitLValue(&DRE).getAddress(CGF));
   }
   for (const auto &Pair : PrivatePtrs) {
-Address Replacement =
-Address::deprecated(CGF.Builder.CreateLoad(Pair.second),
-CGF.getContext().getDeclAlign(Pair.first));
+Address Replacement = Address(
+CGF.Builder.CreateLoad(Pair.second),
+CGF.ConvertTypeForMem(Pair.first->getType().getNonReferenceType()),
+CGF.getContext().getDeclAlign(Pair.first));
 Scope.addPrivate(Pair.first, Replacement);
 if (auto *DI = CGF.getDebugInfo())
   if (CGF.CGM.getCodeGenOpts().hasReducedDebugInfo())
@@ -4647,18 +4650,22 @@ void CodeGenFunction::EmitOMPTaskBasedDirec

[PATCH] D120824: [clang][ASTImporter] Fix a bug when importing CXXDefaultInitExpr.

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

balazske added a comment.

Ping
I had a question about if the lit-test is needed with the new unit tests (that 
contain essentially similar code). But if there is no answer I will commit it 
with all tests, more test is always better.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120824

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

[PATCH] D118935: [SYCL] Disallow explicit casts between mismatching address spaces

[Mariya Podchishchaeva via Phabricator via cfe-commits]

Fznamznon added a comment.

> I misread what was done with addrspace_cast, that new operator only allows 
> conversions that are otherwise also allowed. Based on that, this change does 
> actually align with what was done for OpenCL mode, it does not restrict 
> anything that is allowed in OpenCL mode. It does make sense, then. A slightly 
> more verbose commit message might have helped though :)

Right, the intention of this change is to re-use OpenCL mode logic. There is no 
connection between addrspace_cast operator and this change. In fact, there is 
no such operator in SYCL.

> Even better, some comments in the code explaining the "why" would have helped.

I was under impression that the change is small and therefore easy to 
understand. Would some comment like "SYCL re-uses OpenCL mode diagnostics to 
emit errors in case the cast happens between disjoint address spaces" help?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118935

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

[PATCH] D122160: [clang][extract-api] Refactor ExtractAPI and improve docs

[Daniel Grumberg via Phabricator via cfe-commits]

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

LGTM once the last few bits of feedback I left are considered.




Comment at: clang/include/clang/ExtractAPI/API.h:139
 
+  /// Create and add a global variable record into the API set.
   GlobalRecord *addGlobalVar(StringRef Name, StringRef USR, PresumedLoc Loc,

Maybe we should document that the caller is meant to keep `Name` and `USR` 
alive. Name is expected to refer to the appropriate `Decl` member so it 
shouldn't be an issue, but this usage should be documented. WRT `USR` these 
comments would be a good place to mention the existence of `recordUSR` and the 
`copyString` method as a way of getting USR strings that are guaranteed to have 
the right lifetime.



Comment at: clang/include/clang/ExtractAPI/API.h:194
+  /// \returns a StringRef of the copied string in the \p Allocator.
+  StringRef copyString(StringRef String, llvm::BumpPtrAllocator &Allocator);
+

Should this be a private member function? The overload that doesn't take in an 
allocator makes sense since it uses the `APISet`'s allocator, but this one does 
not seem like it should be publicly accessible, since it just ties the lifetime 
of the returned String to that of an arbitrary allocator?



Comment at: clang/include/clang/ExtractAPI/API.h:129
 public:
-  APISet(const llvm::Triple &Target, const LangOptions &LangOpts)
-  : Target(Target), LangOpts(LangOpts) {}
-
-  const llvm::Triple &getTarget() const { return Target; }
-  const LangOptions &getLangOpts() const { return LangOpts; }
-
+  /// Create and add a GlobalRecord of kind \p Kind into the API set.
   GlobalRecord *addGlobal(GVKind Kind, StringRef Name, StringRef USR,

zixuw wrote:
> dang wrote:
> > why is stuff reordered in the patch?
> I looked into some other class definitions inside Clang, and was trying to 
> list "interesting" APIs upfront and less-interesting stuff like constructors 
> near the end for large class definitions to make them more readable.
OK that is a nice change then, I wasn't aware that this is a thing. I was just 
complaining because it made the diff harder to parse for me ;)



Comment at: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp:195
+// the Symbol Graph format.
+SymbolGraphSerializer SGSerializer(Visitor.getAPI());
+SGSerializer.serialize(*OS);

zixuw wrote:
> dang wrote:
> > The kind of serializer should be made configurable somehow.
> Yes for sure. I didn't include that change here because it would probably 
> involve adding command line options and changing the driver, which should 
> have its own patch later.
> I added a FIXME here to point it out for now.
Yeah that makes sense, we can do this as a follow up patch, definitely not 
urgent as SymbolGraph is the only use case at the moment. However, we should do 
this relatively soon so that the structure of the existing command line 
arguments is stable before we are fully done with this.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:30
+/// at position \p Key.
 static void serializeObject(Object &Paren, StringRef Key,
 Optional Obj) {

You don't need both static an anonymous namespace afaik.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122160

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

[PATCH] D122215: [WebAssembly] Initial support for reference types in clang

[Alex Bradbury via Phabricator via cfe-commits]

asb added inline comments.



Comment at: clang/test/CodeGen/WebAssembly/wasm-externref.c:1
+// RUN: %clang_cc1 -triple wasm32-unknown-unknown -target-feature 
+reference-types -o - -emit-llvm %s | FileCheck %s
+

It might be worth using update_cc_test_checks.py for this and wasm-funcref.c.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122215

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

[PATCH] D121201: [clang] Merge the SourceRange into ParsedAttributes

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

tbaeder updated this revision to Diff 417235.
tbaeder marked 5 inline comments as done.

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

https://reviews.llvm.org/D121201

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaOpenCL/address-spaces.cl

Index: clang/test/SemaOpenCL/address-spaces.cl
===
--- clang/test/SemaOpenCL/address-spaces.cl
+++ clang/test/SemaOpenCL/address-spaces.cl
@@ -258,7 +258,7 @@
 
 void func_multiple_addr2(void) {
   typedef __private int private_int_t;
-  __private __attribute__((opencl_global)) int var1;   // expected-error {{multiple address spaces specified for type}} \
+  __attribute__((opencl_global)) __private int var1;   // expected-error {{multiple address spaces specified for type}} \
// expected-error {{function scope variable cannot be declared in global address space}}
   __private __attribute__((opencl_global)) int *var2;  // expected-error {{multiple address spaces specified for type}}
   __attribute__((opencl_global)) private_int_t var3;   // expected-error {{multiple address spaces specified for type}}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -368,7 +368,8 @@
 };
 
 static void processTypeAttrs(TypeProcessingState &state, QualType &type,
- TypeAttrLocation TAL, ParsedAttributesView &attrs);
+ TypeAttrLocation TAL,
+ const ParsedAttributesView &attrs);
 
 static bool handleFunctionTypeAttr(TypeProcessingState &state, ParsedAttr &attr,
QualType &type);
@@ -8138,7 +8139,12 @@
 
 static void processTypeAttrs(TypeProcessingState &state, QualType &type,
  TypeAttrLocation TAL,
- ParsedAttributesView &attrs) {
+ const ParsedAttributesView &attrs) {
+
+  state.setParsedNoDeref(false);
+  if (attrs.empty())
+return;
+
   // Scan through and apply attributes to this type where it makes sense.  Some
   // attributes (such as __address_space__, __vector_size__, etc) apply to the
   // type, but others can be present in the type specifiers even though they
@@ -8148,9 +8154,6 @@
   // sure we visit every element once. Copy the attributes list, and iterate
   // over that.
   ParsedAttributesView AttrsCopy{attrs};
-
-  state.setParsedNoDeref(false);
-
   for (ParsedAttr &attr : AttrsCopy) {
 
 // Skip attributes that were marked to be invalid.
Index: clang/lib/Sema/SemaStmtAttr.cpp
===
--- clang/lib/Sema/SemaStmtAttr.cpp
+++ clang/lib/Sema/SemaStmtAttr.cpp
@@ -495,8 +495,7 @@
   }
 }
 
-void Sema::ProcessStmtAttributes(Stmt *S,
- const ParsedAttributesWithRange &InAttrs,
+void Sema::ProcessStmtAttributes(Stmt *S, const ParsedAttributes &InAttrs,
  SmallVectorImpl &OutAttrs) {
   for (const ParsedAttr &AL : InAttrs) {
 if (const Attr *A = ProcessStmtAttribute(*this, S, AL, InAttrs.Range))
Index: clang/lib/Sema/SemaStmt.cpp
===
--- clang/lib/Sema/SemaStmt.cpp
+++ clang/lib/Sema/SemaStmt.cpp
@@ -587,7 +587,7 @@
   return AttributedStmt::Create(Context, AttrsLoc, Attrs, SubStmt);
 }
 
-StmtResult Sema::ActOnAttributedStmt(const ParsedAttributesWithRange &Attrs,
+StmtResult Sema::ActOnAttributedStmt(const ParsedAttributes &Attrs,
  Stmt *SubStmt) {
   SmallVector SemanticAttrs;
   ProcessStmtAttributes(SubStmt, Attrs, SemanticAttrs);
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -2615,12 +2615,11 @@
 /// example:
 ///class foo : public bar, virtual private baz {
 /// 'public bar' and 'virtual private baz' are each base-specifiers.
-BaseResult
-Sema::ActOnBaseSpecifier(Decl *classdecl, SourceRange SpecifierRange,
- ParsedAttributes &Attributes,
- bool Virtual,

[PATCH] D121201: [clang] Merge the SourceRange into ParsedAttributes

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

tbaeder added inline comments.



Comment at: clang/include/clang/Sema/DeclSpec.h:2516-2521
+  void takeAttributes(ParsedAttributes &attrs) {
 Attrs.takeAllFrom(attrs);
 
-if (!lastLoc.isInvalid())
-  SetRangeEnd(lastLoc);
+if (attrs.Range.getEnd().isValid())
+  SetRangeEnd(attrs.Range.getEnd());
   }

aaron.ballman wrote:
> 
I blindly changed this and it took me a while to figure out that's wrong from 
the test failures: 

`Attrs.takeAllFrom(Attrs)`...



Comment at: clang/include/clang/Sema/ParsedAttr.h:920
 
   void clearListOnly() { AttrList.clear(); }
 

erichkeane wrote:
> Also, this function is now strange/likely needs a rename, since it likely 
> needs to reset the range as well.  I believe the point of this being a 
> separate function is to not clear the 'pool'.
Clearing the source range in here causes a bunch of  test failures FWIW, I 
assume because the old `ParsedAttributesWithRange` did it in `clean()` as well.


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

https://reviews.llvm.org/D121201

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

[PATCH] D118935: [SYCL] Disallow explicit casts between mismatching address spaces

[Ronan Keryell via Phabricator via cfe-commits]

keryell added a comment.

Yes, your comment idea looks good and helpful to me.
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118935

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

[clang] 73777b4 - [Debugify] Optimize debugify original mode

[Djordje Todorovic via cfe-commits]

Author: Djordje Todorovic
Date: 2022-03-22T12:14:00+01:00
New Revision: 73777b4c35a390617cce0f6b4516e98fe5a88df1

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

LOG: [Debugify] Optimize debugify original mode

Before we start addressing the issue with having
a lot of false positives when using debugify in
the original mode, we have made a few patches that
should speed up the execution of the testing
utility Passes.

For example, when testing a large project
(let's say LLVM project itself), we can face
a lot of potential DI issues. Usually, we use
-verify-each-debuginfo-preserve (that is very
similar to -debugify-each) -- it collects
DI metadata before each Pass, and after the Pass
it checks if the Pass preserved the DI metadata.
However, we can speed up this process, since we
don't need to collect DI metadata before each
Pass -- we could use the DI metadata that are
collected after the previous Pass from
the pipeline as an input for the next Pass.

This patch speeds up the utility for ~2x.

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

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp
llvm/include/llvm/Transforms/Utils/Debugify.h
llvm/lib/Transforms/Utils/Debugify.cpp
llvm/tools/opt/opt.cpp
llvm/unittests/Transforms/Utils/DebugifyTest.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 716a565ee7871..eaf34eedcb2bb 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -1003,10 +1003,10 @@ void 
EmitAssemblyHelper::EmitAssemblyWithLegacyPassManager(
 TheModule->setDataLayout(TM->createDataLayout());
 
   DebugifyCustomPassManager PerModulePasses;
-  DebugInfoPerPassMap DIPreservationMap;
+  DebugInfoPerPass DebugInfoBeforePass;
   if (CodeGenOpts.EnableDIPreservationVerify) {
 PerModulePasses.setDebugifyMode(DebugifyMode::OriginalDebugInfo);
-PerModulePasses.setDIPreservationMap(DIPreservationMap);
+PerModulePasses.setDebugInfoBeforePass(DebugInfoBeforePass);
 
 if (!CodeGenOpts.DIBugsReportFilePath.empty())
   PerModulePasses.setOrigDIVerifyBugsReportFilePath(

diff  --git a/llvm/include/llvm/Transforms/Utils/Debugify.h 
b/llvm/include/llvm/Transforms/Utils/Debugify.h
index 892e354cd9edb..405bbb8e0be8a 100644
--- a/llvm/include/llvm/Transforms/Utils/Debugify.h
+++ b/llvm/include/llvm/Transforms/Utils/Debugify.h
@@ -23,7 +23,8 @@
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Pass.h"
 
-using DebugFnMap = llvm::MapVector;
+using DebugFnMap =
+llvm::MapVector;
 using DebugInstMap = llvm::MapVector;
 using DebugVarMap = llvm::MapVector;
 using WeakInstValueMap =
@@ -42,9 +43,6 @@ struct DebugInfoPerPass {
   DebugVarMap DIVariables;
 };
 
-/// Map pass names to a per-pass DebugInfoPerPass instance.
-using DebugInfoPerPassMap = llvm::MapVector;
-
 namespace llvm {
 class DIBuilder;
 
@@ -69,24 +67,24 @@ bool stripDebugifyMetadata(Module &M);
 ///
 /// \param M The module to collect debug information from.
 /// \param Functions A range of functions to collect debug information from.
-/// \param DIPreservationMap A map to collect the DI metadata.
+/// \param DebugInfoBeforePass DI metadata before a pass.
 /// \param Banner A prefix string to add to debug/error messages.
 /// \param NameOfWrappedPass A name of a pass to add to debug/error messages.
 bool collectDebugInfoMetadata(Module &M,
   iterator_range Functions,
-  DebugInfoPerPassMap &DIPreservationMap,
+  DebugInfoPerPass &DebugInfoBeforePass,
   StringRef Banner, StringRef NameOfWrappedPass);
 
 /// Check original debug information after a pass.
 ///
 /// \param M The module to collect debug information from.
 /// \param Functions A range of functions to collect debug information from.
-/// \param DIPreservationMap A map used to check collected the DI metadata.
+/// \param DebugInfoBeforePass DI metadata before a pass.
 /// \param Banner A prefix string to add to debug/error messages.
 /// \param NameOfWrappedPass A name of a pass to add to debug/error messages.
 bool checkDebugInfoMetadata(Module &M,
 iterator_range Functions,
-DebugInfoPerPassMap &DIPreservationMap,
+DebugInfoPerPass &DebugInfoBeforePass,
 StringRef Banner, StringRef NameOfWrappedPass,
 StringRef OrigDIVerifyBugsReportFilePath);
 } // namespace llvm
@@ -97,11 +95,11 @@ enum class DebugifyMode { NoDebugify, SyntheticDebugInfo, 
OriginalDebugInfo };
 llvm::ModulePass *createDebugifyModulePass(
 enum DebugifyMode Mode = DebugifyMode::SyntheticDebugInfo,

[PATCH] D115622: [Debugify] Optimize debugify original mode

[Djordje Todorovic 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 rG73777b4c35a3: [Debugify] Optimize debugify original mode 
(authored by djtodoro).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115622

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  llvm/include/llvm/Transforms/Utils/Debugify.h
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/tools/opt/opt.cpp
  llvm/unittests/Transforms/Utils/DebugifyTest.cpp

Index: llvm/unittests/Transforms/Utils/DebugifyTest.cpp
===
--- llvm/unittests/Transforms/Utils/DebugifyTest.cpp
+++ llvm/unittests/Transforms/Utils/DebugifyTest.cpp
@@ -121,15 +121,15 @@
 
   DebugInfoDrop *P = new DebugInfoDrop();
 
-  DebugInfoPerPassMap DIPreservationMap;
+  DebugInfoPerPass DIBeforePass;
   DebugifyCustomPassManager Passes;
-  Passes.setDIPreservationMap(DIPreservationMap);
+  Passes.setDebugInfoBeforePass(DIBeforePass);
   Passes.add(createDebugifyModulePass(DebugifyMode::OriginalDebugInfo, "",
-  &(Passes.getDebugInfoPerPassMap(;
+  &(Passes.getDebugInfoPerPass(;
   Passes.add(P);
   Passes.add(createCheckDebugifyModulePass(false, "", nullptr,
DebugifyMode::OriginalDebugInfo,
-   &(Passes.getDebugInfoPerPassMap(;
+   &(Passes.getDebugInfoPerPass(;
 
   testing::internal::CaptureStderr();
   Passes.run(*M);
@@ -172,15 +172,15 @@
 
   DebugValueDrop *P = new DebugValueDrop();
 
-  DebugInfoPerPassMap DIPreservationMap;
+  DebugInfoPerPass DIBeforePass;
   DebugifyCustomPassManager Passes;
-  Passes.setDIPreservationMap(DIPreservationMap);
+  Passes.setDebugInfoBeforePass(DIBeforePass);
   Passes.add(createDebugifyModulePass(DebugifyMode::OriginalDebugInfo, "",
-  &(Passes.getDebugInfoPerPassMap(;
+  &(Passes.getDebugInfoPerPass(;
   Passes.add(P);
   Passes.add(createCheckDebugifyModulePass(false, "", nullptr,
DebugifyMode::OriginalDebugInfo,
-   &(Passes.getDebugInfoPerPassMap(;
+   &(Passes.getDebugInfoPerPass(;
 
   testing::internal::CaptureStderr();
   Passes.run(*M);
@@ -225,15 +225,15 @@
 
   DebugInfoDummyAnalysis *P = new DebugInfoDummyAnalysis();
 
-  DebugInfoPerPassMap DIPreservationMap;
+  DebugInfoPerPass DIBeforePass;
   DebugifyCustomPassManager Passes;
-  Passes.setDIPreservationMap(DIPreservationMap);
+  Passes.setDebugInfoBeforePass(DIBeforePass);
   Passes.add(createDebugifyModulePass(DebugifyMode::OriginalDebugInfo, "",
-  &(Passes.getDebugInfoPerPassMap(;
+  &(Passes.getDebugInfoPerPass(;
   Passes.add(P);
   Passes.add(createCheckDebugifyModulePass(false, "", nullptr,
DebugifyMode::OriginalDebugInfo,
-   &(Passes.getDebugInfoPerPassMap(;
+   &(Passes.getDebugInfoPerPass(;
 
   testing::internal::CaptureStderr();
   Passes.run(*M);
Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -858,13 +858,13 @@
   // the (-check)-debugify passes.
   DebugifyCustomPassManager Passes;
   DebugifyStatsMap DIStatsMap;
-  DebugInfoPerPassMap DIPreservationMap;
+  DebugInfoPerPass DebugInfoBeforePass;
   if (DebugifyEach) {
 Passes.setDebugifyMode(DebugifyMode::SyntheticDebugInfo);
 Passes.setDIStatsMap(DIStatsMap);
   } else if (VerifyEachDebugInfoPreserve) {
 Passes.setDebugifyMode(DebugifyMode::OriginalDebugInfo);
-Passes.setDIPreservationMap(DIPreservationMap);
+Passes.setDebugInfoBeforePass(DebugInfoBeforePass);
 if (!VerifyDIPreserveExport.empty())
   Passes.setOrigDIVerifyBugsReportFilePath(VerifyDIPreserveExport);
   }
@@ -884,10 +884,10 @@
   Passes.setDIStatsMap(DIStatsMap);
   Passes.add(createDebugifyModulePass());
 } else if (VerifyDebugInfoPreserve) {
-  Passes.setDIPreservationMap(DIPreservationMap);
+  Passes.setDebugInfoBeforePass(DebugInfoBeforePass);
   Passes.add(createDebugifyModulePass(
   DebugifyMode::OriginalDebugInfo, "",
-  &(Passes.getDebugInfoPerPassMap(;
+  &(Passes.getDebugInfoPerPass(;
 }
   }
 
@@ -1026,7 +1026,7 @@
 Passes.setOrigDIVerifyBugsReportFilePath(VerifyDIPreserveExport);
   Passes.add(createCheckDebugifyModulePass(
   false, "", nullptr, DebugifyMode::OriginalDebugInfo,
- 

[PATCH] D121873: [clang][extract-api] Add enum support

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/include/clang/ExtractAPI/API.h:33
 
+namespace {
+

I still think this belongs in `APISet`. It belongs there because it defines 
unique pointers that are specifically tied to the lifetime of `APISet`. Any 
other thing that needs a reference to things allocated in the APISet should 
just get the raw pointer out, ideally they should be storing the pointer 
returned by the `add*` methods of APISet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121873

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

[PATCH] D122143: [clang][dataflow] Add support for disabling warnings on smart pointers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 417243.
ymandel marked an inline comment as done.
ymandel added a comment.

Addressed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122143

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

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1219,7 +1219,9 @@
 llvm::Error Error = checkDataflow(
 SourceCode, FuncMatcher,
 [](ASTContext &Ctx, Environment &) {
-  return UncheckedOptionalAccessModel(Ctx);
+  return UncheckedOptionalAccessModel(
+  Ctx, UncheckedOptionalAccessModelOptions{
+   /*IgnoreSmartPointerDereference=*/true});
 },
 [&MatchesLatticeChecks](
 llvm::ArrayRef
+struct smart_ptr {
+  T& operator*() &;
+  T* operator->();
+};
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target() {
+  smart_ptr foo;
+  *foo->opt;
+  /*[[check-1]]*/
+  *(*foo).opt;
+  /*[[check-2]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
+}
+
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -45,15 +45,23 @@
 
 auto hasOptionalType() { return hasType(optionalClass()); }
 
-auto isOptionalMemberCallWithName(llvm::StringRef MemberName) {
+auto isOptionalMemberCallWithName(
+llvm::StringRef MemberName,
+llvm::Optional Ignorable = llvm::None) {
+  auto Exception = unless(Ignorable ? expr(anyOf(*Ignorable, cxxThisExpr()))
+: cxxThisExpr());
   return cxxMemberCallExpr(
-  on(expr(unless(cxxThisExpr(,
+  on(expr(Exception)),
   callee(cxxMethodDecl(hasName(MemberName), ofClass(optionalClass();
 }
 
-auto isOptionalOperatorCallWithName(llvm::StringRef OperatorName) {
-  return cxxOperatorCallExpr(hasOverloadedOperatorName(OperatorName),
- callee(cxxMethodDecl(ofClass(optionalClass();
+auto isOptionalOperatorCallWithName(
+llvm::StringRef operator_name,
+llvm::Optional Ignorable = llvm::None) {
+  return cxxOperatorCallExpr(
+  hasOverloadedOperatorName(operator_name),
+  callee(cxxMethodDecl(ofClass(optionalClass(,
+  Ignorable ? callExpr(unless(hasArgument(0, *Ignorable))) : callExpr());
 }
 
 auto isMakeOptionalCall() {
@@ -333,10 +341,22 @@
   transferSwap(*OptionalLoc1, *OptionalLoc2, State);
 }
 
-auto buildTransferMatchSwitch() {
+llvm::Optional
+ignorableOptional(const UncheckedOptionalAccessModelOptions &Options) {
+  if (Options.IgnoreSmartPointerDereference)
+return memberExpr(hasObjectExpression(ignoringParenImpCasts(
+cxxOperatorCallExpr(anyOf(hasOverloadedOperatorName("->"),
+  hasOverloadedOperatorName("*")),
+unless(hasArgument(0, expr(hasOptionalType(;
+  return llvm::None;
+}
+
+auto buildTransferMatchSwitch(
+const UncheckedOptionalAccessModelOptions &Options) {
   // FIXME: Evaluate the efficiency of matchers. If using matchers results in a
   // lot of duplicated work (e.g. string comparisons), consider providing APIs
   // that avoid it through memoization.
+  auto IgnorableOptional = ignorableOptional(Options);
   return MatchSwitchBuilder()
   // Attach a symbolic "has_value" state to optional values that we see for
   // the first time.
@@ -371,19 +391,20 @@
 
   // optional::value
   .CaseOf(
-  isOptionalMemberCallWithName("value"),
+  isOptionalMemberCallWithName("value", IgnorableOptional),
   [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
  LatticeTransferState &State) {
 transferUnwrapCall(E, E->getImplicitObjectArgument(), State);
   })
 
   // optional::operator*, optional::operator->
-  .CaseOf(expr(anyOf(isOptionalOperatorCallWithName("*"),
-   isOptionalOperatorCallWithName("->"))),
-[](const CallExpr *E, const MatchFinder::MatchResult &,
-   LatticeTransferState &State) {
-

[PATCH] D122143: [clang][dataflow] Add support for disabling warnings on smart pointers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 4 inline comments as done.
ymandel added inline comments.



Comment at: 
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:31
+struct UncheckedOptionalAccessModelOptions {
+  /// Ignore optionals reachable by derefencing a smart pointer (other than
+  /// optional itself). The analysis does not track smart-pointer pointees, so

sgatev wrote:
> Can you clarify what "smart pointer" refers to? It's not only standard types 
> like `unique_ptr`, `shared_ptr`, and `weak_ptr`, right?
I've rewritten it to specify the operators directly.



Comment at: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:1923-1927
+template 
+struct unique_ptr {
+  T& operator*() &;
+  T* operator->();
+};

sgatev wrote:
> Let's add this to a `memory.h` header.
I specifically wanted a locally-defined type. Notice that it's not in `std`. 
I've renamed to generic "smart_ptr" to emphasize that. WDYT?



Comment at: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:1929
+
+struct Member {
+  $ns::$optional opt;

sgatev wrote:
> Why `Member`? I suggest either using one of the generic names that we use 
> above or maybe something more descriptive.
Went with generic. I think `Member` was from copy pasting...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122143

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

[PATCH] D121824: [clang] Do not crash on arrow operator on dependent type.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I reduced something very similar recently as 
https://github.com/clangd/clangd/issues/1073

This patch does not fix it, but looks closely related, want to take a look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121824

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

[PATCH] D122008: [flang][driver] Add support for generating executables

[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 :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[clang] 9858884 - [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

[via cfe-commits]

Author: Vince Bridgers
Date: 2022-03-22T07:35:13-05:00
New Revision: 985888411da9c62aea7b0b41f93eb75ad31587a6

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

LOG: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

Usages of makeNull need to be deprecated in favor of makeNullWithWidth
for architectures where the pointer size should not be assumed. This can
occur when pointer sizes can be of different sizes, depending on address
space for example. See https://reviews.llvm.org/D118050 as an example.

This was uncovered initially in a downstream compiler project, and
tested through those systems tests.

steakhal performed systems testing across a large set of open source
projects.

Co-authored-by: steakhal
Resolves: https://github.com/llvm/llvm-project/issues/53664

Reviewed By: NoQ, steakhal

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

Added: 


Modified: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
clang/test/Analysis/cast-value-notes.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 1df47dae25bf5..1f02ee84c898a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -365,13 +365,21 @@ class SValBuilder {
   /// space.
   /// \param type pointer type.
   Loc makeNullWithType(QualType type) {
+// We cannot use the `isAnyPointerType()`.
+assert((type->isPointerType() || type->isObjCObjectPointerType() ||
+type->isBlockPointerType() || type->isNullPtrType() ||
+type->isReferenceType()) &&
+   "makeNullWithType must use pointer type");
+
+// The `sizeof(T&)` is `sizeof(T)`, thus we replace the reference with a
+// pointer. Here we assume that references are actually implemented by
+// pointers under-the-hood.
+type = type->isReferenceType()
+   ? Context.getPointerType(type->getPointeeType())
+   : type;
 return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type));
   }
 
-  Loc makeNull() {
-return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
-  }
-
   Loc makeLoc(SymbolRef sym) {
 return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
   }

diff  --git a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
index 4235c0c138210..e2b99fca7d148 100644
--- a/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
@@ -250,7 +250,7 @@ static void addCastTransition(const CallEvent &Call, 
DefinedOrUnknownSVal DV,
   CastSucceeds);
 
   SVal V = CastSucceeds ? C.getSValBuilder().evalCast(DV, CastToTy, CastFromTy)
-: C.getSValBuilder().makeNull();
+: C.getSValBuilder().makeNullWithType(CastToTy);
   C.addTransition(
   State->BindExpr(Call.getOriginExpr(), C.getLocationContext(), V, false),
   getNoteTag(C, CastInfo, CastToTy, Object, CastSucceeds, IsKnownCast));
@@ -359,7 +359,9 @@ static void evalNullParamNullReturn(const CallEvent &Call,
   if (ProgramStateRef State = C.getState()->assume(DV, false))
 C.addTransition(State->BindExpr(Call.getOriginExpr(),
 C.getLocationContext(),
-C.getSValBuilder().makeNull(), false),
+C.getSValBuilder().makeNullWithType(
+Call.getOriginExpr()->getType()),
+false),
 C.getNoteTag("Assuming null pointer is passed into cast",
  /*IsPrunable=*/true));
 }

diff  --git a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
index 238022d1253b9..63b1065ae4de0 100644
--- a/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2593,8 +2593,8 @@ MallocChecker::ReallocMemAux(CheckerContext &C, const 
CallEvent &Call,
 
   SValBuilder &svalBuilder = C.getSValBuilder();
 
-  DefinedOrUnkno

[PATCH] D119601: [analyzer] Refactor makeNull to makeNullWithWidth (NFC)

[Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG985888411da9: [analyzer] Refactor makeNull to 
makeNullWithWidth (NFC) (authored by vabridgers, committed by einvbri 
).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
  clang/test/Analysis/cast-value-notes.cpp

Index: clang/test/Analysis/cast-value-notes.cpp
===
--- clang/test/Analysis/cast-value-notes.cpp
+++ clang/test/Analysis/cast-value-notes.cpp
@@ -1,9 +1,22 @@
 // RUN: %clang_analyze_cc1 -std=c++14 \
 // RUN:  -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
-// RUN:  -analyzer-output=text -verify %s
+// RUN:  -analyzer-output=text -verify -DDEFAULT_TRIPLE %s 2>&1 | FileCheck %s -check-prefix=DEFAULT-CHECK
+//
+// RUN: %clang_analyze_cc1 -std=c++14 -triple amdgcn-unknown-unknown \
+// RUN: -analyzer-checker=core,apiModeling.llvm.CastValue,debug.ExprInspection\
+// RUN: -analyzer-output=text -verify -DAMDGCN_TRIPLE %s 2>&1 | FileCheck %s -check-prefix=AMDGCN-CHECK
 
 #include "Inputs/llvm.h"
 
+// The amggcn triple case uses an intentionally different address space.
+// The core.NullDereference checker intentionally ignores checks
+// that use address spaces, so the case is differentiated here.
+//
+// From https://llvm.org/docs/AMDGPUUsage.html#address-spaces,
+// select address space 3 (local), since the pointer size is
+// different than Generic.
+#define DEVICE __attribute__((address_space(3)))
+
 namespace clang {
 struct Shape {
   template 
@@ -21,12 +34,30 @@
 using namespace llvm;
 using namespace clang;
 
+void clang_analyzer_printState();
+
+#if defined(DEFAULT_TRIPLE)
 void evalReferences(const Shape &S) {
   const auto &C = dyn_cast(S);
   // expected-note@-1 {{Assuming 'S' is not a 'Circle'}}
   // expected-note@-2 {{Dereference of null pointer}}
   // expected-warning@-3 {{Dereference of null pointer}}
+  clang_analyzer_printState();
+  // DEFAULT-CHECK: "dynamic_types": [
+  // DEFAULT-CHECK-NEXT: { "region": "SymRegion{reg_$0}", "dyn_type": "const class clang::Circle &", "sub_classable": true }
+  (void)C;
+}
+#elif defined(AMDGCN_TRIPLE)
+void evalReferences(const Shape &S) {
+  const auto &C = dyn_cast(S);
+  clang_analyzer_printState();
+  // AMDGCN-CHECK: "dynamic_types": [
+  // AMDGCN-CHECK-NEXT: { "region": "SymRegion{reg_$0}", "dyn_type": "const __attribute__((address_space(3))) class clang::Circle &", "sub_classable": true }
+  (void)C;
 }
+#else
+#error Target must be specified, and must be pinned
+#endif
 
 void evalNonNullParamNonNullReturnReference(const Shape &S) {
   const auto *C = dyn_cast_or_null(S);
Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -61,7 +61,7 @@
 
 DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) {
   if (Loc::isLocType(type))
-return makeNull();
+return makeNullWithType(type);
 
   if (type->isIntegralOrEnumerationType())
 return makeIntVal(0, type);
@@ -359,7 +359,7 @@
 return makeBoolVal(cast(E));
 
   case Stmt::CXXNullPtrLiteralExprClass:
-return makeNull();
+return makeNullWithType(E->getType());
 
   case Stmt::CStyleCastExprClass:
   case Stmt::CXXFunctionalCastExprClass:
@@ -399,7 +399,7 @@
 
 if (Loc::isLocType(E->getType()))
   if (E->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull))
-return makeNull();
+return makeNullWithType(E->getType());
 
 return None;
   }
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2410,7 +2410,7 @@
   SVal V;
 
   if (Loc::isLocType(T))
-V = svalBuilder.makeNull();
+V = svalBuilder.makeNullWithType(T);
   else if (T->isIntegralOrEnumerationType())
 V = svalBuilder.makeZeroVal(T);
   else if (T->isStructureOrClassType() || T->isArrayType()) {
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -46

[PATCH] D122224: Allow -Wno-gnu to silence GNU extensions related to pointer arithmetic

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: nickdesaulniers, andrew.w.kaylor, eli.friedman, 
erichkeane.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

These diagnostics were added to a diagnostic group, but that diagnostic group 
was not under `-Wgnu`. I've now split them into their own diagnostic group that 
is added both to the original group (so user's currently opting in or out of 
these should not see a change) and under the `-Wgnu` group so that `-Wno-gnu` 
can be used to disable all GNU extension diagnostics. This fixes Issue 5.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D14

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/Sema/pointer-addition.c

Index: clang/test/Sema/pointer-addition.c
===
--- clang/test/Sema/pointer-addition.c
+++ clang/test/Sema/pointer-addition.c
@@ -1,6 +1,7 @@
-// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -std=c11
-// RUN: %clang_cc1 %s -fsyntax-only -triple i686-unknown-unknown -verify -pedantic -Wextra -std=c11
-// RUN: %clang_cc1 %s -fsyntax-only -triple x86_64-unknown-unknown -verify -pedantic -Wextra -std=c11
+// RUN: %clang_cc1 %s -fsyntax-only -verify=gnu,expected -pedantic -Wextra -std=c11
+// RUN: %clang_cc1 %s -fsyntax-only -triple i686-unknown-unknown -verify=gnu,expected -pedantic -Wextra -std=c11
+// RUN: %clang_cc1 %s -fsyntax-only -triple x86_64-unknown-unknown -verify=gnu,expected -pedantic -Wextra -std=c11
+// RUN: %clang_cc1 %s -fsyntax-only -verify -pedantic -Wextra -Wno-gnu -std=c11
 
 #include 
 
@@ -10,22 +11,22 @@
   void (*fp)(int) = 0;
   b++;   // expected-error {{arithmetic on a pointer to an incomplete type}}
   b += 1;// expected-error {{arithmetic on a pointer to an incomplete type}}
-  c++;   // expected-warning {{arithmetic on a pointer to void is a GNU extension}}
-  c += 1;// expected-warning {{arithmetic on a pointer to void is a GNU extension}}
-  c--;   // expected-warning {{arithmetic on a pointer to void is a GNU extension}}
-  c -= 1;// expected-warning {{arithmetic on a pointer to void is a GNU extension}}
-  (void) c[1]; // expected-warning {{subscript of a pointer to void is a GNU extension}}
+  c++;   // gnu-warning {{arithmetic on a pointer to void is a GNU extension}}
+  c += 1;// gnu-warning {{arithmetic on a pointer to void is a GNU extension}}
+  c--;   // gnu-warning {{arithmetic on a pointer to void is a GNU extension}}
+  c -= 1;// gnu-warning {{arithmetic on a pointer to void is a GNU extension}}
+  (void) c[1]; // gnu-warning {{subscript of a pointer to void is a GNU extension}}
   b = 1+b;   // expected-error {{arithmetic on a pointer to an incomplete type}}
   /* The next couple tests are only pedantic warnings in gcc */
   void (*d)(S*,void*) = a;
-  d += 1;// expected-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
-  d++;   // expected-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
-  d--;   // expected-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
-  d -= 1;// expected-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
-  (void)(1 + d); // expected-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
+  d += 1;// gnu-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
+  d++;   // gnu-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
+  d--;   // gnu-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
+  d -= 1;// gnu-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
+  (void)(1 + d); // gnu-warning {{arithmetic on a pointer to the function type 'void (S *, void *)' (aka 'void (struct S *, void *)') is a GNU extension}}
   e++;   // expected-error {{arithmetic on a pointer to an incomplete type}}
   intptr_t i = (intptr_t)b;
-  char *f = (char*)0 + i; // expected-warning {{arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension}}
+  char *f = (char*)0 + i; // gnu-warning {{arithmetic on a null pointer treated as a cast from integer to pointer is a GNU extension}}
   // Cases that don't match the G

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

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

steakhal requested changes to this revision.
steakhal added a comment.
This revision now requires changes to proceed.

I'm liking it. We need to improve the diagnostics and the user-facing docs as 
well.




Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:545
+  Dependencies<[ErrnoModeling]>,
+  Documentation;
+

Then we should have documentation, and examples for it.



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp:47-49
+  mutable bool ErrnoInitialized = false;
+  mutable Optional ErrnoLoc;
+  mutable const MemRegion *ErrnoRegion = nullptr;

One should not define member variables like this.
The analysis engine might explore exploded nodes in an unpredictable order, 
invoking the checker's handler callbacks in an indeterminate order.
You should have registered simple value traits for this purpose, associating 
the necessary information with a given State.



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp:68-71
+  if (ErrnoLoc) {
+ErrnoRegion = ErrnoLoc->getAsRegion();
+assert(ErrnoRegion && "The 'errno' location should be a memory region.");
+  }

`Loc` always wrap a memregion of some sort.



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp:114-116
+  if (auto L = Loc.getAs()) {
+if (*ErrnoLoc != *L)
+  return;

I think an early return could have spared an indentation level in the outer 
scope.



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp:198
+  // allow read and write without bug reports.
+  if (llvm::find(Regions, ErrnoRegion) != Regions.end())
+return setErrnoStateIrrelevant(State);





Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoChecker.cpp:203-206
+  const MemSpaceRegion *GlobalSystemSpace =
+  State->getStateManager().getRegionManager().getGlobalsRegion(
+  MemRegion::GlobalSystemSpaceRegionKind);
+  if (llvm::find(Regions, GlobalSystemSpace) != Regions.end())





Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:271-272
+if (IdentifierInfo *II = FD->getIdentifier())
+  return llvm::find(ErrnoLocationFuncNames, II->getName()) !=
+ std::end(ErrnoLocationFuncNames);
+  return false;





Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h:24
 
+enum ErrnoCheckState : unsigned {
+  Errno_Irrelevant = 0,

Did you consider enum classes?



Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h:35
+
+llvm::Optional getErrnoLoc(ProgramStateRef State);
+

I would appreciate some notes about when it returns `None`.



Comment at: clang/test/Analysis/errno.c:73-75
+  int X = ErrnoTesterChecker_setErrnoCheckState();
+  if (X == 0) {
+if (errno) { // expected-warning{{Value of 'errno' could be undefined 
after a call to a function that does not promise to not change 'errno' 
[alpha.unix.Errno]}}

We should have a note describing which function left the `errno` in an 
undefined/indeterminate state.
You can chain multiple NoteTags by using the ExplodedNode returned by the 
`addTransition()` and passing it along if needed.



Comment at: clang/test/Analysis/errno.c:170
+clang_analyzer_eval(errno); // expected-warning{{TRUE}}
+something();
+clang_analyzer_eval(errno); // expected-warning{{UNKNOWN}}

So this is the case when `errno` gets indirectly invalidated by some opaque 
function call.  I see.
However, it will test that the value associated with the errno region gets 
invalidated, that's fine. However, you should test if the metadata (the enum 
value) attached to memregion also gets invalidated.
Please make sure you have tests for that as well.
A `ErrnoTesterChecker_dumpCheckState()` should be perfect for this.


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] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Hello, @devin.jeanpierre! I'm wondering if we should revert this while you 
investigate the fix, or do you think you'll have this solved sometime today? 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119017

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

[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

1 request for a test, and 1 hope that @aaron.ballman will help bikeshed (or 
just say its fine!), but otherwise LGTM.




Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:69
+def note_constexpr_static_local : Note<
+  "control flows through the declaration of a %select{static|thread_local}0 
variable">;
 def note_constexpr_subobject_declared_here : Note<

Hmm... this feels a little awkward to me.  Though, I usually have Aaron do the 
bikeshedding, so if he didn't come up with better I guess we can let it go.



Comment at: clang/lib/AST/ExprConstant.cpp:5010
+  // through a declaration of a variable with static or thread storage 
duration.
+  if (VD->isLocalVarDecl() && VD->isStaticLocal()) {
+Info.CCEDiag(VD->getLocation(), diag::note_constexpr_static_local)

Can we make sure we have tests for the 'other' two types of TSCSpec?  I'm not 
too worried about C11 thread local (it does, after all, require static), but 
the GNU __thread does not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

[Aaron Ballman via Phabricator via cfe-commits]

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

I don't spot anything overly concerning in this patch, I believe it LGTM as 
well.




Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp:38
+  if (!b)
+NonLiteral n;
+}

hubert.reinterpretcast wrote:
> cor3ntin wrote:
> > hubert.reinterpretcast wrote:
> > > For consistency, this should warn (under `-Wpre-c++2b-compat`).
> > I though we decided *not* to do that
> Actually, I think we only decided that it should always be an error in older 
> modes (and we didn't decide not to add a compat warning). That is, the 
> extension and compat warnings just happen to be paired currently in the 
> patch. Now that the code has cleared out of my system a bit, I believe that 
> there is no fundamental reason for the two warnings to be paired.
> 
> I'm fine with getting this patch landed and then fixing this after. Maybe 
> @aaron.ballman would comment as well.
I would also be fine with addressing that after this patch lands.



Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp:1-3
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu 
-std=c++11 -Werror=c++14-extensions -Werror=c++20-extensions 
-Werror=c++2b-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu 
-std=c++14 -DCXX14 -Werror=c++20-extensions -Werror=c++2b-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu 
-std=c++20 -DCXX14 -DCXX20 -Werror=c++2b-extensions %s

hubert.reinterpretcast wrote:
> cor3ntin wrote:
> > hubert.reinterpretcast wrote:
> > > cor3ntin wrote:
> > > > hubert.reinterpretcast wrote:
> > > > > hubert.reinterpretcast wrote:
> > > > > > cor3ntin wrote:
> > > > > > > hubert.reinterpretcast wrote:
> > > > > > > > The use `-Werror` hides potential issues where the error is 
> > > > > > > > categorically produced (instead of under the warning group).
> > > > > > > > 
> > > > > > > > Considering that there is a relevant design consistency issue 
> > > > > > > > of exactly that sort right now that this test could have 
> > > > > > > > highlighted (but didn't), a change to stop using `-Werror` may 
> > > > > > > > be prudent.
> > > > > > > This seems inconsistent with how that file is currently 
> > > > > > > structured though
> > > > > > I meant to change the entire file to check for warnings instead of 
> > > > > > errors. I guess that really should be a separate patch.
> > > > > I guess the change will cause the "never produces a constant 
> > > > > expression" warning unless if that is suppressed with 
> > > > > `-Wno-invalid-constexpr`.
> > > > Yes, we could cleanup this entire file to get rid of the #ifdef, then 
> > > > change how warnings are diagnosed.
> > > I am not in favour of getting rid of the `#ifdef`s. They still tell us 
> > > that the "conformance" warnings are associated with the right modes.
> > To be clear, i meant using `//cxx20-warning` and things like that instead, 
> > which is functionally equivalent. Does that make sense?
> Yes it does. I think that would be good (but does not need to be part of this 
> patch).
FWIW, I also agree that it'd be good to use `-verify=something` instead of 
`#ifdef` (but not strictly necessary for this patch).



Comment at: clang/test/SemaCXX/constant-expression-cxx2b.cpp:118
+
+} // namespace eval_goto
+

hubert.reinterpretcast wrote:
> cor3ntin wrote:
> > hubert.reinterpretcast wrote:
> > > Move `#endif` to here (from below) so the explicitly-`constexpr` lambda 
> > > cases are also tried in C++20 mode.
> > I mean sure I can do that, but what are we testing here?
> We're testing that the extension behaviour is actually extended to 
> explicitly-`constexpr` lambdas in C++20 mode despite the non-application (in 
> C++20 mode) of the new rules when determining whether a lambda is implicitly 
> `constexpr`.
> 
> Having the test reinforces that the behaviour is as intended, which serves a 
> design documentation purpose.
> 
> The test also arises when applying closed-box testing methodology to 
> speculate how a desired behaviour may have been implemented in a way that 
> also leads to undesired behaviour. In other words, maybe the code keyed off 
> too much on being in a lambda body.
> 
> Yes, I know we can read the code, but then again that's today's code and not 
> necessarily tomorrow's.
> Having the test reinforces that the behaviour is as intended, which serves a 
> design documentation purpose.

We do this in our tests somewhat regularly, but please be sure to add comments 
to explain that the test is serving a design documentation purpose, otherwise 
it can be easy to later go "oh, this is obviously broken nonsense so it's fine 
that behavior changed" by accident.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

__

[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:69
+def note_constexpr_static_local : Note<
+  "control flows through the declaration of a %select{static|thread_local}0 
variable">;
 def note_constexpr_subobject_declared_here : Note<

erichkeane wrote:
> Hmm... this feels a little awkward to me.  Though, I usually have Aaron do 
> the bikeshedding, so if he didn't come up with better I guess we can let it 
> go.
I didn't think it was overly awkward, about the only question I had was "should 
we quote the `static` and `thread_local` as keywords?" when I reviewed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:69
+def note_constexpr_static_local : Note<
+  "control flows through the declaration of a %select{static|thread_local}0 
variable">;
 def note_constexpr_subobject_declared_here : Note<

aaron.ballman wrote:
> erichkeane wrote:
> > Hmm... this feels a little awkward to me.  Though, I usually have Aaron do 
> > the bikeshedding, so if he didn't come up with better I guess we can let it 
> > go.
> I didn't think it was overly awkward, about the only question I had was 
> "should we quote the `static` and `thread_local` as keywords?" when I 
> reviewed.
Ok, I can live with that.  AS far as quoting, I think we are consistently 
inconsistent on that one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[clang] 5856f30 - [LTO] Add configuartion option to use default optimization pipeline

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-03-22T09:28:45-04:00
New Revision: 5856f30b5ae06153ff4a7d3db73ff8d9a05b4144

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

LOG: [LTO] Add configuartion option to use default optimization pipeline

This patch adds a configuration option to simply use the default pass
pipeline in favor of the LTO-specific one. We observed some severe
performance penalties when uding device-side LTO for OpenMP offloading
applications caused by the LTO-pass pipeline. This is primarily because
OpenMP uses an LLVM bitcode library to implement a GPU runtime library.
In a standard compilation we link this bitcode library into each source
file and optimize it with the default pipeline. When performing LTO we
link it late with all the files, but the bitcode library never has the
regular optimization pipeline applied to it so we miss a few
optimizations just using the LTO pipeline to optimize it.

I'm not committed to this solution, but it's the easiest method to solve
this performance regression when using LTO without changing the
optimizatin pipeline for other users.

Reviewed By: tianshilei1992

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

Added: 


Modified: 
clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
llvm/include/llvm/LTO/Config.h
llvm/lib/LTO/LTOBackend.cpp

Removed: 




diff  --git a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp 
b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
index a64648e6fe155..e9f616653d33e 100644
--- a/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ b/clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -860,6 +860,7 @@ std::unique_ptr createLTO(
   // TODO: Handle index-only thin-LTO
   Backend = lto::createInProcessThinBackend(
   llvm::heavyweight_hardware_concurrency(1));
+  Conf.UseDefaultPipeline = true;
 
   Conf.CPU = Arch.str();
   Conf.Options = codegen::InitTargetOptionsFromCodeGenFlags(TheTriple);

diff  --git a/llvm/include/llvm/LTO/Config.h b/llvm/include/llvm/LTO/Config.h
index eb793d62907e2..0cf84de7b9f87 100644
--- a/llvm/include/llvm/LTO/Config.h
+++ b/llvm/include/llvm/LTO/Config.h
@@ -60,6 +60,9 @@ struct Config {
   /// Use the new pass manager
   bool UseNewPM = LLVM_ENABLE_NEW_PASS_MANAGER;
 
+  /// Use the standard optimization pipeline.
+  bool UseDefaultPipeline = false;
+
   /// Flag to indicate that the optimizer should not assume builtins are 
present
   /// on the target.
   bool Freestanding = false;

diff  --git a/llvm/lib/LTO/LTOBackend.cpp b/llvm/lib/LTO/LTOBackend.cpp
index 3877def53c3f1..91807b151eb9e 100644
--- a/llvm/lib/LTO/LTOBackend.cpp
+++ b/llvm/lib/LTO/LTOBackend.cpp
@@ -298,6 +298,8 @@ static void runNewPMPasses(const Config &Conf, Module &Mod, 
TargetMachine *TM,
   report_fatal_error(Twine("unable to parse pass pipeline description '") +
  Conf.OptPipeline + "': " + toString(std::move(Err)));
 }
+  } else if (Conf.UseDefaultPipeline) {
+MPM.addPass(PB.buildPerModuleDefaultPipeline(OL));
   } else if (IsThinLTO) {
 MPM.addPass(PB.buildThinLTODefaultPipeline(OL, ImportSummary));
   } else {



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

[PATCH] D122133: [LTO] Add configuartion option to use default optimization pipeline

[Joseph Huber 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 rG5856f30b5ae0: [LTO] Add configuartion option to use default 
optimization pipeline (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122133

Files:
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  llvm/include/llvm/LTO/Config.h
  llvm/lib/LTO/LTOBackend.cpp


Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -298,6 +298,8 @@
   report_fatal_error(Twine("unable to parse pass pipeline description '") +
  Conf.OptPipeline + "': " + toString(std::move(Err)));
 }
+  } else if (Conf.UseDefaultPipeline) {
+MPM.addPass(PB.buildPerModuleDefaultPipeline(OL));
   } else if (IsThinLTO) {
 MPM.addPass(PB.buildThinLTODefaultPipeline(OL, ImportSummary));
   } else {
Index: llvm/include/llvm/LTO/Config.h
===
--- llvm/include/llvm/LTO/Config.h
+++ llvm/include/llvm/LTO/Config.h
@@ -60,6 +60,9 @@
   /// Use the new pass manager
   bool UseNewPM = LLVM_ENABLE_NEW_PASS_MANAGER;
 
+  /// Use the standard optimization pipeline.
+  bool UseDefaultPipeline = false;
+
   /// Flag to indicate that the optimizer should not assume builtins are 
present
   /// on the target.
   bool Freestanding = false;
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -860,6 +860,7 @@
   // TODO: Handle index-only thin-LTO
   Backend = lto::createInProcessThinBackend(
   llvm::heavyweight_hardware_concurrency(1));
+  Conf.UseDefaultPipeline = true;
 
   Conf.CPU = Arch.str();
   Conf.Options = codegen::InitTargetOptionsFromCodeGenFlags(TheTriple);


Index: llvm/lib/LTO/LTOBackend.cpp
===
--- llvm/lib/LTO/LTOBackend.cpp
+++ llvm/lib/LTO/LTOBackend.cpp
@@ -298,6 +298,8 @@
   report_fatal_error(Twine("unable to parse pass pipeline description '") +
  Conf.OptPipeline + "': " + toString(std::move(Err)));
 }
+  } else if (Conf.UseDefaultPipeline) {
+MPM.addPass(PB.buildPerModuleDefaultPipeline(OL));
   } else if (IsThinLTO) {
 MPM.addPass(PB.buildThinLTODefaultPipeline(OL, ImportSummary));
   } else {
Index: llvm/include/llvm/LTO/Config.h
===
--- llvm/include/llvm/LTO/Config.h
+++ llvm/include/llvm/LTO/Config.h
@@ -60,6 +60,9 @@
   /// Use the new pass manager
   bool UseNewPM = LLVM_ENABLE_NEW_PASS_MANAGER;
 
+  /// Use the standard optimization pipeline.
+  bool UseDefaultPipeline = false;
+
   /// Flag to indicate that the optimizer should not assume builtins are present
   /// on the target.
   bool Freestanding = false;
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -860,6 +860,7 @@
   // TODO: Handle index-only thin-LTO
   Backend = lto::createInProcessThinBackend(
   llvm::heavyweight_hardware_concurrency(1));
+  Conf.UseDefaultPipeline = true;
 
   Conf.CPU = Arch.str();
   Conf.Options = codegen::InitTargetOptionsFromCodeGenFlags(TheTriple);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D121451: [clang-format] Add space to comments starting with '#'.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D121451#3395801 , @krasimir wrote:

> This makes me think that we should really consider not adding indent if the 
> first character is punctuation WAI, with rationale:

That was the case before this patch.

> - comments starting with such characters often have special meaning, 
> clang-format errors out on the conservative side to not touch them. Example 
> is stuff like `//!` in Doxygen blocks: 
> https://www.doxygen.nl/manual/docblocks.html (although that's special-handled 
> elsewhere).

I'm aware of these special characters, but I wasn't aware of the fact that `#` 
is a special case too (I don't know proto too much).

> - it's possible to work around this by changing the source code to use `// 
> #`; clang-format will respect indentation in that case (also for other 
> punctuation characters).

Again, that was the case before this patch.

What I wanted to achieve was to only treat a limited list of special 
punctuation characters, not all of them.
As you mentioned, we have at least `///, //<, //!` for Doxygen comments. There 
are more of course.
I think that every (group of) language(s) should have a different way of 
defining whether a given character is special and so should be kept with the 
leading `//`.
I hope it makes sense given that C-language family is pretty different from 
Proto where it comes to comments.
So I'd rather keep the old behaviour for `#` in Proto but not in C-like 
languages.
Is that acceptable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121451

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

[PATCH] D121916: [clang-format] [doc] Add script to automatically update help output in ClangFormat.rst.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

If nobody is against this patch. I'll land it soon, probably tomorrow.




Comment at: clang/docs/tools/dump_format_help.py:29
+def get_help_output():
+args = ["clang-format", "--help"]
+cmd = subprocess.Popen(args, stdout=subprocess.PIPE,

sstwcw wrote:
> You intentionally did not write `build/bin/clang-format` to accommodate 
> people who build in other directories, right? Sorry this comment is late.
I wanted to be coherent with the `generate_formatted_state.py` script that 
calls `clang-format` from PATH as well.
A user that builds elsewhere needs to set PATH when executing the script to use 
clang-format of their choice.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121916

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

[PATCH] D122179: Serialize PragmaAssumeNonNullLoc to support preambles

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:748
 
+TEST(DiagnosticsTest, PreambleWithPragmaAssumeNonnull) {
+  Annotations Header(R"cpp(

This test seems to have a lot of extraneous elements.

The following seems sufficient, with no header:
```
#pragma clang assume_nonnull begin
void foo(int *x);
#pragma clang assume_nonnull end
```



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:748
 
+TEST(DiagnosticsTest, PreambleWithPragmaAssumeNonnull) {
+  Annotations Header(R"cpp(

sammccall wrote:
> This test seems to have a lot of extraneous elements.
> 
> The following seems sufficient, with no header:
> ```
> #pragma clang assume_nonnull begin
> void foo(int *x);
> #pragma clang assume_nonnull end
> ```
this should be a clang test, rather than a clangd test (or in addition)

probably under clang/PCH/, of the form
```
// RUN: %clang_cc1 -emit-pch ...
// RUN: %clang_cc1 -include-pch -fsyntax-only -verify -ast-dump ...
// and optionally without PCH:
// RUN: %clang_cc1 -fsyntax-only -verify -ast-dump ...
```

Then you can verify both the lack of diagnostics and the AST that gets 
generated.



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:775
+  TU.AdditionalFiles["header.h"] = std::string(Header.code());
+  EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
+}

you verify no diagnostics, but likely also want to verify that the parameter 
has the nonnull attribute.

This is probably simple enough, e.g 

```
ParsedAST AST = TU.build();
ParmVarDecl *X = cast(findDecl(AST, "foo")).getParamDecl(0);
ASSERT_TRUE(X->hasAttr());
```

(or by matching the AST dump)



Comment at: clang/include/clang/Lex/PreprocessorOptions.h:131
   /// conditional #if stack, so the ASTWriter/ASTReader can save/restore it 
when
-  /// processing the rest of the file.
+  /// processing the rest of the file. In addition, for preambles, we keep 
track
+  /// of an unterminated pragma assume nonnull.

nit: "Similarly, we track an unterminated #pragma assume_nonnull"?

Currently it's not completely clear what the connection is.



Comment at: clang/lib/Lex/PPLexerChange.cpp:430
 
-  // Complain about reaching a true EOF within assume_nonnull.
+  // Complain about reaching a true EOF within assume_nonnull unless we're
+  // building a preamble.

unless we're hitting _the end_ of the preamble

This is a special case though, pull it out of the one-sentence summary (as the 
other special case is)



Comment at: clang/lib/Lex/PPLexerChange.cpp:435
   // instantiation or a _Pragma.
-  if (PragmaAssumeNonNullLoc.isValid() &&
+  if (PragmaAssumeNonNullLoc.isValid() && !this->PPOpts->GeneratePreamble &&
+  !(CurLexer && CurLexer->getFileID() == PredefinesFileID) &&

It's not valid *anywhere* we exit a file when building a preamble, only when we 
fall off the end of the preamble region itself within the main file.



Comment at: clang/lib/Lex/PPLexerChange.cpp:436
+  if (PragmaAssumeNonNullLoc.isValid() && !this->PPOpts->GeneratePreamble &&
+  !(CurLexer && CurLexer->getFileID() == PredefinesFileID) &&
   !isEndOfMacro && !(CurLexer && CurLexer->Is_PragmaLexer)) {

what's the PredefinesFileID special case about?



Comment at: clang/lib/Serialization/ASTWriter.cpp:2303
 
+  // If we have an unterminated pragma assume non null, remember it since it
+  // might be at the end of the preamble.

nit: #pragma assume_nonnull



Comment at: clang/lib/Serialization/ASTWriter.cpp:2304
+  // If we have an unterminated pragma assume non null, remember it since it
+  // might be at the end of the preamble.
+  SourceLocation AssumeNonNullLoc = PP.getPragmaAssumeNonNullLoc();

this "might" is doing a lot of spooky action at a distance.

At minimum we should assert this is a preamble, right? Or move this block into 
the if() below?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122179

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

[PATCH] D121451: [clang-format] Add space to comments starting with '#'.

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

> So I'd rather keep the old behaviour for # in Proto but not in C-like 
> languages. Is that acceptable?

Sounds good. I'll work on a patch doing that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121451

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

[PATCH] D121450: [clang-format] Handle attributes before case label.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added a comment.

In D121450#3397861 , @jgorbe wrote:

> Hi,
>
> We've observed that this patch introduces infinite loops in some cases. 
> Here's a reduced test case:
>
>   export class Foo extends Bar {
> get case(): Case {
>   return (
>   (this.Bar$has('case')) ? (this.Bar$get('case')) :
>(this.case = new Case()));
> }
>   }
>
> Saving this as `/tmp/test.ts` and running `clang-format /tmp/test.ts` loops 
> indefinitely with this patch (I stopped it after 1m27s) and finishes 
> instantly (0.12s) after reverting the patch locally. I'm going to go ahead 
> and push a revert. Please let me know if you need help reproducing the 
> problem.

Thanks for the reproducer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121450

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

[PATCH] D121450: [clang-format] Handle attributes before case label.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1826
+  if (Style.isJavaScript() && Line->MustBeDeclaration)
+// 'case: string' field declaration.
+break;

HazardyKnusperkeks wrote:
> Here's the loop. In the switch before that worked, since it was not in a 
> loop. Here we need to do something. (nextToken()?)
Yes, it seems so. I'll add a test case and fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121450

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

[PATCH] D122155: Add warning when eval-method is set in the presence of value unsafe floating-point calculations.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 417279.
zahiraam marked 5 inline comments as done.

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

https://reviews.llvm.org/D122155

Files:
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/Sema/eval-method-with-unsafe-math.c


Index: clang/test/Sema/eval-method-with-unsafe-math.c
===
--- /dev/null
+++ clang/test/Sema/eval-method-with-unsafe-math.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -ffp-eval-method=source \
+// RUN: -verify %s
+
+// RUN: %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -fapprox-func -ffp-eval-method=source \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=WARN
+
+// RUN: %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -freciprocal-math  -ffp-eval-method=source \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=WARN
+
+// RUN: %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -mreassociate -ffp-eval-method=source \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=WARN
+
+// expected-no-diagnostics
+
+float f1(float a, float b, float c) {
+  return a * b + c;
+  // WARN: setting the eval method via the '-ffp-eval-method' option or 
'#pragma clang fp eval_method' has not effect when numeric results of 
floating-point calculations aren't value-safe.
+}
+
+float f2(float a, float b, float c) {
+#pragma clang fp eval_method(double)
+  // WARN: setting the eval method via the '-ffp-eval-method' option or 
'#pragma clang fp eval_method' has not effect when numeric results of 
floating-point calculations aren't value-safe.
+  return a * b + c;
+}
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -486,6 +486,10 @@
 NewFPFeatures.setFPEvalMethodOverride(LangOptions::FEM_Extended);
 break;
   }
+  if (getLangOpts().ApproxFunc || getLangOpts().AllowFPReassoc ||
+  getLangOpts().AllowRecip)
+Diags.Report(
+diag::warn_eval_method_setting_is_meaningless_in_value_unsafe_context);
   FpPragmaStack.Act(Loc, PSK_Set, StringRef(), NewFPFeatures);
   CurFPFeatures = NewFPFeatures.applyOverrides(getLangOpts());
   PP.setCurrentFPEvalMethod(Loc, Value);
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -252,10 +252,22 @@
 // Use setting from TargetInfo.
 PP.setCurrentFPEvalMethod(SourceLocation(),
   ctxt.getTargetInfo().getFPEvalMethod());
-  else
+  else {
 // Set initial value of __FLT_EVAL_METHOD__ from the command line.
 PP.setCurrentFPEvalMethod(SourceLocation(),
   getLangOpts().getFPEvalMethod());
+if (getLangOpts().ApproxFunc || getLangOpts().AllowFPReassoc ||
+getLangOpts().AllowRecip)
+  // When these options are used, the compiler is allowed to make
+  // transformation that may affect the final result. For example
+  // (x+y)+z is transformed to x+(y+z) but may not give the same
+  // final result; it's not value safe.
+  // Another example can be to simlify x/x to 1.0 but x could be 0.0, INF
+  // or NaN. Final result may then differ.
+  Diags.Report(
+  diag::
+  warn_eval_method_setting_is_meaningless_in_value_unsafe_context);
+  }
   CurFPFeatures.setFPEvalMethod(PP.getCurrentFPEvalMethod());
 }
 
Index: clang/include/clang/Basic/DiagnosticCommonKinds.td
===
--- clang/include/clang/Basic/DiagnosticCommonKinds.td
+++ clang/include/clang/Basic/DiagnosticCommonKinds.td
@@ -124,6 +124,9 @@
   "'consteval' specifier is incompatible with C++ standards before C++20">,
   InGroup, DefaultIgnore;
 
+def warn_eval_method_setting_is_meaningless_in_value_unsafe_context : Warning<
+  "setting the eval method via '-ffp-eval-method' or '#pragma clang fp 
eval_method' "
+  "has not effect when numeric results of floating-point calculations aren't 
value-safe.">, InGroup;
 }
 
 let CategoryName = "Nullability Issue" in {


Index: clang/test/Sema/eval-method-with-unsafe-math.c
===
--- /dev/null
+++ clang/test/Sema/eval-method-with-unsafe-math.c
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -ffp-eval-method=source \
+// RUN: -verify %s
+
+// RUN: %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -fapprox-func -ffp-eval-method=source \
+// RUN: %s 2>&1 | FileCheck %s --check-prefix=WARN
+
+// RUN: %clang_cc1 -fexperimental-strict-floating-point \
+// RUN

[PATCH] D122102: [clangd] Introduce "add subclass" tweak

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added a comment.

Thank for your comments, @adamcz and @njames93!

I agree that a "Implement interface" tweak is probably more useful, and I can 
imagine to evolve this tweak in that direction.

The main reason, I first went for the "create a new class from scratch" 
approach, was because I was not sure how to merge the newly inserted methods 
with the potentially existing methods to get an intuitive order.

How do you think we should further proceed here?

- Do we see value in both an "Implement methods" and an "Add subclass" tweak?
- Are you, @njames93, planning to follow-up on your existing review?
- Would you like me to evolve this patch into an "Implement methods" patch, 
drawing inspiration from your existing review & the comments on it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122102

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

[PATCH] D122227: Fix _BitInt suffix width calculation

[Aaron Ballman via Phabricator via cfe-commits]

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

@mgehre-amd pointed out the following post-commit review feedback on the 
changes in 8cba72177dcd8de5d37177dbaf2347e5c1f0f1e8 
:

  As an example, the paper says 3wb /* Yields an _BitInt(3); two value bits, 
one sign bit */.
  So I would expect that 0xFwb gives _BitInt(5); four value bits, one sign bit, 
but with this implementation I get _BitInt(2).
  This is because ResultVal as 4 bits, and getMinSignedBits() inteprets it as 
negative and thus says that 1 bit is enough to represent -1.

This corrects the behavior for calculating the bit-width and adds some test 
coverage.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D17

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Lexer/bitint-constants.c


Index: clang/test/Lexer/bitint-constants.c
===
--- clang/test/Lexer/bitint-constants.c
+++ clang/test/Lexer/bitint-constants.c
@@ -142,3 +142,18 @@
   0x''''''''1wb; // expected-error {{integer 
literal is too large to be represented in any signed integer type}}
   0x''''''''1uwb; // expected-error {{integer 
literal is too large to be represented in any integer type}}
 }
+
+void TestTypes(void) {
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3wb), _BitInt(3)));
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-3wb), _BitInt(3)));
+  // 2 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3uwb), unsigned 
_BitInt(2)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFwb), _BitInt(5)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-0xFwb), _BitInt(5)));
+  // 4 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFuwb), unsigned 
_BitInt(4)));
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -3979,8 +3979,8 @@
   if (Literal.isBitInt) {
 // The signed version has one more bit for the sign value. There are no
 // zero-width bit-precise integers, even if the literal value is 0.
-Width = Literal.isUnsigned ? std::max(ResultVal.getActiveBits(), 1u)
-   : std::max(ResultVal.getMinSignedBits(), 
2u);
+Width = std::max(ResultVal.getActiveBits(), 1u) +
+(Literal.isUnsigned ? 0u : 1u);
 
 // Diagnose if the width of the constant is larger than 
BITINT_MAXWIDTH,
 // and reset the type to the largest supported width.


Index: clang/test/Lexer/bitint-constants.c
===
--- clang/test/Lexer/bitint-constants.c
+++ clang/test/Lexer/bitint-constants.c
@@ -142,3 +142,18 @@
   0x''''''''1wb; // expected-error {{integer literal is too large to be represented in any signed integer type}}
   0x''''''''1uwb; // expected-error {{integer literal is too large to be represented in any integer type}}
 }
+
+void TestTypes(void) {
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3wb), _BitInt(3)));
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-3wb), _BitInt(3)));
+  // 2 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3uwb), unsigned _BitInt(2)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFwb), _BitInt(5)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-0xFwb), _BitInt(5)));
+  // 4 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFuwb), unsigned _BitInt(4)));
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -3979,8 +3979,8 @@
   if (Literal.isBitInt) {
 // The signed version has one more bit for the sign value. There are no
 // zero-width bit-precise integers, even if the literal value is 0.
-Width = Literal.isUnsigned ? std::max(ResultVal.getActiveBits(), 1u)
-   : std::max(ResultVal.getMinSignedBits(), 2u);
+Width = std::max(ResultVal.getActiveBits(), 1u) +
+(Literal.isUnsigned ? 0u : 1u);
 
 // Diagnose if the width of the constant is larger than BITINT_MAXWIDTH,
 // and rese

[PATCH] D122102: [clangd] Introduce "add subclass" tweak

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added a comment.

@adamcz

> Ideally I'd like to see this code action show up as code completion option as 
> well.

I think in the long-term such a "Implement methods" code action should also 
apply as a quickfix for

  class Base {
 virtual void foo() = 0;
  };
  
  class Derived final : public Base {}; // as a quickfix for "Abstract class is 
marked 'final'"
  
  auto foo = make_unique(); // as a quickfix for "allocating an object 
of abstract class type 'Derived'"
  Derived d; // as a quickfix for "Variable type 'Derived' is an abstract class"

But that's probably for a future commit. Let's first see that we get the basic 
functionality into clangd :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122102

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

[PATCH] D122227: Fix _BitInt suffix width calculation

[Matthias Gehre via Phabricator via cfe-commits]

mgehre-amd accepted this revision.
mgehre-amd added a comment.

Thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

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

[clang] 9cf8f81 - Fix _BitInt suffix width calculation

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-03-22T10:00:05-04:00
New Revision: 9cf8f81ca45de198013f29442a7de6600b226d70

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

LOG: Fix _BitInt suffix width calculation

@mgehre-amd pointed out the following post-commit review feedback on
the changes in 8cba72177dcd8de5d37177dbaf2347e5c1f0f1e8:

As an example, the paper says 3wb /* Yields an _BitInt(3); two value
bits, one sign bit */.
So I would expect that 0xFwb gives _BitInt(5); four value bits, one
sign bit, but with this implementation I get _BitInt(2).
This is because ResultVal as 4 bits, and getMinSignedBits() inteprets
it as negative and thus says that 1 bit is enough to represent -1.

This corrects the behavior for calculating the bit-width and adds some
test coverage.

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/Lexer/bitint-constants.c

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index be90c58e066d8..7fdeb7a8e30be 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -3979,8 +3979,8 @@ ExprResult Sema::ActOnNumericConstant(const Token &Tok, 
Scope *UDLScope) {
   if (Literal.isBitInt) {
 // The signed version has one more bit for the sign value. There are no
 // zero-width bit-precise integers, even if the literal value is 0.
-Width = Literal.isUnsigned ? std::max(ResultVal.getActiveBits(), 1u)
-   : std::max(ResultVal.getMinSignedBits(), 
2u);
+Width = std::max(ResultVal.getActiveBits(), 1u) +
+(Literal.isUnsigned ? 0u : 1u);
 
 // Diagnose if the width of the constant is larger than 
BITINT_MAXWIDTH,
 // and reset the type to the largest supported width.

diff  --git a/clang/test/Lexer/bitint-constants.c 
b/clang/test/Lexer/bitint-constants.c
index 243f8c0377c27..2ff35e8207786 100644
--- a/clang/test/Lexer/bitint-constants.c
+++ b/clang/test/Lexer/bitint-constants.c
@@ -142,3 +142,18 @@ void ValidSuffixInvalidValue(void) {
   0x''''''''1wb; // expected-error {{integer 
literal is too large to be represented in any signed integer type}}
   0x''''''''1uwb; // expected-error {{integer 
literal is too large to be represented in any integer type}}
 }
+
+void TestTypes(void) {
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3wb), _BitInt(3)));
+  // 2 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-3wb), _BitInt(3)));
+  // 2 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(3uwb), unsigned 
_BitInt(2)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFwb), _BitInt(5)));
+  // 4 value bits, one sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(-0xFwb), _BitInt(5)));
+  // 4 value bits, no sign bit
+  _Static_assert(__builtin_types_compatible_p(__typeof__(0xFuwb), unsigned 
_BitInt(4)));
+}



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

[PATCH] D122227: Fix _BitInt suffix width calculation

[Aaron Ballman via Phabricator via cfe-commits]

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

Thanks for the quick reviews, and thanks for catching the issue @mgehre-amd! 
I've committed in 9cf8f81ca45de198013f29442a7de6600b226d70 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17

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

[clang] 73c0d05 - [CGOpenMPRuntimeGPU] Remove uses of deprecated address constructor

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-03-22T15:02:45+01:00
New Revision: 73c0d05e6a90612399de06a98201e9d6a9059797

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

LOG: [CGOpenMPRuntimeGPU] Remove uses of deprecated address constructor

Worth noting that the code marked with FIXME is dead and would
produce invalid IR if hit. Someone familiar with this code should
probably look into that.

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
index f8f469d09bb5d..78d6d1b1f93fe 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -2042,6 +2042,8 @@ static void emitReductionListCopy(
 // address of the next element in scratchpad memory, unless we're currently
 // processing the last one.  Memory alignment is also taken care of here.
 if ((IncrScratchpadDest || IncrScratchpadSrc) && (Idx + 1 < Size)) {
+  // FIXME: This code doesn't make any sense, it's trying to perform
+  // integer arithmetic on pointers.
   llvm::Value *ScratchpadBasePtr =
   IncrScratchpadDest ? DestBase.getPointer() : SrcBase.getPointer();
   llvm::Value *ElementSizeInChars = CGF.getTypeSize(Private->getType());
@@ -2063,9 +2065,10 @@ static void emitReductionListCopy(
 
   if (IncrScratchpadDest)
 DestBase =
-Address::deprecated(ScratchpadBasePtr, CGF.getPointerAlign());
+Address(ScratchpadBasePtr, CGF.VoidPtrTy, CGF.getPointerAlign());
   else /* IncrScratchpadSrc = true */
-SrcBase = Address::deprecated(ScratchpadBasePtr, 
CGF.getPointerAlign());
+SrcBase =
+Address(ScratchpadBasePtr, CGF.VoidPtrTy, CGF.getPointerAlign());
 }
 
 ++Idx;
@@ -2214,7 +2217,7 @@ static llvm::Value 
*emitInterWarpCopyFunction(CodeGenModule &CGM,
   llvm::Value *ElemPtrPtr = CGF.EmitLoadOfScalar(
   ElemPtrPtrAddr, /*Volatile=*/false, C.VoidPtrTy, SourceLocation());
   // elemptr = ((CopyType*)(elemptrptr)) + I
-  Address ElemPtr = Address::deprecated(ElemPtrPtr, Align);
+  Address ElemPtr(ElemPtrPtr, CGF.Int8Ty, Align);
   ElemPtr = Bld.CreateElementBitCast(ElemPtr, CopyType);
   if (NumIters > 1)
 ElemPtr = Bld.CreateGEP(ElemPtr, Cnt);
@@ -2224,10 +2227,14 @@ static llvm::Value 
*emitInterWarpCopyFunction(CodeGenModule &CGM,
   llvm::Value *MediumPtrVal = Bld.CreateInBoundsGEP(
   TransferMedium->getValueType(), TransferMedium,
   {llvm::Constant::getNullValue(CGM.Int64Ty), WarpID});
-  Address MediumPtr = Address::deprecated(MediumPtrVal, Align);
   // Casting to actual data type.
   // MediumPtr = (CopyType*)MediumPtrAddr;
-  MediumPtr = Bld.CreateElementBitCast(MediumPtr, CopyType);
+  Address MediumPtr(
+  Bld.CreateBitCast(
+  MediumPtrVal,
+  CopyType->getPointerTo(
+  MediumPtrVal->getType()->getPointerAddressSpace())),
+  CopyType, Align);
 
   // elem = *elemptr
   //*MediumPtr = elem
@@ -2273,15 +2280,19 @@ static llvm::Value 
*emitInterWarpCopyFunction(CodeGenModule &CGM,
   llvm::Value *SrcMediumPtrVal = Bld.CreateInBoundsGEP(
   TransferMedium->getValueType(), TransferMedium,
   {llvm::Constant::getNullValue(CGM.Int64Ty), ThreadID});
-  Address SrcMediumPtr = Address::deprecated(SrcMediumPtrVal, Align);
   // SrcMediumVal = *SrcMediumPtr;
-  SrcMediumPtr = Bld.CreateElementBitCast(SrcMediumPtr, CopyType);
+  Address SrcMediumPtr(
+  Bld.CreateBitCast(
+  SrcMediumPtrVal,
+  CopyType->getPointerTo(
+  SrcMediumPtrVal->getType()->getPointerAddressSpace())),
+  CopyType, Align);
 
   // TargetElemPtr = (CopyType*)(SrcDataAddr[i]) + I
   Address TargetElemPtrPtr = Bld.CreateConstArrayGEP(LocalReduceList, Idx);
   llvm::Value *TargetElemPtrVal = CGF.EmitLoadOfScalar(
   TargetElemPtrPtr, /*Volatile=*/false, C.VoidPtrTy, Loc);
-  Address TargetElemPtr = Address::deprecated(TargetElemPtrVal, Align);
+  Address TargetElemPtr(TargetElemPtrVal, CGF.Int8Ty, Align);
   TargetElemPtr = Bld.CreateElementBitCast(TargetElemPtr, CopyType);
   if (NumIters > 1)
 TargetElemPtr = Bld.CreateGEP(TargetElemPtr, Cnt);
@@ -2609,10 +2620,11 @@ static llvm::Value *emitListToGlobalCopyFunction(
 LValue GlobLVal = CGF.EmitLValueForField(
 CGF.MakeNaturalAlignAddrLValue(BufferArrPtr, StaticTy), FD);
 Address GlobAddr = GlobLVal.getAddress(CGF);
-llvm::Value *BufferPtr = Bld.CreateInBoundsGEP(
-GlobAddr.getElementType(), GlobAddr.getPointer(), Idxs);
-GlobLVal.setAd

[PATCH] D122230: [clang-format] don't break up #-style comment sections

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a project: All.
krasimir requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Follow-up from 
https://github.com/llvm/llvm-project/commit/36d13d3f8adb3d1a6bae71370afa23d11a94dc78.

Restore the old behavior in situations where we use # as comments and
long strings of #'s for comment sections.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122230

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestTextProto.cpp


Index: clang/unittests/Format/FormatTestTextProto.cpp
===
--- clang/unittests/Format/FormatTestTextProto.cpp
+++ clang/unittests/Format/FormatTestTextProto.cpp
@@ -408,6 +408,23 @@
"dd: 100\n"
" another quadriple-hash comment\n",
Style));
+
+  // Ensure we support a common pattern for naming sections.
+  EXPECT_EQ("##\n"
+"# section name\n"
+"##",
+format("##\n"
+   "# section name\n"
+   "##",
+   Style));
+
+  EXPECT_EQ("///\n"
+"// section name\n"
+"///",
+format("///\n"
+   "// section name\n"
+   "///",
+   Style));
 }
 
 TEST_F(FormatTestTextProto, FormatsExtensions) {
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -785,7 +785,16 @@
 Lines[i].substr(IndentPrefix.size(), FirstCharByteSize),
 Encoding) != 1)
   return false;
-if (FirstCommentChar == '#')
+// In C-like comments, add a space before #. For example this is useful
+// to preserve the relative indentation when commenting out code with
+// #includes.
+//
+// In languages using # as the comment leader such as proto, don't
+// add a space to support patterns like:
+// #
+// # section
+// #
+if (FirstCommentChar == '#' && !TokenText.startswith("#"))
   return false;
 return FirstCommentChar == '\\' || isPunctuation(FirstCommentChar) ||
isHorizontalWhitespace(FirstCommentChar);


Index: clang/unittests/Format/FormatTestTextProto.cpp
===
--- clang/unittests/Format/FormatTestTextProto.cpp
+++ clang/unittests/Format/FormatTestTextProto.cpp
@@ -408,6 +408,23 @@
"dd: 100\n"
" another quadriple-hash comment\n",
Style));
+
+  // Ensure we support a common pattern for naming sections.
+  EXPECT_EQ("##\n"
+"# section name\n"
+"##",
+format("##\n"
+   "# section name\n"
+   "##",
+   Style));
+
+  EXPECT_EQ("///\n"
+"// section name\n"
+"///",
+format("///\n"
+   "// section name\n"
+   "///",
+   Style));
 }
 
 TEST_F(FormatTestTextProto, FormatsExtensions) {
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -785,7 +785,16 @@
 Lines[i].substr(IndentPrefix.size(), FirstCharByteSize),
 Encoding) != 1)
   return false;
-if (FirstCommentChar == '#')
+// In C-like comments, add a space before #. For example this is useful
+// to preserve the relative indentation when commenting out code with
+// #includes.
+//
+// In languages using # as the comment leader such as proto, don't
+// add a space to support patterns like:
+// #
+// # section
+// #
+if (FirstCommentChar == '#' && !TokenText.startswith("#"))
   return false;
 return FirstCommentChar == '\\' || isPunctuation(FirstCommentChar) ||
isHorizontalWhitespace(FirstCommentChar);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122231: [clang][dataflow] Add support for `value_or` in a comparison.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: sgatev, xazax.hun.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

This patch adds limited modeling of the `value_or` method. Specifically, when
used in a particular idiom in a comparison to implicitly check whether the
optional holds a value.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122231

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -1710,6 +1710,95 @@
  UnorderedElementsAre(Pair("check", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
+  // Pointers.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+int* target() {
+  $ns::$optional opt;
+  if (opt.value_or(nullptr) != nullptr) {
+return *opt;
+/*[[check-ptrs-1]]*/
+  } else {
+*opt;
+/*[[check-ptrs-2]]*/
+  }
+  return nullptr;
+}
+  )code",
+  UnorderedElementsAre(Pair("check-ptrs-1", "safe"),
+   Pair("check-ptrs-2", "unsafe: input.cc:10:10")));
+
+  // Integers.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+int target() {
+  $ns::$optional opt;
+  if (opt.value_or(0) != 0) {
+return *opt;
+/*[[check-ints-1]]*/
+  } else {
+*opt;
+/*[[check-ints-2]]*/
+  }
+  return 0;
+}
+  )code",
+  UnorderedElementsAre(Pair("check-ints-1", "safe"),
+   Pair("check-ints-2", "unsafe: input.cc:10:10")));
+
+  // Strings.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+namespace std {
+  struct string {
+bool empty();
+  };
+}
+
+std::string target() {
+  $ns::$optional opt;
+  if (!opt.value_or("").empty()) {
+return *opt;
+/*[[check-strings-1]]*/
+  } else {
+*opt;
+/*[[check-strings-2]]*/
+  }
+  return {};
+}
+  )code",
+  UnorderedElementsAre(Pair("check-strings-1", "safe"),
+   Pair("check-strings-2", "unsafe: input.cc:16:10")));
+
+  // Pointer-to-optional.
+  ExpectLatticeChecksFor(
+  R"code(
+#include "unchecked_optional_access_test.h"
+
+int target() {
+  $ns::$optional opt;
+  auto *opt_p = &opt;
+  if (opt_p->value_or(0) != 0) {
+return opt_p->value();
+/*[[check-pto-1]]*/
+  } else {
+opt_p->value();
+/*[[check-pto-2]]*/
+  }
+  return 0;
+}
+  )code",
+  UnorderedElementsAre(Pair("check-pto-1", "safe"),
+   Pair("check-pto-2", "unsafe: input.cc:11:9")));
+}
+
 TEST_P(UncheckedOptionalAccessTest, Emplace) {
   ExpectLatticeChecksFor(R"(
 #include "unchecked_optional_access_test.h"
@@ -2008,5 +2097,4 @@
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
 // - invalidation (passing optional by non-const reference/pointer)
-// - `value_or(nullptr) != nullptr`, `value_or(0) != 0`, `value_or("").empty()`
 // - nested `optional` values
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -113,6 +113,41 @@
   hasArgument(1, hasOptionalType()));
 }
 
+auto isValueOrCondition(llvm::StringRef CallID) {
+  // `!opt.value_or("").empty()`.
+  auto NonEmptyStringOptional = unaryOperator(
+  hasOperatorName("!"),
+  hasUnaryOperand(cxxMemberCallExpr(
+  callee(cxxMethodDecl(hasName("empty"))),
+  onImplicitObjectArgument(ignoringImplicit(
+  cxxMemberCallExpr(on(expr(unless(cxxThisExpr(,
+callee(cxxMethodDecl(hasName("value_or"),
+ ofClass(optionalClass(,
+hasArgument(0, stringLiteral(hasSize(0
+  .bind(CallID));
+
+  auto ComparesToSame =
+  [CallID](clang::ast_matchers::internal::Matcher Arg) {
+return hasOperands(
+cxxMemberCallExpr(on(expr(unless(cxxThisExpr(,
+  callee(cxxMethodDecl(hasName("value_or"),
+  

[PATCH] D122155: Add warning when eval-method is set in the presence of value unsafe floating-point calculations.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added inline comments.
This revision now requires changes to proceed.



Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:127-129
+def warn_eval_method_setting_is_meaningless_in_value_unsafe_context : Warning<
+  "setting the eval method via '-ffp-eval-method' or '#pragma clang fp 
eval_method' "
+  "has not effect when numeric results of floating-point calculations aren't 
value-safe.">, InGroup;

I think we should split the diagnostics into two -- one as a Sema diagnostic 
(as a warning for the pragma case, in the Pragmas diagnostic group) and one as 
a Frontend diagnostic (as an error, for the command line case).

Basically, I think we should treat the command line options as being mutually 
exclusive and handle it the same as any other "these options cannot be used 
together" situation.



Comment at: clang/lib/Sema/Sema.cpp:259-260
   getLangOpts().getFPEvalMethod());
+if (getLangOpts().ApproxFunc || getLangOpts().AllowFPReassoc ||
+getLangOpts().AllowRecip)
+  // When these options are used, the compiler is allowed to make

I don't think this is the right place to diagnose this -- these are command 
line options that don't play well together, so they should be handled when 
processing command line options.


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

https://reviews.llvm.org/D122155

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

[PATCH] D122230: [clang-format] don't break up #-style comment sections

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I hit something like this today with some code which was

  //#undef

I'm not sure I think that

  // #undef

or

  //# undef

is what I want either


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122230

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

[PATCH] D122230: [clang-format] don't break up #-style comment sections

[Krasimir Georgiev 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 rGeb35e0ecbe0a: [clang-format] don't break up #-style 
comment sections (authored by krasimir).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122230

Files:
  clang/lib/Format/BreakableToken.cpp
  clang/unittests/Format/FormatTestTextProto.cpp


Index: clang/unittests/Format/FormatTestTextProto.cpp
===
--- clang/unittests/Format/FormatTestTextProto.cpp
+++ clang/unittests/Format/FormatTestTextProto.cpp
@@ -408,6 +408,23 @@
"dd: 100\n"
" another quadriple-hash comment\n",
Style));
+
+  // Ensure we support a common pattern for naming sections.
+  EXPECT_EQ("##\n"
+"# section name\n"
+"##",
+format("##\n"
+   "# section name\n"
+   "##",
+   Style));
+
+  EXPECT_EQ("///\n"
+"// section name\n"
+"///",
+format("///\n"
+   "// section name\n"
+   "///",
+   Style));
 }
 
 TEST_F(FormatTestTextProto, FormatsExtensions) {
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -785,7 +785,16 @@
 Lines[i].substr(IndentPrefix.size(), FirstCharByteSize),
 Encoding) != 1)
   return false;
-if (FirstCommentChar == '#')
+// In C-like comments, add a space before #. For example this is useful
+// to preserve the relative indentation when commenting out code with
+// #includes.
+//
+// In languages using # as the comment leader such as proto, don't
+// add a space to support patterns like:
+// #
+// # section
+// #
+if (FirstCommentChar == '#' && !TokenText.startswith("#"))
   return false;
 return FirstCommentChar == '\\' || isPunctuation(FirstCommentChar) ||
isHorizontalWhitespace(FirstCommentChar);


Index: clang/unittests/Format/FormatTestTextProto.cpp
===
--- clang/unittests/Format/FormatTestTextProto.cpp
+++ clang/unittests/Format/FormatTestTextProto.cpp
@@ -408,6 +408,23 @@
"dd: 100\n"
" another quadriple-hash comment\n",
Style));
+
+  // Ensure we support a common pattern for naming sections.
+  EXPECT_EQ("##\n"
+"# section name\n"
+"##",
+format("##\n"
+   "# section name\n"
+   "##",
+   Style));
+
+  EXPECT_EQ("///\n"
+"// section name\n"
+"///",
+format("///\n"
+   "// section name\n"
+   "///",
+   Style));
 }
 
 TEST_F(FormatTestTextProto, FormatsExtensions) {
Index: clang/lib/Format/BreakableToken.cpp
===
--- clang/lib/Format/BreakableToken.cpp
+++ clang/lib/Format/BreakableToken.cpp
@@ -785,7 +785,16 @@
 Lines[i].substr(IndentPrefix.size(), FirstCharByteSize),
 Encoding) != 1)
   return false;
-if (FirstCommentChar == '#')
+// In C-like comments, add a space before #. For example this is useful
+// to preserve the relative indentation when commenting out code with
+// #includes.
+//
+// In languages using # as the comment leader such as proto, don't
+// add a space to support patterns like:
+// #
+// # section
+// #
+if (FirstCommentChar == '#' && !TokenText.startswith("#"))
   return false;
 return FirstCommentChar == '\\' || isPunctuation(FirstCommentChar) ||
isHorizontalWhitespace(FirstCommentChar);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] eb35e0e - [clang-format] don't break up #-style comment sections

[Krasimir Georgiev via cfe-commits]

Author: Krasimir Georgiev
Date: 2022-03-22T15:29:02+01:00
New Revision: eb35e0ecbe0ae153481f1df16d91cc6001477d21

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

LOG: [clang-format] don't break up #-style comment sections

Follow-up from 
https://github.com/llvm/llvm-project/commit/36d13d3f8adb3d1a6bae71370afa23d11a94dc78;
 https://reviews.llvm.org/D121451.

Restore the old behavior in situations where we use # as comments and long 
strings of #'s for comment sections.

Reviewed By: MyDeveloperDay

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

Added: 


Modified: 
clang/lib/Format/BreakableToken.cpp
clang/unittests/Format/FormatTestTextProto.cpp

Removed: 




diff  --git a/clang/lib/Format/BreakableToken.cpp 
b/clang/lib/Format/BreakableToken.cpp
index db7361bf5f2ff..94bd9823e1f5a 100644
--- a/clang/lib/Format/BreakableToken.cpp
+++ b/clang/lib/Format/BreakableToken.cpp
@@ -785,7 +785,16 @@ BreakableLineCommentSection::BreakableLineCommentSection(
 Lines[i].substr(IndentPrefix.size(), FirstCharByteSize),
 Encoding) != 1)
   return false;
-if (FirstCommentChar == '#')
+// In C-like comments, add a space before #. For example this is useful
+// to preserve the relative indentation when commenting out code with
+// #includes.
+//
+// In languages using # as the comment leader such as proto, don't
+// add a space to support patterns like:
+// #
+// # section
+// #
+if (FirstCommentChar == '#' && !TokenText.startswith("#"))
   return false;
 return FirstCommentChar == '\\' || isPunctuation(FirstCommentChar) ||
isHorizontalWhitespace(FirstCommentChar);

diff  --git a/clang/unittests/Format/FormatTestTextProto.cpp 
b/clang/unittests/Format/FormatTestTextProto.cpp
index 44b67d275bcc3..10a43c1519d6d 100644
--- a/clang/unittests/Format/FormatTestTextProto.cpp
+++ b/clang/unittests/Format/FormatTestTextProto.cpp
@@ -408,6 +408,23 @@ TEST_F(FormatTestTextProto, UnderstandsHashComments) {
"dd: 100\n"
" another quadriple-hash comment\n",
Style));
+
+  // Ensure we support a common pattern for naming sections.
+  EXPECT_EQ("##\n"
+"# section name\n"
+"##",
+format("##\n"
+   "# section name\n"
+   "##",
+   Style));
+
+  EXPECT_EQ("///\n"
+"// section name\n"
+"///",
+format("///\n"
+   "// section name\n"
+   "///",
+   Style));
 }
 
 TEST_F(FormatTestTextProto, FormatsExtensions) {



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

[clang] 4f5640c - [CGOpenMPRuntime] Remove some uses of deprecated Address ctor

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-03-22T15:35:45+01:00
New Revision: 4f5640cad3e0cdff9ab8d6b63c9abd760c32832b

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

LOG: [CGOpenMPRuntime] Remove some uses of deprecated Address ctor

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 4e0c682723e50..66a87fd956723 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1689,10 +1689,10 @@ Address 
CGOpenMPRuntime::getAddrOfDeclareTargetVar(const VarDecl *VD) {
   OS << "_decl_tgt_ref_ptr";
 }
 llvm::Value *Ptr = CGM.getModule().getNamedValue(PtrName);
+QualType PtrTy = CGM.getContext().getPointerType(VD->getType());
+llvm::Type *LlvmPtrTy = CGM.getTypes().ConvertTypeForMem(PtrTy);
 if (!Ptr) {
-  QualType PtrTy = CGM.getContext().getPointerType(VD->getType());
-  Ptr = 
getOrCreateInternalVariable(CGM.getTypes().ConvertTypeForMem(PtrTy),
-PtrName);
+  Ptr = getOrCreateInternalVariable(LlvmPtrTy, PtrName);
 
   auto *GV = cast(Ptr);
   GV->setLinkage(llvm::GlobalValue::WeakAnyLinkage);
@@ -1701,7 +1701,7 @@ Address CGOpenMPRuntime::getAddrOfDeclareTargetVar(const 
VarDecl *VD) {
 GV->setInitializer(CGM.GetAddrOfGlobal(VD));
   registerTargetGlobalVariable(VD, cast(Ptr));
 }
-return Address::deprecated(Ptr, CGM.getContext().getDeclAlign(VD));
+return Address(Ptr, LlvmPtrTy, CGM.getContext().getDeclAlign(VD));
   }
   return Address::invalid();
 }
@@ -1730,12 +1730,12 @@ Address 
CGOpenMPRuntime::getAddrOfThreadPrivate(CodeGenFunction &CGF,
   CGF.Builder.CreatePointerCast(VDAddr.getPointer(), CGM.Int8PtrTy),
   CGM.getSize(CGM.GetTargetTypeStoreSize(VarTy)),
   getOrCreateThreadPrivateCache(VD)};
-  return Address::deprecated(
+  return Address(
   CGF.EmitRuntimeCall(
   OMPBuilder.getOrCreateRuntimeFunction(
   CGM.getModule(), OMPRTL___kmpc_threadprivate_cached),
   Args),
-  VDAddr.getAlignment());
+  CGF.Int8Ty, VDAddr.getAlignment());
 }
 
 void CGOpenMPRuntime::emitThreadPrivateVarInit(
@@ -1792,7 +1792,7 @@ llvm::Function 
*CGOpenMPRuntime::emitThreadPrivateVarDefinition(
   llvm::Value *ArgVal = CtorCGF.EmitLoadOfScalar(
   CtorCGF.GetAddrOfLocalVar(&Dst), /*Volatile=*/false,
   CGM.getContext().VoidPtrTy, Dst.getLocation());
-  Address Arg = Address::deprecated(ArgVal, VDAddr.getAlignment());
+  Address Arg(ArgVal, CtorCGF.Int8Ty, VDAddr.getAlignment());
   Arg = CtorCGF.Builder.CreateElementBitCast(
   Arg, CtorCGF.ConvertTypeForMem(ASTTy));
   CtorCGF.EmitAnyExprToMem(Init, Arg, Init->getType().getQualifiers(),
@@ -1828,9 +1828,10 @@ llvm::Function 
*CGOpenMPRuntime::emitThreadPrivateVarDefinition(
   llvm::Value *ArgVal = DtorCGF.EmitLoadOfScalar(
   DtorCGF.GetAddrOfLocalVar(&Dst),
   /*Volatile=*/false, CGM.getContext().VoidPtrTy, Dst.getLocation());
-  DtorCGF.emitDestroy(Address::deprecated(ArgVal, VDAddr.getAlignment()),
-  ASTTy, 
DtorCGF.getDestroyer(ASTTy.isDestructedType()),
-  DtorCGF.needsEHCleanup(ASTTy.isDestructedType()));
+  DtorCGF.emitDestroy(
+  Address(ArgVal, DtorCGF.Int8Ty, VDAddr.getAlignment()), ASTTy,
+  DtorCGF.getDestroyer(ASTTy.isDestructedType()),
+  DtorCGF.needsEHCleanup(ASTTy.isDestructedType()));
   DtorCGF.FinishFunction();
   Dtor = Fn;
 }
@@ -1935,11 +1936,11 @@ bool 
CGOpenMPRuntime::emitDeclareTargetVarDefinition(const VarDecl *VD,
 AddrInAS0 = llvm::ConstantExpr::getAddrSpaceCast(
 Addr, llvm::PointerType::getWithSamePointeeType(
   cast(Addr->getType()), 0));
-  CtorCGF.EmitAnyExprToMem(
-  Init,
-  Address::deprecated(AddrInAS0, CGM.getContext().getDeclAlign(VD)),
-  Init->getType().getQualifiers(),
-  /*IsInitializer=*/true);
+  CtorCGF.EmitAnyExprToMem(Init,
+   Address(AddrInAS0, Addr->getValueType(),
+   CGM.getContext().getDeclAlign(VD)),
+   Init->getType().getQualifiers(),
+   /*IsInitializer=*/true);
   CtorCGF.FinishFunction();
   Ctor = Fn;
   ID = llvm::ConstantExpr::getBitCast(Fn, CGM.Int8PtrTy);
@@ -1981,10 +1982,10 @@ bool 
CGOpenMPRuntime::emitDeclareTargetVarDefinition(const VarDecl *VD,
 AddrInAS0 = llvm::ConstantExpr::getAddrSpaceCast(
 

[PATCH] D121201: [clang] Merge the SourceRange into ParsedAttributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Sema/DeclSpec.h:2516-2521
+  void takeAttributes(ParsedAttributes &attrs) {
 Attrs.takeAllFrom(attrs);
 
-if (!lastLoc.isInvalid())
-  SetRangeEnd(lastLoc);
+if (attrs.Range.getEnd().isValid())
+  SetRangeEnd(attrs.Range.getEnd());
   }

tbaeder wrote:
> aaron.ballman wrote:
> > 
> I blindly changed this and it took me a while to figure out that's wrong from 
> the test failures: 
> 
> `Attrs.takeAllFrom(Attrs)`...
Oh god, I'm so sorry for that terrible suggestion, I hadn't spotted I was 
reusing the name. Feel free to go with `A` or `PA` or something for the 
parameter name to avoid that conflict.


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

https://reviews.llvm.org/D121201

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

[PATCH] D122111: [clang-tidy] Fix false positives in `misc-redundant-expression` check

[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! Please also add a release note for the fix when you land it.




Comment at: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp:605
+  unless(isMacro()), unless(isInTemplateInstantiation()),
+  hasRHS(ignoringParenImpCasts(integerLiteral().bind(ConstId
   .bind(OverloadId);

fwolff wrote:
> aaron.ballman wrote:
> > This seems incorrect to me -- the LHS and RHS can be swapped along with 
> > which operator is used and still achieve the same semantic results.
> > 
> > While playing around to test the behavior we currently have, I was a bit 
> > surprised at the difference in results here: 
> > https://godbolt.org/z/hE4qfca1b.
> > 
> > Also, do we need to bind? Nothing seems to look at that binding that I can 
> > see.
> > This seems incorrect to me -- the LHS and RHS can be swapped along with 
> > which operator is used and still achieve the same semantic results.
> 
> Fixed.
> 
> > While playing around to test the behavior we currently have, I was a bit 
> > surprised at the difference in results here: 
> > https://godbolt.org/z/hE4qfca1b.
> 
> All four conditions get a warning with my changes. I've added your example to 
> the test file.
> 
> > Also, do we need to bind? Nothing seems to look at that binding that I can 
> > see.
> 
> Yes, it is hard to see, but [[ 
> https://github.com/llvm/llvm-project/blob/240e06dfe77feabe38a03cbb1c94875639d0f9ff/clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp#L498
>  | here ]] the constant is actually read.
> Yes, it is hard to see, but here the constant is actually read.

Oh, thanks for the info!


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

https://reviews.llvm.org/D122111

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

[PATCH] D120228: [RISCV] Add policy operand for masked compare and vmsbf/vmsif/vmsof IR intrinsics.

[Zakk Chen via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG10fd2822b77e: [RISCV] Add policy operand for masked compare 
and vmsbf/vmsif/vmsof IR (authored by khchen).
Herald added a subscriber: StephenFan.

Changed prior to commit:
  https://reviews.llvm.org/D120228?vs=416057&id=417297#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120228

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmsbf.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmsif.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded/vmsof.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmsbf.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmsif.c
  clang/test/CodeGen/RISCV/rvv-intrinsics/vmsof.c
  llvm/include/llvm/IR/IntrinsicsRISCV.td
  llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
  llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
  llvm/test/CodeGen/RISCV/rvv/masked-tama.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tamu.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tuma.ll
  llvm/test/CodeGen/RISCV/rvv/masked-tumu.ll
  llvm/test/CodeGen/RISCV/rvv/vmfeq.ll
  llvm/test/CodeGen/RISCV/rvv/vmfge.ll
  llvm/test/CodeGen/RISCV/rvv/vmfgt.ll
  llvm/test/CodeGen/RISCV/rvv/vmfle.ll
  llvm/test/CodeGen/RISCV/rvv/vmflt.ll
  llvm/test/CodeGen/RISCV/rvv/vmfne.ll
  llvm/test/CodeGen/RISCV/rvv/vmsbf.ll
  llvm/test/CodeGen/RISCV/rvv/vmseq-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmseq-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsge-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsge-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgeu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgeu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgt-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgt-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgtu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsgtu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsif.ll
  llvm/test/CodeGen/RISCV/rvv/vmsle-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsle-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsleu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsleu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmslt-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmslt-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsltu-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsltu-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsne-rv32.ll
  llvm/test/CodeGen/RISCV/rvv/vmsne-rv64.ll
  llvm/test/CodeGen/RISCV/rvv/vmsof.ll

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

[PATCH] D122232: [clang][NFC] Refactor logic for picking standard library on Apple

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: egorzhdan.
Herald added a project: All.
ldionne requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Flip the logic around: always default to libc++ except on older platforms,
instead of defaulting to libstdc++ except on newer platforms. Since roughly
all supported platforms use libc++ now, it makes more sense to make that
the default, and allows the removal of some downstream diff.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122232

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


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -869,13 +869,13 @@
 bool MachO::HasNativeLLVMSupport() const { return true; }
 
 ToolChain::CXXStdlibType Darwin::GetDefaultCXXStdlibType() const {
-  // Default to use libc++ on OS X 10.9+ and iOS 7+.
-  if ((isTargetMacOSBased() && !isMacosxVersionLT(10, 9)) ||
-  (isTargetIOSBased() && !isIPhoneOSVersionLT(7, 0)) ||
-  isTargetWatchOSBased())
-return ToolChain::CST_Libcxx;
+  // Use libstdc++ on old targets (OSX < 10.9 and iOS < 7)
+  if ((isTargetMacOSBased() && isMacosxVersionLT(10, 9)) ||
+  (isTargetIOSBased() && isIPhoneOSVersionLT(7, 0)))
+return ToolChain::CST_Libstdcxx;
 
-  return ToolChain::CST_Libstdcxx;
+  // On all other targets, use libc++
+  return ToolChain::CST_Libcxx;
 }
 
 /// Darwin provides an ARC runtime starting in MacOS X 10.7 and iOS 5.0.


Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -869,13 +869,13 @@
 bool MachO::HasNativeLLVMSupport() const { return true; }
 
 ToolChain::CXXStdlibType Darwin::GetDefaultCXXStdlibType() const {
-  // Default to use libc++ on OS X 10.9+ and iOS 7+.
-  if ((isTargetMacOSBased() && !isMacosxVersionLT(10, 9)) ||
-  (isTargetIOSBased() && !isIPhoneOSVersionLT(7, 0)) ||
-  isTargetWatchOSBased())
-return ToolChain::CST_Libcxx;
+  // Use libstdc++ on old targets (OSX < 10.9 and iOS < 7)
+  if ((isTargetMacOSBased() && isMacosxVersionLT(10, 9)) ||
+  (isTargetIOSBased() && isIPhoneOSVersionLT(7, 0)))
+return ToolChain::CST_Libstdcxx;
 
-  return ToolChain::CST_Libstdcxx;
+  // On all other targets, use libc++
+  return ToolChain::CST_Libcxx;
 }
 
 /// Darwin provides an ARC runtime starting in MacOS X 10.7 and iOS 5.0.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120134: [analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers updated this revision to Diff 417300.
vabridgers added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120134

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp


Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -742,9 +742,6 @@
   // This change is needed for architectures with varying
   // pointer widths. See the amdgcn opencl reproducer with
   // this change as an example: solver-sym-simplification-ptr-bool.cl
-  // FIXME: Cleanup remainder of `getZeroWithPtrWidth ()`
-  //and `getIntWithPtrWidth()` functions to prevent future
-  //confusion
   if (!Ty->isReferenceType())
 return makeNonLoc(Sym, BO_NE, BasicVals.getZeroWithTypeSize(Ty),
   CastTy);
Index: clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -1344,8 +1344,9 @@
 case Stmt::GNUNullExprClass: {
   // GNU __null is a pointer-width integer, not an actual pointer.
   ProgramStateRef state = Pred->getState();
-  state = state->BindExpr(S, Pred->getLocationContext(),
-  svalBuilder.makeIntValWithPtrWidth(0, false));
+  state = state->BindExpr(
+  S, Pred->getLocationContext(),
+  svalBuilder.makeIntValWithWidth(getContext().VoidPtrTy, 0));
   Bldr.generateNode(S, Pred, state);
   break;
 }
Index: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2607,9 +2607,9 @@
 return nullptr;
 
   // Compare the size argument to 0.
-  DefinedOrUnknownSVal SizeZero =
-svalBuilder.evalEQ(State, TotalSize.castAs(),
-   svalBuilder.makeIntValWithPtrWidth(0, false));
+  DefinedOrUnknownSVal SizeZero = svalBuilder.evalEQ(
+  State, TotalSize.castAs(),
+  svalBuilder.makeIntValWithWidth(svalBuilder.getArrayIndexType(), 0));
 
   ProgramStateRef StatePtrIsNull, StatePtrNotNull;
   std::tie(StatePtrIsNull, StatePtrNotNull) = State->assume(PtrEQ);
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -332,9 +332,8 @@
 return nonloc::ConcreteInt(BasicVals.getIntValue(integer, isUnsigned));
   }
 
-  NonLoc makeIntValWithPtrWidth(uint64_t integer, bool isUnsigned) {
-return nonloc::ConcreteInt(
-BasicVals.getIntWithPtrWidth(integer, isUnsigned));
+  NonLoc makeIntValWithWidth(QualType ptrType, uint64_t integer) {
+return nonloc::ConcreteInt(BasicVals.getValue(integer, ptrType));
   }
 
   NonLoc makeLocAsInteger(Loc loc, unsigned bits) {
Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -220,14 +220,6 @@
 return getValue(0, Ctx.getTypeSize(T), true);
   }
 
-  const llvm::APSInt &getZeroWithPtrWidth(bool isUnsigned = true) {
-return getValue(0, Ctx.getTypeSize(Ctx.VoidPtrTy), isUnsigned);
-  }
-
-  const llvm::APSInt &getIntWithPtrWidth(uint64_t X, bool isUnsigned) {
-return getValue(X, Ctx.getTypeSize(Ctx.VoidPtrTy), isUnsigned);
-  }
-
   const llvm::APSInt &getTruthValue(bool b, QualType T) {
 return getValue(b ? 1 : 0, Ctx.getIntWidth(T),
 T->isUnsignedIntegerOrEnumerationType());


Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -742,9 +742,6 @@
   // This change is needed for architectures with varying
   // pointer widths. See the amdgcn opencl reproducer with
   // this change as an example: solver-sym-simplification-ptr-bool.cl
-  // FIXME: Cleanup remainder of `getZeroWithPtrWidth ()`
-  //and `getIntWithPtrWidth()` functions to prevent future
-  //c

[PATCH] D120134: [analyzer] refactor makeIntValWithPtrWidth, remove getZeroWithPtrWidth (NFC)

[Vince Bridgers via Phabricator via cfe-commits]

vabridgers added a comment.

ping! @steakhal accepted, should we wait for @NoQ before landing? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120134

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

[PATCH] D121201: [clang] Merge the SourceRange into ParsedAttributes

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

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



Comment at: clang/include/clang/Sema/DeclSpec.h:2516-2521
+  void takeAttributes(ParsedAttributes &attrs) {
 Attrs.takeAllFrom(attrs);
 
-if (!lastLoc.isInvalid())
-  SetRangeEnd(lastLoc);
+if (attrs.Range.getEnd().isValid())
+  SetRangeEnd(attrs.Range.getEnd());
   }

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > 
> > I blindly changed this and it took me a while to figure out that's wrong 
> > from the test failures: 
> > 
> > `Attrs.takeAllFrom(Attrs)`...
> Oh god, I'm so sorry for that terrible suggestion, I hadn't spotted I was 
> reusing the name. Feel free to go with `A` or `PA` or something for the 
> parameter name to avoid that conflict.
Haha, no problem. Do you think adding an assertion for this case to 
`takeAllFrom()` (and `takeOneFrom()`) makes sense?


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

https://reviews.llvm.org/D121201

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

[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 417303.
pengfei added a comment.

Address review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107141

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/Sema/asm.c

Index: clang/test/Sema/asm.c
===
--- clang/test/Sema/asm.c
+++ clang/test/Sema/asm.c
@@ -313,3 +313,54 @@
   asm ("jne %l0":::);
   asm goto ("jne %l0"lab);
 }
+
+typedef struct _st_size64 {
+  int a;
+  char b;
+} st_size64;
+
+typedef struct _st_size96 {
+  int a;
+  int b;
+  int c;
+} st_size96;
+
+typedef struct _st_size16 {
+  char a;
+  char b;
+} st_size16;
+
+typedef struct _st_size32 {
+  char a;
+  char b;
+  char c;
+  char d;
+} st_size32;
+
+typedef struct _st_size128 {
+  int a;
+  int b;
+  int c;
+  int d;
+} st_size128;
+
+void test19(long long x)
+{
+  st_size64 a;
+  st_size96 b;
+  st_size16 c;
+  st_size32 d;
+  st_size128 e;
+  asm ("" : "=rm" (a): "0" (1)); // no-error
+  asm ("" : "=rm" (d): "0" (1)); // no-error
+  asm ("" : "=rm" (c): "0" (x)); // no-error
+  // FIXME: This case is actually supported by codegen.
+  asm ("" : "=rm" (x): "0" (a)); // expected-error {{unsupported inline asm: input with type 'st_size64' (aka 'struct _st_size64') matching output with type 'long long'}}
+  // FIXME: This case is actually supported by codegen.
+  asm ("" : "=rm" (a): "0" (d)); // expected-error {{unsupported inline asm: input with type 'st_size32' (aka 'struct _st_size32') matching output with type 'st_size64' (aka 'struct _st_size64')}}
+  asm ("" : "=rm" (b): "0" (1)); // expected-error {{impossible constraint in asm: can't store value into a register}}
+  // FIXME: This case should be supported by codegen, but it fails now.
+  asm ("" : "=rm" (e): "0" (1)); // no-error
+  // FIXME: This case should be supported by codegen, but it fails now.
+  asm ("" : "=rm" (x): "0" (e)); // expected-error {{unsupported inline asm: input with type 'st_size128' (aka 'struct _st_size128') matching output with type 'long long'}}
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -667,8 +667,17 @@
 // output was a register, just extend the shorter one to the size of the
 // larger one.
 if (!SmallerValueMentioned && InputDomain != AD_Other &&
-OutputConstraintInfos[TiedTo].allowsRegister())
+OutputConstraintInfos[TiedTo].allowsRegister()) {
+  // FIXME: GCC supports the OutSize to be 128 at maximum. Currently codegen
+  // crash when the size larger than the register size. So we limit it here.
+  if (OutTy->isStructureType() &&
+  Context.getIntTypeForBitwidth(OutSize, /*Signed*/ false).isNull()) {
+targetDiag(OutputExpr->getExprLoc(), diag::err_store_value_to_reg);
+return NS;
+  }
+
   continue;
+}
 
 // Either both of the operands were mentioned or the smaller one was
 // mentioned.  One more special case that we'll allow: if the tied input is
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -2725,9 +2725,8 @@
   QualType Ty = getContext().getIntTypeForBitwidth(Size, /*Signed*/ false);
   if (Ty.isNull()) {
 const Expr *OutExpr = S.getOutputExpr(i);
-CGM.Error(
-OutExpr->getExprLoc(),
-"impossible constraint in asm: can't store value into a register");
+CGM.getDiags().Report(OutExpr->getExprLoc(),
+  diag::err_store_value_to_reg);
 return;
   }
   Dest = MakeAddrLValue(A, Ty);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -8763,6 +8763,8 @@
 " in asm %select{input|output}1 with a memory constraint '%2'">;
   def err_asm_input_duplicate_match : Error<
 "more than one input constraint matches the same output '%0'">;
+  def err_store_value_to_reg : Error<
+"impossible constraint in asm: can't store value into a register">;
 
   def warn_asm_label_on_auto_decl : Warning<
 "ignored asm label '%0' on automatic variable">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D107141: [Inline-asm] Add diagnosts for unsupported inline assembly arguments

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/lib/Sema/SemaStmtAsm.cpp:622
+if (InTy->isIntegerType() || InTy->isPointerType() ||
+InTy->isStructureType() || InTy->isConstantArrayType())
   InputDomain = AD_Int;

jyu2 wrote:
> pengfei wrote:
> > jyu2 wrote:
> > > Are you sure you want to change the Input/output Domain?  Since you 
> > > changed this, could you add both codegen and sema check tests for  struct 
> > > type(you already has sema check for struct type, but I don't see any 
> > > array type) and array type.  
> > > 
> > > Thanks.
> > > Jennifer
> > The input / output domain are just for sema check here. I don't think it 
> > changes codegen behavior.
> Yes, for seam check, once error emit, compiler stop at CodeGen.  However, if 
> no error emit, compiler will go though the CodeGen.
> 
> Since pointer/struct and constantArray are allowed for small size of type, so 
> would you please add code gen test for that?
> 
> In your test where no-error should emit part...
> 
> asm ("" : "=rm" (a): "0" (1)); // no-error
> 
> Thanks.
> Jennifer
Thanks Jennifer! I see your point. I'll only add sema check (rather than loose 
in some way) in this patch.
The codegen part is complicated, some cases will pass but some still fail. I 
have left comments in the tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107141

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

[PATCH] D121201: [clang] Merge the SourceRange into ParsedAttributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Sema/DeclSpec.h:2516-2521
+  void takeAttributes(ParsedAttributes &attrs) {
 Attrs.takeAllFrom(attrs);
 
-if (!lastLoc.isInvalid())
-  SetRangeEnd(lastLoc);
+if (attrs.Range.getEnd().isValid())
+  SetRangeEnd(attrs.Range.getEnd());
   }

tbaeder wrote:
> aaron.ballman wrote:
> > tbaeder wrote:
> > > aaron.ballman wrote:
> > > > 
> > > I blindly changed this and it took me a while to figure out that's wrong 
> > > from the test failures: 
> > > 
> > > `Attrs.takeAllFrom(Attrs)`...
> > Oh god, I'm so sorry for that terrible suggestion, I hadn't spotted I was 
> > reusing the name. Feel free to go with `A` or `PA` or something for the 
> > parameter name to avoid that conflict.
> Haha, no problem. Do you think adding an assertion for this case to 
> `takeAllFrom()` (and `takeOneFrom()`) makes sense?
An assertion that the attributes are actually taken from the argument (so 
validating the size of the container after taking from it)? Probably wouldn't 
hurt.


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

https://reviews.llvm.org/D121201

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

[PATCH] D121201: [clang] Merge the SourceRange into ParsedAttributes

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Sema/DeclSpec.h:2516-2521
+  void takeAttributes(ParsedAttributes &attrs) {
 Attrs.takeAllFrom(attrs);
 
-if (!lastLoc.isInvalid())
-  SetRangeEnd(lastLoc);
+if (attrs.Range.getEnd().isValid())
+  SetRangeEnd(attrs.Range.getEnd());
   }

aaron.ballman wrote:
> tbaeder wrote:
> > aaron.ballman wrote:
> > > tbaeder wrote:
> > > > aaron.ballman wrote:
> > > > > 
> > > > I blindly changed this and it took me a while to figure out that's 
> > > > wrong from the test failures: 
> > > > 
> > > > `Attrs.takeAllFrom(Attrs)`...
> > > Oh god, I'm so sorry for that terrible suggestion, I hadn't spotted I was 
> > > reusing the name. Feel free to go with `A` or `PA` or something for the 
> > > parameter name to avoid that conflict.
> > Haha, no problem. Do you think adding an assertion for this case to 
> > `takeAllFrom()` (and `takeOneFrom()`) makes sense?
> An assertion that the attributes are actually taken from the argument (so 
> validating the size of the container after taking from it)? Probably wouldn't 
> hurt.
I'd suggest more of a 'Make sure other side is not this side' :D 



Comment at: clang/include/clang/Sema/ParsedAttr.h:813
   void takeAllFrom(AttributePool &pool) {
 takePool(pool);
 pool.Attrs.clear();

so: `assert(&pool != this && "Stealing from yourself?  Super bad...")`.


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

https://reviews.llvm.org/D121201

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

[PATCH] D119721: [clang][lex] Use `ConstSearchDirIterator` in lookup cache

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added subscribers: rnk, sberg.
sberg added a comment.
Herald added a project: All.

I started to experience Clang crashing when building Firebird (as part of 
building LibreOffice) in clang-cl mode on Windows, and I think it is due to 
this change in combination with D2733  by @rnk:

When `HeaderSearch::LookupFile` exits through one of the `if 
(checkMSVCHeaderSearch...` branches introduced in D2733 
, it does not update `CacheLookup.HitIt` 
(through one of the calls to `cacheLookupSuccess` or via the "Otherwise, didn't 
find it" step at the end of the function), even though it already may have done 
the "We will fill in our found location below, so prime the start point value" 
step of calling `CacheLookup.reset`.  That means that a later call to 
`HeaderSearch::LookupFile` can go into the `if (!SkipCache && 
CacheLookup.StartIt == NextIt)` true branch, set `It = CacheLookup.HitIt` to an 
invalid iterator (the default `HitIt = nullptr` value), and then assert/crash 
when dereferencing that invalid `It`.

Which was not an issue before this change, as the initial `HitIdx = 0` was not 
an invalid value, so `i = CacheLookup.HitIdx` never caused `i` to become 
invalid.

I locally worked around this in my build with

  -  if (!SkipCache && CacheLookup.StartIt == NextIt) {
  +  if (!SkipCache && CacheLookup.StartIt == NextIt && CacheLookup.HitIt) {

which seems to work well, but I'm not sure what the best real fix would be.  
(D2733  stated "I believe the correct behavior 
here is to avoid updating the cache when we find the header via MSVC's search 
rules.")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119721

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

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

[Matthias Gehre via Phabricator via cfe-commits]

mgehre-amd created this revision.
mgehre-amd added reviewers: aaron.ballman, erichkeane.
Herald added subscribers: luke957, s.egerton, mstorsjo, simoncook, 
fedor.sergeev, dschuff.
Herald added a project: All.
mgehre-amd requested review of this revision.
Herald added subscribers: pcwang-thead, aheejin.
Herald added a project: clang.

According to the RFC [0], this review contains the clang parts of large integer 
divison for _BitInt.

It contains the following parts:

- Driver: Gnu, MinGW: Link libbitint when available
- clang/Basic/TargetInfo: Increase getMaxBitIntWidth to 
llvm::IntegerType::MAX_INT_BITS
- Sema: Diagnose when converting _BitInt to/from floating point for bit width > 
128 because that breaks in the backend
- Lex: Speedup parsing of large integer literals with a power-of-two radix, so 
parsing a hex literal with 2097152 digits (=MAX_INT_BITS) doesn't take forever

[0] 
https://discourse.llvm.org/t/rfc-add-support-for-division-of-large-bitint-builtins-selectiondag-globalisel-clang/60329


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122234

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Lex/LiteralSupport.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/test/CodeGen/ext-int-cc.c
  clang/test/Driver/linux-ld.c
  clang/test/Lexer/bitint-constants.c
  clang/test/Preprocessor/init-aarch64.c
  clang/test/Preprocessor/init.c
  clang/test/Sema/ext-int.c
  clang/test/SemaCXX/ext-int.cpp

Index: clang/test/SemaCXX/ext-int.cpp
===
--- clang/test/SemaCXX/ext-int.cpp
+++ clang/test/SemaCXX/ext-int.cpp
@@ -17,7 +17,7 @@
   unsigned _BitInt(5) e = 5;
   _BitInt(5) unsigned f;
 
-  _BitInt(-3) g; // expected-error{{signed _BitInt of bit sizes greater than 128 not supported}}
+  _BitInt(-3) g; // expected-error{{signed _BitInt of bit sizes greater than 8388608 not supported}}
   _BitInt(0) h; // expected-error{{signed _BitInt must have a bit size of at least 2}}
   _BitInt(1) i; // expected-error{{signed _BitInt must have a bit size of at least 2}}
   _BitInt(2) j;;
@@ -29,12 +29,12 @@
   constexpr _BitInt(7) o = 33;
 
   // Check imposed max size.
-  _BitInt(129) p;   // expected-error {{signed _BitInt of bit sizes greater than 128 not supported}}
-  unsigned _BitInt(0xFF) q; // expected-error {{unsigned _BitInt of bit sizes greater than 128 not supported}}
+  _BitInt(8388609) p;// expected-error {{signed _BitInt of bit sizes greater than 8388608 not supported}}
+  unsigned _BitInt(0xFF) q; // expected-error {{unsigned _BitInt of bit sizes greater than 8388608 not supported}}
 
 // Ensure template params are instantiated correctly.
-  // expected-error@5{{signed _BitInt of bit sizes greater than 128 not supported}}
-  // expected-error@6{{unsigned _BitInt of bit sizes greater than 128 not supported}}
+  // expected-error@5{{signed _BitInt of bit sizes greater than 8388608 not supported}}
+  // expected-error@6{{unsigned _BitInt of bit sizes greater than 8388608 not supported}}
   // expected-note@+1{{in instantiation of template class }}
   HasExtInt<-1> r;
   // expected-error@5{{signed _BitInt must have a bit size of at least 2}}
@@ -285,3 +285,16 @@
 void FromPaper2(_BitInt(8) a1, _BitInt(24) a2) {
   static_assert(is_same::value, "");
 }
+
+void LargeBitIntToFloat(_BitInt(129) a) {
+  float fa = a; // expected-error {{cannot convert '_BitInt' operands of more than 128 bits to floating point}}
+  fa = static_cast(a); // expected-error {{cannot convert '_BitInt' operands of more than 128 bits to floating point}}
+  fa = a + 0.1; // expected-error {{cannot convert '_BitInt' operands of more than 128 bits to floating point}}
+}
+
+_BitInt(129) LargeBitIntFromFloat(float f) {
+  _BitInt(129) a = f; // expected-error {{cannot convert floating point operands to a '_BitInt' of more than 128 bits}}
+  a = f; // expected-error {{cannot convert floating point operands to a '_BitInt' of more than 128 bits}}
+  a = static_cast<_BitInt(129)>(f); // expected-error {{cannot convert floating point operands to a '_BitInt' of more than 128 bits}}
+  return f; // expected-error {{cannot convert floating point operands to a '_BitInt' of more than 128 bits}}
+}
Index: clang/test/Sema/ext-int.c
===
--- clang/test/Sema/ext-int.c
+++ clang/test/Sema/ext-int.c
@@ -73,3 +73,16 @@
 void FromPaper2(_BitInt(8) a1, _BitInt(24) a2) {
   _Static_assert(EXPR_HAS_TYPE(a1 * (_BitInt(32))a2, _BitInt(32)), "");
 }
+
+
+void LargeBitIntToFloat(_BitInt(129) a) {
+  float fa = a; // expected-error {{cannot convert '_BitInt' operands of more than 128 bits to floating point}}

[clang] cd6d9ae - [CGOpenMPRuntime] Remove some uses of deprecated Adddress ctor

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-03-22T16:29:35+01:00
New Revision: cd6d9ae26313258d4dbf8a5db4d977434c03ed6a

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

LOG: [CGOpenMPRuntime] Remove some uses of deprecated Adddress ctor

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 66a87fd956723..24cd78a64cfd4 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6232,12 +6232,12 @@ Address 
CGOpenMPRuntime::getTaskReductionItem(CodeGenFunction &CGF,
  ReductionsPtr,
  CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
  SharedLVal.getPointer(CGF), CGM.VoidPtrTy)};
-  return Address::deprecated(
+  return Address(
   CGF.EmitRuntimeCall(
   OMPBuilder.getOrCreateRuntimeFunction(
   CGM.getModule(), OMPRTL___kmpc_task_reduction_get_th_data),
   Args),
-  SharedLVal.getAlignment());
+  CGF.Int8Ty, SharedLVal.getAlignment());
 }
 
 void CGOpenMPRuntime::emitTaskwaitCall(CodeGenFunction &CGF, SourceLocation 
Loc,
@@ -9053,16 +9053,13 @@ class MappableExprsHandler {
   void generateInfoForLambdaCaptures(
   const ValueDecl *VD, llvm::Value *Arg, MapCombinedInfoTy &CombinedInfo,
   llvm::DenseMap &LambdaPointers) const {
-const auto *RD = VD->getType()
- .getCanonicalType()
- .getNonReferenceType()
- ->getAsCXXRecordDecl();
+QualType VDType = VD->getType().getCanonicalType().getNonReferenceType();
+const auto *RD = VDType->getAsCXXRecordDecl();
 if (!RD || !RD->isLambda())
   return;
-Address VDAddr =
-Address::deprecated(Arg, CGF.getContext().getDeclAlign(VD));
-LValue VDLVal = CGF.MakeAddrLValue(
-VDAddr, VD->getType().getCanonicalType().getNonReferenceType());
+Address VDAddr(Arg, CGF.ConvertTypeForMem(VDType),
+   CGF.getContext().getDeclAlign(VD));
+LValue VDLVal = CGF.MakeAddrLValue(VDAddr, VDType);
 llvm::DenseMap Captures;
 FieldDecl *ThisCapture = nullptr;
 RD->getCaptureFields(Captures, ThisCapture);
@@ -10622,13 +10619,13 @@ void CGOpenMPRuntime::emitTargetCall(
 
 InputInfo.NumberOfTargetItems = Info.NumberOfPtrs;
 InputInfo.BasePointersArray =
-Address::deprecated(Info.BasePointersArray, CGM.getPointerAlign());
+Address(Info.BasePointersArray, CGF.VoidPtrTy, CGM.getPointerAlign());
 InputInfo.PointersArray =
-Address::deprecated(Info.PointersArray, CGM.getPointerAlign());
+Address(Info.PointersArray, CGF.VoidPtrTy, CGM.getPointerAlign());
 InputInfo.SizesArray =
-Address::deprecated(Info.SizesArray, CGM.getPointerAlign());
+Address(Info.SizesArray, CGF.Int64Ty, CGM.getPointerAlign());
 InputInfo.MappersArray =
-Address::deprecated(Info.MappersArray, CGM.getPointerAlign());
+Address(Info.MappersArray, CGF.VoidPtrTy, CGM.getPointerAlign());
 MapTypesArray = Info.MapTypesArray;
 MapNamesArray = Info.MapNamesArray;
 if (RequiresOuterTask)
@@ -11505,13 +11502,13 @@ void CGOpenMPRuntime::emitTargetDataStandAloneCall(
 {/*ForEndCall=*/false});
 InputInfo.NumberOfTargetItems = Info.NumberOfPtrs;
 InputInfo.BasePointersArray =
-Address::deprecated(Info.BasePointersArray, CGM.getPointerAlign());
+Address(Info.BasePointersArray, CGF.VoidPtrTy, CGM.getPointerAlign());
 InputInfo.PointersArray =
-Address::deprecated(Info.PointersArray, CGM.getPointerAlign());
+Address(Info.PointersArray, CGF.VoidPtrTy, CGM.getPointerAlign());
 InputInfo.SizesArray =
-Address::deprecated(Info.SizesArray, CGM.getPointerAlign());
+Address(Info.SizesArray, CGF.Int64Ty, CGM.getPointerAlign());
 InputInfo.MappersArray =
-Address::deprecated(Info.MappersArray, CGM.getPointerAlign());
+Address(Info.MappersArray, CGF.VoidPtrTy, CGM.getPointerAlign());
 MapTypesArray = Info.MapTypesArray;
 MapNamesArray = Info.MapNamesArray;
 if (RequiresOuterTask)
@@ -12362,9 +12359,10 @@ Address 
CGOpenMPRuntime::getAddressOfLocalVariable(CodeGenFunction &CGF,
 CGF.EmitRuntimeCall(RTLFn, Args);
   }
 };
-Address VDAddr = UntiedRealAddr.isValid()
- ? UntiedRealAddr
- : Address::deprecated(Addr, Align);
+Address VDAddr =
+UntiedRealAddr.isValid()
+? UntiedRealAddr
+: Address(Addr, CGF.ConvertTypeForMem(CVD->getType()), Align);
 CGF.EHStack.pushCleanup(
 NormalAndEHCleanup, FiniRTLFn, CVD->

[PATCH] D122234: [clang] Link libbitint for large division of _BitInt; increase max _BitInt size

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8422
 
+def err_int_to_float_bit_int_max_size : Error<
+  "cannot convert '_BitInt' operands of more than %0 bits to floating point">;

Can you explain the issue here?  This is supposed to be well-defined behavior.



Comment at: clang/include/clang/Basic/TargetInfo.h:599
   virtual size_t getMaxBitIntWidth() const {
-// 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.
-return 128;
+// TODO: Return 128 for targets that don't link libbitint?
+return llvm::IntegerType::MAX_INT_BITS;

This is definitely a TODO we need to do before accepting this.  The purpose of 
this function is that each individual target can 'override' this function.  
This is an 'opt in'.



Comment at: clang/lib/Lex/LiteralSupport.cpp:1232
 
+  if (llvm::isPowerOf2_32(radix)) {
+unsigned BitsPerDigit = llvm::Log2(radix);

Can you explain what is going on here?  This isn't at all obvious to me.


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] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: erichkeane.
aaron.ballman added a comment.

Adding Erich as he's been staring at the template instantiation code recently 
and may have thoughts/opinions. To me, this seems like the correct approach 
though.




Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2536
 
-  TypeSourceInfo *BaseTypeLoc = SubstType(Base.getTypeSourceInfo(),
-  TemplateArgs,
-  Base.getSourceRange().getBegin(),
-  DeclarationName());
+BaseTypeLoc =
+SubstType(Base.getTypeSourceInfo(), TemplateArgs,

RKSimon wrote:
> I'm not familiar with this code at all - but we've gone from having a local 
> shadow BaseTypeLoc variable to reusing the BaseTypeLoc declared at line#2510 
> - is that what we actually want?
I don't think it matters. After this `for` loop terminates, we `continue` the 
outer loop. That said, this does make the code harder to read, we may want to 
use a local instead of reusing the outer variable like this.



Comment at: clang/test/SemaCXX/template-base-class-pack-expansion.cpp:2
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// Don't crash (#53609).
+

Can we test more than just not crashing? (e.g., can we test that the expansion 
happened properly?) Perhaps an AST dump test, at a minimum?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119063

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

[PATCH] D121201: [clang] Merge the SourceRange into ParsedAttributes

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

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



Comment at: clang/include/clang/Sema/ParsedAttr.h:813
   void takeAllFrom(AttributePool &pool) {
 takePool(pool);
 pool.Attrs.clear();

erichkeane wrote:
> so: `assert(&pool != this && "Stealing from yourself?  Super bad...")`.
RIght, that's what I had locally.


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

https://reviews.llvm.org/D121201

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

[PATCH] D119063: [SemaCXX] Properly scope ArgumentPackSubstitutionIndex when expanding base types

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I think this patch makes sense to me.  I Do like the ideas of making sure these 
instantiate properly though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119063

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

[PATCH] D121916: [clang-format] [doc] Add script to automatically update help output in ClangFormat.rst.

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/docs/tools/dump_format_help.py:29
+def get_help_output():
+args = ["clang-format", "--help"]
+cmd = subprocess.Popen(args, stdout=subprocess.PIPE,

curdeius wrote:
> sstwcw wrote:
> > You intentionally did not write `build/bin/clang-format` to accommodate 
> > people who build in other directories, right? Sorry this comment is late.
> I wanted to be coherent with the `generate_formatted_state.py` script that 
> calls `clang-format` from PATH as well.
> A user that builds elsewhere needs to set PATH when executing the script to 
> use clang-format of their choice.
you definitely can't assume "build", for me my build directory is build_ninja




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121916

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

[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:69
+def note_constexpr_static_local : Note<
+  "control flows through the declaration of a %select{static|thread_local}0 
variable">;
 def note_constexpr_subobject_declared_here : Note<

erichkeane wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > Hmm... this feels a little awkward to me.  Though, I usually have Aaron 
> > > do the bikeshedding, so if he didn't come up with better I guess we can 
> > > let it go.
> > I didn't think it was overly awkward, about the only question I had was 
> > "should we quote the `static` and `thread_local` as keywords?" when I 
> > reviewed.
> Ok, I can live with that.  AS far as quoting, I think we are consistently 
> inconsistent on that one.
Minor nit:
The code already behaves as-if the resolution of CWG 2552 is applied. Let's use 
the corresponding wording.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400

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

[PATCH] D121927: [Clang] Work with multiple pragmas weak before definition

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Can you also add a release note for the fix?




Comment at: clang/include/clang/Sema/Sema.h:1075-1077
   /// WeakUndeclaredIdentifiers - Identifiers contained in
   /// \#pragma weak before declared. rare. may alias another
   /// identifier, declared or undeclared

Should probably update this comment as the type just got much weirder than 
before.



Comment at: clang/include/clang/Sema/Weak.h:18-19
 #include "clang/Basic/SourceLocation.h"
 
+#include "llvm/ADT/DenseMapInfo.h"
+





Comment at: clang/include/clang/Sema/Weak.h:27-30
   IdentifierInfo *alias;  // alias (optional)
   SourceLocation loc; // for diagnostics
-  bool used;  // identifier later declared?
 public:
+  WeakInfo() : alias(nullptr), loc(SourceLocation()) {}





Comment at: clang/include/clang/Sema/Weak.h:33
+  : alias(Alias), loc(Loc) {}
+  inline IdentifierInfo *getAlias() const { return alias; }
   inline SourceLocation getLocation() const { return loc; }

Would it be onerous to make the return type be `const IdentifierInfo *` given 
that the function is `const`? (If it is, just ignore the suggestion -- I love 
adding const correctness where we can get it basically for free.)



Comment at: clang/include/clang/Sema/Weak.h:62
+return false;
+  return LHS.getAlias()->getName() == RHS.getAlias()->getName();
+}

Previously we cared about the source locations being the same. But... is there 
a situation where the aliases are the same but the source locations are 
different? I can't think of any, so perhaps that's worth an assert that the 
locations match?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121927

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

[PATCH] D121560: [clang][Opt] Add NoArgUnusedWith to not warn for unused redundant options

[Alex Brachet via Phabricator via cfe-commits]

abrachet updated this revision to Diff 417313.
abrachet marked an inline comment as done.
abrachet added a comment.
Herald added subscribers: pmatos, asb, StephenFan.

clang-format


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

https://reviews.llvm.org/D121560

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/DriverOptions.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/claim-unused.c
  lld/COFF/Driver.h
  lld/COFF/DriverUtils.cpp
  lld/ELF/Driver.h
  lld/ELF/DriverUtils.cpp
  lld/MachO/Driver.h
  lld/MachO/DriverUtils.cpp
  lld/MinGW/Driver.cpp
  lld/wasm/Driver.cpp
  lldb/tools/driver/Driver.cpp
  lldb/tools/lldb-server/lldb-gdbserver.cpp
  lldb/tools/lldb-vscode/lldb-vscode.cpp
  llvm/include/llvm/Option/OptParser.td
  llvm/include/llvm/Option/OptTable.h
  llvm/include/llvm/Option/Option.h
  llvm/lib/ToolDrivers/llvm-dlltool/DlltoolDriver.cpp
  llvm/lib/ToolDrivers/llvm-lib/LibDriver.cpp
  llvm/tools/dsymutil/dsymutil.cpp
  llvm/tools/llvm-cvtres/llvm-cvtres.cpp
  llvm/tools/llvm-cxxfilt/llvm-cxxfilt.cpp
  llvm/tools/llvm-lipo/llvm-lipo.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-mt/llvm-mt.cpp
  llvm/tools/llvm-nm/llvm-nm.cpp
  llvm/tools/llvm-objcopy/ObjcopyOptions.cpp
  llvm/tools/llvm-objdump/ObjdumpOptID.h
  llvm/tools/llvm-objdump/llvm-objdump.cpp
  llvm/tools/llvm-rc/llvm-rc.cpp
  llvm/tools/llvm-readobj/llvm-readobj.cpp
  llvm/tools/llvm-size/llvm-size.cpp
  llvm/tools/llvm-strings/llvm-strings.cpp
  llvm/tools/llvm-symbolizer/llvm-symbolizer.cpp
  llvm/tools/llvm-tli-checker/llvm-tli-checker.cpp
  llvm/unittests/Option/OptionMarshallingTest.cpp
  llvm/unittests/Option/OptionParsingTest.cpp
  llvm/utils/TableGen/OptParserEmitter.cpp

Index: llvm/utils/TableGen/OptParserEmitter.cpp
===
--- llvm/utils/TableGen/OptParserEmitter.cpp
+++ llvm/utils/TableGen/OptParserEmitter.cpp
@@ -258,6 +258,16 @@
   OS << "#endif // PREFIX\n\n";
 
   OS << "/\n";
+
+  OS << R"(
+#ifndef OPT_PREFIX
+#define OPT_PREFIX
+#endif
+
+#define CAT_(A, B) A ## B
+#define CAT(A, B) CAT_(A, B)
+#define GET_OPT(OPT) CAT(OPT_PREFIX, OPT)
+  )";
   OS << "// Groups\n\n";
   OS << "#ifdef OPTION\n";
   for (const Record &R : llvm::make_pointee_range(Groups)) {
@@ -298,6 +308,9 @@
 OS << ", nullptr";
 
 // The option Values (unused for groups).
+OS << ", nullptr";
+
+// NoArgUnusedWith (unused for groups).
 OS << ", nullptr)\n";
   }
   OS << "\n";
@@ -388,6 +401,20 @@
   write_cstring(OS, R.getValueAsString("Values"));
 else
   OS << "nullptr";
+
+// List of flags who's presence should cause this flag to not warn if used.
+OS << ", ";
+std::vector List = R.getValueAsListOfDefs("NoArgUnusedWith");
+if (!List.size()) {
+  OS << "nullptr";
+  return;
+}
+OS << "((const unsigned *)&(unsigned []){";
+// First element is the length of the array.
+OS << List.size();
+for (Record *R : List)
+  OS << ", GET_OPT(" << R->getName() << ")";
+OS << "})";
   };
 
   auto IsMarshallingOption = [](const Record &R) {
@@ -405,6 +432,11 @@
   OptsWithMarshalling.push_back(&R);
   }
   OS << "#endif // OPTION\n";
+  OS << R"(#undef GET_OPT
+#undef CAT
+#undef CAT_
+#undef OPT_PREFIX
+)";
 
   auto CmpMarshallingOpts = [](const Record *const *A, const Record *const *B) {
 unsigned AID = (*A)->getID();
Index: llvm/unittests/Option/OptionParsingTest.cpp
===
--- llvm/unittests/Option/OptionParsingTest.cpp
+++ llvm/unittests/Option/OptionParsingTest.cpp
@@ -18,7 +18,7 @@
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
+   HELPTEXT, METAVAR, VALUES, UNUSEDWITH)  \
   OPT_##ID,
 #include "Opts.inc"
   LastOption
@@ -37,9 +37,10 @@
 
 static const OptTable::Info InfoTable[] = {
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
-   HELPTEXT, METAVAR, VALUES)  \
-  {PREFIX, NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class,\
-   PARAM,  FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES},
+   HELPTEXT, METAVAR, VALUES, UNUSEDWITH)  \
+  {PREFIX,NAME,  HELPTEXT,METAVAR, OPT_##ID,  Option::KIND##Class, \
+   PARAM, FLAGS, OPT_##GROUP, OPT_##ALIAS, ALIASARGS, VALUES,  \
+   UNUSEDWITH},
 #include "Opts.inc"
 #undef OPTION
 };
Index: llvm/unittests/Option/OptionMarshallingTest.cpp
===
--- llvm/unittests/Option/OptionMarshallingT