[PATCH] D134749: [clang][Interp] Implement Div opcode

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

tbaeder updated this revision to Diff 465667.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D134749

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -174,3 +174,25 @@
// expected-note {{value 2147483648 is outside the range}} \
 
 };
+
+namespace div {
+  constexpr int zero() { return 0; }
+  static_assert(12 / 3 == 4, "");
+  static_assert(12 / zero() == 12, ""); // ref-error {{not an integral constant expression}} \
+// ref-note {{division by zero}} \
+// expected-error {{not an integral constant expression}} \
+// expected-note {{division by zero}}
+  static_assert(12 / -3 == -4, "");
+  static_assert(-12 / 3 == -4, "");
+
+
+  constexpr int LHS = 12;
+  constexpr long unsigned RHS = 3;
+  static_assert(LHS / RHS == 4, "");
+
+  constexpr int x = INT_MIN / - 1; // ref-error {{must be initialized by a constant expression}} \
+   // ref-note {{value 2147483648 is outside the range}} \
+   // expected-error {{must be initialized by a constant expression}} \
+   // expected-note {{value 2147483648 is outside the range}} \
+
+};
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -401,7 +401,10 @@
   let Types = [NumberTypeClass];
   let HasGroup = 1;
 }
-
+def Div : Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
 
 //===--===//
 // Unary operators.
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -186,6 +186,39 @@
   return false;
 }
 
+/// 1) Pops the RHS from the stack.
+/// 2) Pops the LHS from the stack.
+/// 3) Pushes 'LHS / RHS' on the stack
+template ::T>
+bool Div(InterpState &S, CodePtr OpPC) {
+  const T &RHS = S.Stk.pop();
+  const T &LHS = S.Stk.pop();
+
+  if (RHS.isZero()) {
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+S.FFDiag(Loc, diag::note_expr_divide_by_zero);
+return false;
+  }
+
+  if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
+APSInt LHSInt = LHS.toAPSInt();
+SmallString<32> Trunc;
+(-LHSInt.extend(LHSInt.getBitWidth() + 1)).toString(Trunc, 10);
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+const Expr *E = S.Current->getExpr(OpPC);
+S.CCEDiag(Loc, diag::note_constexpr_overflow) << Trunc << E->getType();
+return false;
+  }
+
+  const unsigned Bits = RHS.bitWidth() * 2;
+  T Result;
+  if (!T::div(LHS, RHS, Bits, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+  return false;
+}
+
 //===--===//
 // Inv
 //===--===//
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -215,6 +215,11 @@
 return false;
   }
 
+  static bool div(Integral A, Integral B, unsigned OpBits, Integral *R) {
+*R = Integral(A.V / B.V);
+return false;
+  }
+
   static bool neg(Integral A, Integral *R) {
 *R = -A;
 return false;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -213,6 +213,8 @@
   return Discard(this->emitMul(*T, BO));
 case BO_Rem:
   return Discard(this->emitRem(*T, BO));
+case BO_Div:
+  return Discard(this->emitDiv(*T, BO));
 case BO_Assign:
   if (!this->emitStore(*T, BO))
 return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135128: [clang][cli] Simplify repetitive macro invocations

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

This looks fine to me in principle. But I wonder if we should land the flag 
change first separately and make sure that no buildbots break because of it. 
Then we can merge the simplification a few days later when we are sure it's 
stabilized, since something similar happened when we upgraded to C++17 - and 
people already had merged code that used C++17 it was really hard to revert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135128

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

[PATCH] D135259: [clang] Remove CLANG_ENABLE_OPAQUE_POINTERS cmake option

[Nikita Popov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd785a8eaa25d: [clang] Remove CLANG_ENABLE_OPAQUE_POINTERS 
cmake option (authored by nikic).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135259

Files:
  clang/CMakeLists.txt
  clang/include/clang/Config/config.h.cmake
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/CMakeLists.txt
  clang/test/Driver/lto-no-opaque-pointers.c
  clang/test/Driver/lto-opaque-pointers.c
  clang/test/Driver/opaque-pointers-off.c
  clang/test/lit.cfg.py
  clang/test/lit.site.cfg.py.in

Index: clang/test/lit.site.cfg.py.in
===
--- clang/test/lit.site.cfg.py.in
+++ clang/test/lit.site.cfg.py.in
@@ -24,7 +24,6 @@
 config.have_zstd = @LLVM_ENABLE_ZSTD@
 config.clang_arcmt = @CLANG_ENABLE_ARCMT@
 config.clang_default_pie_on_linux = @CLANG_DEFAULT_PIE_ON_LINUX@
-config.clang_enable_opaque_pointers = @CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL@
 config.clang_default_std_cxx = "@CLANG_DEFAULT_STD_CXX@"
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
 config.clang_staticanalyzer = @CLANG_ENABLE_STATIC_ANALYZER@
Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -126,9 +126,6 @@
 if config.clang_default_pie_on_linux:
 config.available_features.add('default-pie-on-linux')
 
-if config.clang_enable_opaque_pointers:
-config.available_features.add('enable-opaque-pointers')
-
 if config.clang_default_std_cxx != '':
 config.available_features.add('default-std-cxx')
 
Index: clang/test/Driver/opaque-pointers-off.c
===
--- clang/test/Driver/opaque-pointers-off.c
+++ /dev/null
@@ -1,11 +0,0 @@
-// UNSUPPORTED: enable-opaque-pointers
-/// Test -DCLANG_ENABLE_OPAQUE_POINTERS=OFF.
-
-// RUN: %clang -### --target=aarch64-linux-gnu %s 2>&1 | FileCheck %s
-/// User -Xclang -opaque-pointers overrides the default.
-// RUN: %clang -### --target=aarch64-linux-gnu -Xclang -opaque-pointers %s 2>&1 | FileCheck %s --check-prefix=CHECK2
-
-// CHECK:   "-no-opaque-pointers"
-
-// CHECK2:  "-no-opaque-pointers"
-// CHECK2-SAME: "-opaque-pointers"
Index: clang/test/Driver/lto-opaque-pointers.c
===
--- clang/test/Driver/lto-opaque-pointers.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// REQUIRES: enable-opaque-pointers
-// RUN: %clang --target=x86_64-unknown-linux -### %s -flto 2> %t
-// RUN: FileCheck %s < %t
-
-// CHECK-NOT: -plugin-opt=no-opaque-pointers
Index: clang/test/Driver/lto-no-opaque-pointers.c
===
--- clang/test/Driver/lto-no-opaque-pointers.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// UNSUPPORTED: enable-opaque-pointers
-// RUN: %clang --target=x86_64-unknown-linux -### %s -flto 2> %t
-// RUN: FileCheck %s < %t
-
-// CHECK: -plugin-opt=no-opaque-pointers
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -5,7 +5,6 @@
   CLANG_BUILD_EXAMPLES
   CLANG_BUILT_STANDALONE
   CLANG_DEFAULT_PIE_ON_LINUX
-  CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL
   CLANG_ENABLE_ARCMT
   CLANG_ENABLE_STATIC_ANALYZER
   CLANG_PLUGIN_SUPPORT
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -558,9 +558,6 @@
 CmdArgs.push_back(
 Args.MakeArgString("-plugin-opt=jobs=" + Twine(Parallelism)));
 
-  if (!CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL)
-CmdArgs.push_back(Args.MakeArgString("-plugin-opt=no-opaque-pointers"));
-
   // If an explicit debugger tuning argument appeared, pass it along.
   if (Arg *A = Args.getLastArg(options::OPT_gTune_Group,
options::OPT_ggdbN_Group)) {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6697,9 +6697,6 @@
false))
 CmdArgs.push_back("-fmodules-debuginfo");
 
-  if (!CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL)
-CmdArgs.push_back("-no-opaque-pointers");
-
   ObjCRuntime Runtime = AddObjCRuntimeArgs(Args, Inputs, CmdArgs, rewriteKind);
   RenderObjCOptions(TC, D, RawTriple, Args, Runtime, rewriteKind != RK_None,
 Input, CmdArgs);
Index: clang/include/clang/Config/config.h.cmake
===
--- clang/include/clang/Config/config.h.cmake

[clang] d785a8e - [clang] Remove CLANG_ENABLE_OPAQUE_POINTERS cmake option

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-10-06T09:46:04+02:00
New Revision: d785a8eaa25dd1110dc7b24b16d3b21c9c179837

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

LOG: [clang] Remove CLANG_ENABLE_OPAQUE_POINTERS cmake option

Remove the ability to disable opaque pointers by default in clang.
It is still possible to explicitly disable them via cc1
-no-opaque-pointers.

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

Added: 


Modified: 
clang/CMakeLists.txt
clang/include/clang/Config/config.h.cmake
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Driver/ToolChains/CommonArgs.cpp
clang/test/CMakeLists.txt
clang/test/lit.cfg.py
clang/test/lit.site.cfg.py.in

Removed: 
clang/test/Driver/lto-no-opaque-pointers.c
clang/test/Driver/lto-opaque-pointers.c
clang/test/Driver/opaque-pointers-off.c



diff  --git a/clang/CMakeLists.txt b/clang/CMakeLists.txt
index f43fa51a3379c..875bd27e13206 100644
--- a/clang/CMakeLists.txt
+++ b/clang/CMakeLists.txt
@@ -189,17 +189,6 @@ set(CLANG_SPAWN_CC1 OFF CACHE BOOL
 
 option(CLANG_DEFAULT_PIE_ON_LINUX "Default to -fPIE and -pie on linux-gnu" ON)
 
-# Manually handle default so we can change the meaning of a cached default.
-set(CLANG_ENABLE_OPAQUE_POINTERS "DEFAULT" CACHE STRING
-"Enable opaque pointers by default")
-if(CLANG_ENABLE_OPAQUE_POINTERS STREQUAL "DEFAULT")
-  set(CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL ON)
-elseif(CLANG_ENABLE_OPAQUE_POINTERS)
-  set(CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL ON)
-else()
-  set(CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL OFF)
-endif()
-
 # TODO: verify the values against LangStandards.def?
 set(CLANG_DEFAULT_STD_C "" CACHE STRING
   "Default standard to use for C/ObjC code (IDENT from LangStandards.def, 
empty for platform default)")

diff  --git a/clang/include/clang/Config/config.h.cmake 
b/clang/include/clang/Config/config.h.cmake
index a4083827e43f8..3ffc2a12a89fe 100644
--- a/clang/include/clang/Config/config.h.cmake
+++ b/clang/include/clang/Config/config.h.cmake
@@ -101,7 +101,4 @@
 /* Spawn a new process clang.exe for the CC1 tool invocation, when necessary */
 #cmakedefine01 CLANG_SPAWN_CC1
 
-/* Whether to enable opaque pointers by default */
-#cmakedefine01 CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL
-
 #endif

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index e5fce35793598..4f880c27b2f11 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6697,9 +6697,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
false))
 CmdArgs.push_back("-fmodules-debuginfo");
 
-  if (!CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL)
-CmdArgs.push_back("-no-opaque-pointers");
-
   ObjCRuntime Runtime = AddObjCRuntimeArgs(Args, Inputs, CmdArgs, rewriteKind);
   RenderObjCOptions(TC, D, RawTriple, Args, Runtime, rewriteKind != RK_None,
 Input, CmdArgs);

diff  --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp 
b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index d81faa3652285..7f20122722365 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -558,9 +558,6 @@ void tools::addLTOOptions(const ToolChain &ToolChain, const 
ArgList &Args,
 CmdArgs.push_back(
 Args.MakeArgString("-plugin-opt=jobs=" + Twine(Parallelism)));
 
-  if (!CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL)
-CmdArgs.push_back(Args.MakeArgString("-plugin-opt=no-opaque-pointers"));
-
   // If an explicit debugger tuning argument appeared, pass it along.
   if (Arg *A = Args.getLastArg(options::OPT_gTune_Group,
options::OPT_ggdbN_Group)) {

diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index c3dcc53b78ab5..09394a0f7f730 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -5,7 +5,6 @@ llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
   CLANG_BUILT_STANDALONE
   CLANG_DEFAULT_PIE_ON_LINUX
-  CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL
   CLANG_ENABLE_ARCMT
   CLANG_ENABLE_STATIC_ANALYZER
   CLANG_PLUGIN_SUPPORT

diff  --git a/clang/test/Driver/lto-no-opaque-pointers.c 
b/clang/test/Driver/lto-no-opaque-pointers.c
deleted file mode 100644
index 9146ae5da5824..0
--- a/clang/test/Driver/lto-no-opaque-pointers.c
+++ /dev/null
@@ -1,5 +0,0 @@
-// UNSUPPORTED: enable-opaque-pointers
-// RUN: %clang --target=x86_64-unknown-linux -### %s -flto 2> %t
-// RUN: FileCheck %s < %t
-
-// CHECK: -plugin-opt=no-opaque-pointers

diff  --git a/clang/test/Driver/lto-opaque-pointers.c 
b/clang/test/Driver/lto-opaque-pointers.c
deleted file mode 100644
index acdf13c21fd41..0
--- a/clang/test/Driver/lto-opaque-pointers.c
+++ /dev/null
@@ -1,5 +0,0 @@

[PATCH] D135257: [clangd][Tweak] Make sure enclosing function doesnt have invalid children

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D135257#3836511 , @ilya-biryukov 
wrote:

> Having `nullptr` inside `children()` seems really weird. Should we fix those 
> instead to never produce `nullptr`? Or is this something that is expected (I 
> can come up with a few contracts where this would make sense, but that all 
> looks really akward).

that would definitely be great, if I could find any examples that triggers 
this. my main hesitation with such solutions is, no matter how hard we try, due 
to the variety of codepaths that lead to stmt creation, either we'll miss some 
paths or someone in the future will introduce a new one without broken code in 
mind and hell will break lose again. so i personally feel like being defensive 
on the application side, especially when it isn't a huge performance hit, is 
healthier approach in the long term.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135257

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

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/CMakeLists.txt:163
   clangToolingInclusions
+  clangToolingInclusionsSTL
   clangToolingSyntax

sammccall wrote:
> StandardLibrary or Stdlib?
> 
> STL isn't accurate or consistent with the names in the code.
changing to Stdlib



Comment at: clang/lib/Tooling/Inclusions/STL/CMakeLists.txt:2
+add_clang_library(clangToolingInclusionsSTL
+  StandardLibrary.cpp
+

sammccall wrote:
> This means the implementation files and the header files have a different 
> directory structure, which may be confusing to people trying to work out 
> which library to link against based on the headers they included.
> 
> On the other hand, I think the cascading effect of dependencies => libraries 
> => directory structure => header structure is pretty unfortunate leaking of 
> llvm's sad cmake structure. Up to you
> 
> 
right, i was also torn between moving the headers around vs not. but i finally 
managed to convince myself that the implementation being in a different 
subdirectory is actually an unfortunate detail about the way LLVM is build (I 
didn't want to have PARTIAL_SOURCES_INTENDED, either) and shouldn't matter for 
the applications that want to use it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135245

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

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 465671.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Rename STL to Stdlib


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135245

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang/lib/Tooling/Inclusions/CMakeLists.txt
  clang/lib/Tooling/Inclusions/StandardLibrary.cpp
  clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
  clang/unittests/Tooling/CMakeLists.txt

Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -81,6 +81,7 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingInclusionsSTL
   clangToolingRefactoring
   clangTransformer
   )
Index: clang/lib/Tooling/Inclusions/StandardLibrary.cpp
===
--- /dev/null
+++ clang/lib/Tooling/Inclusions/StandardLibrary.cpp
@@ -1,165 +0,0 @@
-//===--- StandardLibrary.cpp *- 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/Tooling/Inclusions/StandardLibrary.h"
-#include "llvm/ADT/Optional.h"
-#include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Casting.h"
-
-namespace clang {
-namespace tooling {
-namespace stdlib {
-
-static llvm::StringRef *HeaderNames;
-static std::pair *SymbolNames;
-static unsigned *SymbolHeaderIDs;
-static llvm::DenseMap *HeaderIDs;
-// Maps symbol name -> Symbol::ID, within a namespace.
-using NSSymbolMap = llvm::DenseMap;
-static llvm::DenseMap *NamespaceSymbols;
-
-static int initialize() {
-  unsigned SymCount = 0;
-#define SYMBOL(Name, NS, Header) ++SymCount;
-#include "clang/Tooling/Inclusions/CSymbolMap.inc"
-#include "clang/Tooling/Inclusions/StdSymbolMap.inc"
-#undef SYMBOL
-  SymbolNames = new std::remove_reference_t[SymCount];
-  SymbolHeaderIDs =
-  new std::remove_reference_t[SymCount];
-  NamespaceSymbols = new std::remove_reference_t;
-  HeaderIDs = new std::remove_reference_t;
-
-  auto AddNS = [&](llvm::StringRef NS) -> NSSymbolMap & {
-auto R = NamespaceSymbols->try_emplace(NS, nullptr);
-if (R.second)
-  R.first->second = new NSSymbolMap();
-return *R.first->second;
-  };
-
-  auto AddHeader = [&](llvm::StringRef Header) -> unsigned {
-return HeaderIDs->try_emplace(Header, HeaderIDs->size()).first->second;
-  };
-
-  auto Add = [&, SymIndex(0)](llvm::StringRef Name, llvm::StringRef NS,
-  llvm::StringRef HeaderName) mutable {
-if (NS == "None")
-  NS = "";
-
-SymbolNames[SymIndex] = {NS, Name};
-SymbolHeaderIDs[SymIndex] = AddHeader(HeaderName);
-
-NSSymbolMap &NSSymbols = AddNS(NS);
-NSSymbols.try_emplace(Name, SymIndex);
-
-++SymIndex;
-  };
-#define SYMBOL(Name, NS, Header) Add(#Name, #NS, #Header);
-#include "clang/Tooling/Inclusions/CSymbolMap.inc"
-#include "clang/Tooling/Inclusions/StdSymbolMap.inc"
-#undef SYMBOL
-
-  HeaderNames = new llvm::StringRef[HeaderIDs->size()];
-  for (const auto &E : *HeaderIDs)
-HeaderNames[E.second] = E.first;
-
-  return 0;
-}
-
-static void ensureInitialized() {
-  static int Dummy = initialize();
-  (void)Dummy;
-}
-
-llvm::Optional Header::named(llvm::StringRef Name) {
-  ensureInitialized();
-  auto It = HeaderIDs->find(Name);
-  if (It == HeaderIDs->end())
-return llvm::None;
-  return Header(It->second);
-}
-llvm::StringRef Header::name() const { return HeaderNames[ID]; }
-llvm::StringRef Symbol::scope() const { return SymbolNames[ID].first; }
-llvm::StringRef Symbol::name() const { return SymbolNames[ID].second; }
-llvm::Optional Symbol::named(llvm::StringRef Scope,
- llvm::StringRef Name) {
-  ensureInitialized();
-  if (NSSymbolMap *NSSymbols = NamespaceSymbols->lookup(Scope)) {
-auto It = NSSymbols->find(Name);
-if (It != NSSymbols->end())
-  return Symbol(It->second);
-  }
-  return llvm::None;
-}
-Header Symbol::header() const { return Header(SymbolHeaderIDs[ID]); }
-llvm::SmallVector Symbol::headers() const {
-  return {header()}; // FIXME: multiple in case of ambiguity
-}
-
-Recognizer::Recognizer() { ensureInitialized(); }
-
-NSSymbolMap *Recognizer::namespaceSymbols(const NamespaceDecl *D) {
-  auto It = NamespaceCache.find(D);
-  if (It != NamespaceCache.end())
-return It->second;
-
-  NSSymbolMap *Result = [&]() -> NSSymbolMap * {
-if (D && D->isAnonymousNamespace())
-  return nullptr;
-// Print the namespace and its parents ommitting inline scopes.
-std::

[PATCH] D129446: [clang][driver] Find Apple default SDK path

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

(With configuration file improvement, you can express the intention with a 
default configuration file.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129446

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

[clang] d1f13c5 - [clang][Tooling] Move STL recognizer to its own library

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2022-10-06T10:09:13+02:00
New Revision: d1f13c54f172875d9a14c46c09afb1f22d78cdf8

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

LOG: [clang][Tooling] Move STL recognizer to its own library

As pointed out in https://reviews.llvm.org/D119130#3829816, this
introduces a clang AST dependency to the clangToolingInclusions, which is used
by clang-format.

Since rest of the inclusion tooling doesn't depend on clang ast, moving this
into a separate library.

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

Added: 
clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp

Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang/lib/Tooling/Inclusions/CMakeLists.txt
clang/unittests/Tooling/CMakeLists.txt

Removed: 
clang/lib/Tooling/Inclusions/StandardLibrary.cpp



diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index de8f087a52a5e..56ff54f8fadde 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -160,6 +160,7 @@ clang_target_link_libraries(clangDaemon
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingInclusionsStdlib
   clangToolingSyntax
   )
 

diff  --git a/clang/lib/Tooling/Inclusions/CMakeLists.txt 
b/clang/lib/Tooling/Inclusions/CMakeLists.txt
index fba003b4e0de5..1954d16a1b231 100644
--- a/clang/lib/Tooling/Inclusions/CMakeLists.txt
+++ b/clang/lib/Tooling/Inclusions/CMakeLists.txt
@@ -3,12 +3,12 @@ set(LLVM_LINK_COMPONENTS support)
 add_clang_library(clangToolingInclusions
   HeaderIncludes.cpp
   IncludeStyle.cpp
-  StandardLibrary.cpp
 
   LINK_LIBS
-  clangAST
   clangBasic
   clangLex
   clangRewrite
   clangToolingCore
   )
+
+  add_subdirectory(Stdlib)

diff  --git a/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt 
b/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
new file mode 100644
index 0..0f52c3590ac9a
--- /dev/null
+++ b/clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
@@ -0,0 +1,6 @@
+add_clang_library(clangToolingInclusionsStdlib
+  StandardLibrary.cpp
+
+  LINK_LIBS
+  clangAST
+  )

diff  --git a/clang/lib/Tooling/Inclusions/StandardLibrary.cpp 
b/clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
similarity index 100%
rename from clang/lib/Tooling/Inclusions/StandardLibrary.cpp
rename to clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp

diff  --git a/clang/unittests/Tooling/CMakeLists.txt 
b/clang/unittests/Tooling/CMakeLists.txt
index dfc1f59cf0a83..424932e529519 100644
--- a/clang/unittests/Tooling/CMakeLists.txt
+++ b/clang/unittests/Tooling/CMakeLists.txt
@@ -81,6 +81,7 @@ clang_target_link_libraries(ToolingTests
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingInclusionsStdlib
   clangToolingRefactoring
   clangTransformer
   )



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

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

[Kadir Cetinkaya 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 rGd1f13c54f172: [clang][Tooling] Move STL recognizer to its 
own library (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D135245?vs=465671&id=465676#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135245

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang/lib/Tooling/Inclusions/CMakeLists.txt
  clang/lib/Tooling/Inclusions/StandardLibrary.cpp
  clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
  clang/lib/Tooling/Inclusions/Stdlib/StandardLibrary.cpp
  clang/unittests/Tooling/CMakeLists.txt


Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -81,6 +81,7 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingInclusionsStdlib
   clangToolingRefactoring
   clangTransformer
   )
Index: clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
===
--- /dev/null
+++ clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
@@ -0,0 +1,6 @@
+add_clang_library(clangToolingInclusionsStdlib
+  StandardLibrary.cpp
+
+  LINK_LIBS
+  clangAST
+  )
Index: clang/lib/Tooling/Inclusions/CMakeLists.txt
===
--- clang/lib/Tooling/Inclusions/CMakeLists.txt
+++ clang/lib/Tooling/Inclusions/CMakeLists.txt
@@ -3,12 +3,12 @@
 add_clang_library(clangToolingInclusions
   HeaderIncludes.cpp
   IncludeStyle.cpp
-  StandardLibrary.cpp
 
   LINK_LIBS
-  clangAST
   clangBasic
   clangLex
   clangRewrite
   clangToolingCore
   )
+
+  add_subdirectory(Stdlib)
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -160,6 +160,7 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingInclusionsStdlib
   clangToolingSyntax
   )
 


Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -81,6 +81,7 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingInclusionsStdlib
   clangToolingRefactoring
   clangTransformer
   )
Index: clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
===
--- /dev/null
+++ clang/lib/Tooling/Inclusions/Stdlib/CMakeLists.txt
@@ -0,0 +1,6 @@
+add_clang_library(clangToolingInclusionsStdlib
+  StandardLibrary.cpp
+
+  LINK_LIBS
+  clangAST
+  )
Index: clang/lib/Tooling/Inclusions/CMakeLists.txt
===
--- clang/lib/Tooling/Inclusions/CMakeLists.txt
+++ clang/lib/Tooling/Inclusions/CMakeLists.txt
@@ -3,12 +3,12 @@
 add_clang_library(clangToolingInclusions
   HeaderIncludes.cpp
   IncludeStyle.cpp
-  StandardLibrary.cpp
 
   LINK_LIBS
-  clangAST
   clangBasic
   clangLex
   clangRewrite
   clangToolingCore
   )
+
+  add_subdirectory(Stdlib)
Index: clang-tools-extra/clangd/CMakeLists.txt
===
--- clang-tools-extra/clangd/CMakeLists.txt
+++ clang-tools-extra/clangd/CMakeLists.txt
@@ -160,6 +160,7 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingInclusionsStdlib
   clangToolingSyntax
   )
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] df61bb2 - [SourceManager] Improve getFileIDLoaded.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-10-06T10:15:09+02:00
New Revision: df61bb271af9ad6e61c1cd470ea6f4255b2182c7

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

LOG: [SourceManager] Improve getFileIDLoaded.

Similar to getFileIDLocal patch, but for the version for load module.

Test with clangd (building AST with preamble), FileID scans in binary
search is reduced:

SemaExpr.cpp: 142K -> 137K (-3%)
FindTarget.cpp: 368K -> 343K (-6%)

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

Added: 


Modified: 
clang/lib/Basic/SourceManager.cpp

Removed: 




diff  --git a/clang/lib/Basic/SourceManager.cpp 
b/clang/lib/Basic/SourceManager.cpp
index de217a95d3921..32234b5cc73ee 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -869,36 +869,38 @@ FileID 
SourceManager::getFileIDLoaded(SourceLocation::UIntTy SLocOffset) const {
   }
 
   // Essentially the same as the local case, but the loaded array is sorted
-  // in the other direction.
+  // in the other direction (decreasing order).
+  // GreaterIndex is the one where the offset is greater, which is actually a
+  // lower index!
+  unsigned GreaterIndex = 0;
+  unsigned LessIndex = LoadedSLocEntryTable.size();
+  if (LastFileIDLookup.ID < 0) {
+// Prune the search space.
+int LastID = LastFileIDLookup.ID;
+if (getLoadedSLocEntryByID(LastID).getOffset() > SLocOffset)
+  GreaterIndex =
+  (-LastID - 2) + 1; // Exclude LastID, else we would have hit the 
cache
+else
+  LessIndex = -LastID - 2;
+  }
 
   // First do a linear scan from the last lookup position, if possible.
-  unsigned I;
-  int LastID = LastFileIDLookup.ID;
-  if (LastID >= 0 || getLoadedSLocEntryByID(LastID).getOffset() < SLocOffset)
-I = 0;
-  else
-I = (-LastID - 2) + 1;
-
   unsigned NumProbes;
   bool Invalid = false;
-  for (NumProbes = 0; NumProbes < 8; ++NumProbes, ++I) {
+  for (NumProbes = 0; NumProbes < 8; ++NumProbes, ++GreaterIndex) {
 // Make sure the entry is loaded!
-const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I, &Invalid);
+const SrcMgr::SLocEntry &E = getLoadedSLocEntry(GreaterIndex, &Invalid);
 if (Invalid)
   return FileID(); // invalid entry.
 if (E.getOffset() <= SLocOffset) {
-  FileID Res = FileID::get(-int(I) - 2);
+  FileID Res = FileID::get(-int(GreaterIndex) - 2);
   LastFileIDLookup = Res;
   NumLinearScans += NumProbes + 1;
   return Res;
 }
   }
 
-  // Linear scan failed. Do the binary search. Note the reverse sorting of the
-  // table: GreaterIndex is the one where the offset is greater, which is
-  // actually a lower index!
-  unsigned GreaterIndex = I;
-  unsigned LessIndex = LoadedSLocEntryTable.size();
+  // Linear scan failed. Do the binary search.
   NumProbes = 0;
   while (true) {
 ++NumProbes;



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

[PATCH] D134813: Properly print unnamed TagDecl objects in diagnostics

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang added a comment.

In D134813#3838069 , @zixuw wrote:

> With the PrintingPolicy fix in https://reviews.llvm.org/D135295 and landed 
> USR fix, the diff within ExtractAPI tests is only the wording with anonymous 
> enums, and we can drop the lit change:
>
>   658c658
>   < "spelling": "(anonymous)"
>   ---
>   > "spelling": "enum (unnamed)"
>   661c661
>   < "title": "(anonymous)"
>   ---
>   > "title": "enum (unnamed)"
>   664c664
>   < "(anonymous)"
>   ---
>   > "enum (unnamed)"
>   706c706
>   < "(anonymous)",
>   ---
>   > "enum (unnamed)",
>   746c746
>   < "spelling": "(anonymous)"
>   ---
>   > "spelling": "enum (unnamed)"
>   749c749
>   < "title": "(anonymous)"
>   ---
>   > "title": "enum (unnamed)"
>   752c752
>   < "(anonymous)"
>   ---
>   > "enum (unnamed)"
>   794c794
>   < "(anonymous)",
>   ---
>   > "enum (unnamed)",
>
> @dang @QuietMisdreavus for this change.

As far as I know this should affect downstream consumers like Swift-DocC. I am 
happy with these changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134813

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

[PATCH] D135258: [SourceManager] Improve getFileIDLoaded.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hokein marked 2 inline comments as done.
Closed by commit rGdf61bb271af9: [SourceManager] Improve getFileIDLoaded. 
(authored by hokein).

Changed prior to commit:
  https://reviews.llvm.org/D135258?vs=465345&id=465678#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135258

Files:
  clang/lib/Basic/SourceManager.cpp


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -869,36 +869,38 @@
   }
 
   // Essentially the same as the local case, but the loaded array is sorted
-  // in the other direction.
+  // in the other direction (decreasing order).
+  // GreaterIndex is the one where the offset is greater, which is actually a
+  // lower index!
+  unsigned GreaterIndex = 0;
+  unsigned LessIndex = LoadedSLocEntryTable.size();
+  if (LastFileIDLookup.ID < 0) {
+// Prune the search space.
+int LastID = LastFileIDLookup.ID;
+if (getLoadedSLocEntryByID(LastID).getOffset() > SLocOffset)
+  GreaterIndex =
+  (-LastID - 2) + 1; // Exclude LastID, else we would have hit the 
cache
+else
+  LessIndex = -LastID - 2;
+  }
 
   // First do a linear scan from the last lookup position, if possible.
-  unsigned I;
-  int LastID = LastFileIDLookup.ID;
-  if (LastID >= 0 || getLoadedSLocEntryByID(LastID).getOffset() < SLocOffset)
-I = 0;
-  else
-I = (-LastID - 2) + 1;
-
   unsigned NumProbes;
   bool Invalid = false;
-  for (NumProbes = 0; NumProbes < 8; ++NumProbes, ++I) {
+  for (NumProbes = 0; NumProbes < 8; ++NumProbes, ++GreaterIndex) {
 // Make sure the entry is loaded!
-const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I, &Invalid);
+const SrcMgr::SLocEntry &E = getLoadedSLocEntry(GreaterIndex, &Invalid);
 if (Invalid)
   return FileID(); // invalid entry.
 if (E.getOffset() <= SLocOffset) {
-  FileID Res = FileID::get(-int(I) - 2);
+  FileID Res = FileID::get(-int(GreaterIndex) - 2);
   LastFileIDLookup = Res;
   NumLinearScans += NumProbes + 1;
   return Res;
 }
   }
 
-  // Linear scan failed. Do the binary search. Note the reverse sorting of the
-  // table: GreaterIndex is the one where the offset is greater, which is
-  // actually a lower index!
-  unsigned GreaterIndex = I;
-  unsigned LessIndex = LoadedSLocEntryTable.size();
+  // Linear scan failed. Do the binary search.
   NumProbes = 0;
   while (true) {
 ++NumProbes;


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -869,36 +869,38 @@
   }
 
   // Essentially the same as the local case, but the loaded array is sorted
-  // in the other direction.
+  // in the other direction (decreasing order).
+  // GreaterIndex is the one where the offset is greater, which is actually a
+  // lower index!
+  unsigned GreaterIndex = 0;
+  unsigned LessIndex = LoadedSLocEntryTable.size();
+  if (LastFileIDLookup.ID < 0) {
+// Prune the search space.
+int LastID = LastFileIDLookup.ID;
+if (getLoadedSLocEntryByID(LastID).getOffset() > SLocOffset)
+  GreaterIndex =
+  (-LastID - 2) + 1; // Exclude LastID, else we would have hit the cache
+else
+  LessIndex = -LastID - 2;
+  }
 
   // First do a linear scan from the last lookup position, if possible.
-  unsigned I;
-  int LastID = LastFileIDLookup.ID;
-  if (LastID >= 0 || getLoadedSLocEntryByID(LastID).getOffset() < SLocOffset)
-I = 0;
-  else
-I = (-LastID - 2) + 1;
-
   unsigned NumProbes;
   bool Invalid = false;
-  for (NumProbes = 0; NumProbes < 8; ++NumProbes, ++I) {
+  for (NumProbes = 0; NumProbes < 8; ++NumProbes, ++GreaterIndex) {
 // Make sure the entry is loaded!
-const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I, &Invalid);
+const SrcMgr::SLocEntry &E = getLoadedSLocEntry(GreaterIndex, &Invalid);
 if (Invalid)
   return FileID(); // invalid entry.
 if (E.getOffset() <= SLocOffset) {
-  FileID Res = FileID::get(-int(I) - 2);
+  FileID Res = FileID::get(-int(GreaterIndex) - 2);
   LastFileIDLookup = Res;
   NumLinearScans += NumProbes + 1;
   return Res;
 }
   }
 
-  // Linear scan failed. Do the binary search. Note the reverse sorting of the
-  // table: GreaterIndex is the one where the offset is greater, which is
-  // actually a lower index!
-  unsigned GreaterIndex = I;
-  unsigned LessIndex = LoadedSLocEntryTable.size();
+  // Linear scan failed. Do the binary search.
   NumProbes = 0;
   while (true) {
 ++NumProbes;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134745: [LV][Metadata] Add loop.interleave.enable for loop vectorizer

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

eopXD added a comment.

Gentle ping, thank you ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134745

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

[PATCH] D135118: [clang/Sema] Fix non-deterministic order for certain kind of diagnostics

[Nikita Popov via Phabricator via cfe-commits]

nikic added a comment.

FYI this caused a noticeable compile-time regression (about 0.4% geomean at 
`-O0`): 
http://llvm-compile-time-tracker.com/compare.php?from=92233159035d1b50face95d886901cf99035bd99&to=371883f46dc23f8464cbf578e2d12a4f92e61917&stat=instructions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135118

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

[PATCH] D134745: [LV][Metadata] Add loop.interleave.enable for loop vectorizer

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

> Adding this metadata allows {loop.vectorize.enable, false} to be used without 
> disabling the whole pass.

Could you please describe the behavior in more detail here? The new metadata 
should also be documented in `LangRef`, the new pragma in 
https://clang.llvm.org/docs/LanguageExtensions.htm.

Will this change the existing behavior if only `llvm.loop.vectorize.enable == 
false`? To preserve backwards compatibility, I think the behavior shouldn;t 
change unless `llvm.loop.interleave.enable` is provided.

Also, shouldn't `#pragma clang loop vectorize(disable) interleave(enable)` be 
equivalent to `#pragma clang loop vectorize(enable) vectorize_width(1)`?




Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2142
 
-  if (Hints.getInterleave() > 1) {
+  if (Hints.getInterleaveForce()) {
 // TODO: Interleave support is future work.

Will `getInterleaveForce()` return `ENABLED` if an interleave count > 1 is set 
through metadata?



Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3013
   assert(!(SCEVCheckBlock->getParent()->hasOptSize() ||
-   (OptForSizeBasedOnProfile &&
-Cost->Hints->getForce() != LoopVectorizeHints::FK_Enabled)) &&
+   (OptForSizeBasedOnProfile && Cost->Hints->getVectorizationForce() !=
+LoopVectorizeHints::FK_Enabled)) &&

Should this check interleaving or vectorization forced?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134745

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

[PATCH] D134745: [LV][Metadata] Add loop.interleave.enable for loop vectorizer

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

eopXD added a comment.

@fhahn Thank you very much for the review, backward compatibility is a good 
point. I will update the revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134745

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

[PATCH] D135314: [clangd] Avoid scanning up to end of file on each comment!

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

wow, that's an interesting finding. thanks!




Comment at: clang-tools-extra/clangd/Headers.cpp:25
 
-const char IWYUPragmaKeep[] = "// IWYU pragma: keep";
-const char IWYUPragmaExport[] = "// IWYU pragma: export";
-const char IWYUPragmaBeginExports[] = "// IWYU pragma: begin_exports";
+llvm::Optional parseIWYUPragma(const char *Text) {
+  // This gets called for every comment seen in the preamble, so it's quite 
hot.

i think this also deserves a comment to make sure people won't refactor it in 
the future to take in a stringref.



Comment at: clang-tools-extra/clangd/Headers.cpp:25
 
-const char IWYUPragmaKeep[] = "// IWYU pragma: keep";
-const char IWYUPragmaExport[] = "// IWYU pragma: export";
-const char IWYUPragmaBeginExports[] = "// IWYU pragma: begin_exports";
+llvm::Optional parseIWYUPragma(const char *Text) {
+  // This gets called for every comment seen in the preamble, so it's quite 
hot.

kadircet wrote:
> i think this also deserves a comment to make sure people won't refactor it in 
> the future to take in a stringref.
i'd actually return an empty stringref, instead of `None`. it makes logic in 
callers less complicated and I don't think we convey any different signal 
between None vs empty right now (at least a signal that can be used by the 
callers)



Comment at: clang-tools-extra/clangd/Headers.cpp:27
+  // This gets called for every comment seen in the preamble, so it's quite 
hot.
+  constexpr char IWYUPragma[] = "// IWYU pragma: ";
+  if (strncmp(Text, IWYUPragma, strlen(IWYUPragma)))

nit: `static constexpr StringLiteral` instead (we can just use `size()` 
afterwards instead of calling `strlen` a bunch.



Comment at: clang-tools-extra/clangd/unittests/HeadersTests.cpp:456
+  << "Only // comments supported!";
+  EXPECT_EQ(parseIWYUPragma("//  IWYU pragma: keep"), llvm::None)
+  << "Sensitive to whitespace";

it might be useful to get rid of the space between `:` and `keep` as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135314

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

[PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added inline comments.



Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:39
 
   const static auto char_min = std::numeric_limits::min();
   const static auto uchar_min = std::numeric_limits::min();

shafik wrote:
> We use `signed char` for the max case but `char` for the min case. Maybe we 
> need a max using `char` and a min using `signed char`.
I'll look into this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135170

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

[PATCH] D135256: [clang] Add Create method for CXXBoolLiteralExpr

[David Spickett via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG7448f38898a8: [clang] Add Create method for 
CXXBoolLiteralExpr (authored by DavidSpickett).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135256

Files:
  clang/include/clang/AST/ExprCXX.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/Sema/SemaTemplate.cpp


Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -7829,8 +7829,8 @@
 E = new (Context) CharacterLiteral(Arg.getAsIntegral().getZExtValue(),
Kind, T, Loc);
   } else if (T->isBooleanType()) {
-E = new (Context) CXXBoolLiteralExpr(Arg.getAsIntegral().getBoolValue(),
- T, Loc);
+E = CXXBoolLiteralExpr::Create(Context, Arg.getAsIntegral().getBoolValue(),
+   T, Loc);
   } else if (T->isNullPtrType()) {
 E = new (Context) CXXNullPtrLiteralExpr(Context.NullPtrTy, Loc);
   } else {
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -7922,8 +7922,8 @@
   if (!ToLocationOrErr)
 return ToLocationOrErr.takeError();
 
-  return new (Importer.getToContext()) CXXBoolLiteralExpr(
-  E->getValue(), *ToTypeOrErr, *ToLocationOrErr);
+  return CXXBoolLiteralExpr::Create(Importer.getToContext(), E->getValue(),
+*ToTypeOrErr, *ToLocationOrErr);
 }
 
 ExpectedStmt ASTNodeImporter::VisitMemberExpr(MemberExpr *E) {
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -730,6 +730,11 @@
   explicit CXXBoolLiteralExpr(EmptyShell Empty)
   : Expr(CXXBoolLiteralExprClass, Empty) {}
 
+  static CXXBoolLiteralExpr *Create(const ASTContext &C, bool Val, QualType Ty,
+SourceLocation Loc) {
+return new (C) CXXBoolLiteralExpr(Val, Ty, Loc);
+  }
+
   bool getValue() const { return CXXBoolLiteralExprBits.Value; }
   void setValue(bool V) { CXXBoolLiteralExprBits.Value = V; }
 


Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -7829,8 +7829,8 @@
 E = new (Context) CharacterLiteral(Arg.getAsIntegral().getZExtValue(),
Kind, T, Loc);
   } else if (T->isBooleanType()) {
-E = new (Context) CXXBoolLiteralExpr(Arg.getAsIntegral().getBoolValue(),
- T, Loc);
+E = CXXBoolLiteralExpr::Create(Context, Arg.getAsIntegral().getBoolValue(),
+   T, Loc);
   } else if (T->isNullPtrType()) {
 E = new (Context) CXXNullPtrLiteralExpr(Context.NullPtrTy, Loc);
   } else {
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -7922,8 +7922,8 @@
   if (!ToLocationOrErr)
 return ToLocationOrErr.takeError();
 
-  return new (Importer.getToContext()) CXXBoolLiteralExpr(
-  E->getValue(), *ToTypeOrErr, *ToLocationOrErr);
+  return CXXBoolLiteralExpr::Create(Importer.getToContext(), E->getValue(),
+*ToTypeOrErr, *ToLocationOrErr);
 }
 
 ExpectedStmt ASTNodeImporter::VisitMemberExpr(MemberExpr *E) {
Index: clang/include/clang/AST/ExprCXX.h
===
--- clang/include/clang/AST/ExprCXX.h
+++ clang/include/clang/AST/ExprCXX.h
@@ -730,6 +730,11 @@
   explicit CXXBoolLiteralExpr(EmptyShell Empty)
   : Expr(CXXBoolLiteralExprClass, Empty) {}
 
+  static CXXBoolLiteralExpr *Create(const ASTContext &C, bool Val, QualType Ty,
+SourceLocation Loc) {
+return new (C) CXXBoolLiteralExpr(Val, Ty, Loc);
+  }
+
   bool getValue() const { return CXXBoolLiteralExprBits.Value; }
   void setValue(bool V) { CXXBoolLiteralExprBits.Value = V; }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 7448f38 - [clang] Add Create method for CXXBoolLiteralExpr

[David Spickett via cfe-commits]

Author: David Spickett
Date: 2022-10-06T09:35:14Z
New Revision: 7448f38898a8a0796fa66f1ebcd07c475f329dc4

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

LOG: [clang] Add Create method for CXXBoolLiteralExpr

Reviewed By: shafik

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

Added: 


Modified: 
clang/include/clang/AST/ExprCXX.h
clang/lib/AST/ASTImporter.cpp
clang/lib/Sema/SemaTemplate.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 967a74db916d8..104fd92784e74 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ b/clang/include/clang/AST/ExprCXX.h
@@ -730,6 +730,11 @@ class CXXBoolLiteralExpr : public Expr {
   explicit CXXBoolLiteralExpr(EmptyShell Empty)
   : Expr(CXXBoolLiteralExprClass, Empty) {}
 
+  static CXXBoolLiteralExpr *Create(const ASTContext &C, bool Val, QualType Ty,
+SourceLocation Loc) {
+return new (C) CXXBoolLiteralExpr(Val, Ty, Loc);
+  }
+
   bool getValue() const { return CXXBoolLiteralExprBits.Value; }
   void setValue(bool V) { CXXBoolLiteralExprBits.Value = V; }
 

diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 5c64cf08f8e1b..65aff71bf691d 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -7922,8 +7922,8 @@ ExpectedStmt 
ASTNodeImporter::VisitCXXBoolLiteralExpr(CXXBoolLiteralExpr *E) {
   if (!ToLocationOrErr)
 return ToLocationOrErr.takeError();
 
-  return new (Importer.getToContext()) CXXBoolLiteralExpr(
-  E->getValue(), *ToTypeOrErr, *ToLocationOrErr);
+  return CXXBoolLiteralExpr::Create(Importer.getToContext(), E->getValue(),
+*ToTypeOrErr, *ToLocationOrErr);
 }
 
 ExpectedStmt ASTNodeImporter::VisitMemberExpr(MemberExpr *E) {

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index e9ceec9a0ec14..f9f34337384ae 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -7829,8 +7829,8 @@ Sema::BuildExpressionFromIntegralTemplateArgument(const 
TemplateArgument &Arg,
 E = new (Context) CharacterLiteral(Arg.getAsIntegral().getZExtValue(),
Kind, T, Loc);
   } else if (T->isBooleanType()) {
-E = new (Context) CXXBoolLiteralExpr(Arg.getAsIntegral().getBoolValue(),
- T, Loc);
+E = CXXBoolLiteralExpr::Create(Context, Arg.getAsIntegral().getBoolValue(),
+   T, Loc);
   } else if (T->isNullPtrType()) {
 E = new (Context) CXXNullPtrLiteralExpr(Context.NullPtrTy, Loc);
   } else {



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

[PATCH] D135314: [clangd] Avoid scanning up to end of file on each comment!

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 3 inline comments as done.
sammccall added inline comments.



Comment at: clang-tools-extra/clangd/Headers.cpp:25
 
-const char IWYUPragmaKeep[] = "// IWYU pragma: keep";
-const char IWYUPragmaExport[] = "// IWYU pragma: export";
-const char IWYUPragmaBeginExports[] = "// IWYU pragma: begin_exports";
+llvm::Optional parseIWYUPragma(const char *Text) {
+  // This gets called for every comment seen in the preamble, so it's quite 
hot.

kadircet wrote:
> kadircet wrote:
> > i think this also deserves a comment to make sure people won't refactor it 
> > in the future to take in a stringref.
> i'd actually return an empty stringref, instead of `None`. it makes logic in 
> callers less complicated and I don't think we convey any different signal 
> between None vs empty right now (at least a signal that can be used by the 
> callers)
The comment is in the header file, extended it to be more explicit about 
avoiding StringRef



Comment at: clang-tools-extra/clangd/Headers.cpp:25
 
-const char IWYUPragmaKeep[] = "// IWYU pragma: keep";
-const char IWYUPragmaExport[] = "// IWYU pragma: export";
-const char IWYUPragmaBeginExports[] = "// IWYU pragma: begin_exports";
+llvm::Optional parseIWYUPragma(const char *Text) {
+  // This gets called for every comment seen in the preamble, so it's quite 
hot.

sammccall wrote:
> kadircet wrote:
> > kadircet wrote:
> > > i think this also deserves a comment to make sure people won't refactor 
> > > it in the future to take in a stringref.
> > i'd actually return an empty stringref, instead of `None`. it makes logic 
> > in callers less complicated and I don't think we convey any different 
> > signal between None vs empty right now (at least a signal that can be used 
> > by the callers)
> The comment is in the header file, extended it to be more explicit about 
> avoiding StringRef
> it makes logic in callers less complicated

I think this encourages a risky pattern for very minimal gain.
We've seen comment handlers are performance-sensitive (each strlen of the 
source file isn't **that** expensive!).
If we bail out explicitly early in the overwhelmingly common no-pragma case, 
then the remaining code is not critical.
If we continue on and "naturally" fail to match any pragma case, I don't see 
performance problems today, however we need to be extra-careful whenever we 
change the code. I'd much prefer to have to read/write an obvious `if (empty) 
return` than to think about performance.

If we're going to prefer an early bailout, StringRef isn't really simpler than 
Optional, and it pushes callers to handle this the right way.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135314

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

[PATCH] D135161: [clang][Lex] Fix a crash on malformed string literals

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

i'll reland the fix without the test case, as it's clearly fixing one of the 
codepaths that'll lead to a crash. it's only the test case that's crashing, 
because i don't think there are certain test cases that exposed literal parser 
to invalid/incomplete input and i am not able to reproduce any of the crashes 
i've seen in buildbots locally (even under asan/msan) and because the bots 
don't have stack traces for whatever reason, it's impossible to chase from my 
side. i've pinged buildbot owners in 
https://discourse.llvm.org/t/buildbots-missing-stack-traces/65753 to see the 
issue fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135161

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

[clang-tools-extra] 5d2d527 - [clangd] Avoid scanning up to end of file on each comment!

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-10-06T11:38:55+02:00
New Revision: 5d2d527c32da2081b814ef8b446bc3e037f74b0a

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

LOG: [clangd] Avoid scanning up to end of file on each comment!

Assigning char* (pointing at comment start) to StringRef was causing us
to scan the rest of the source file looking for the null terminator.

This seems to be eating about 8% of our *total* CPU!

While fixing this, factor out the common bits from the two places we're
parsing IWYU pragmas.

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

Added: 


Modified: 
clang-tools-extra/clangd/Headers.cpp
clang-tools-extra/clangd/Headers.h
clang-tools-extra/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/clangd/unittests/HeadersTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Headers.cpp 
b/clang-tools-extra/clangd/Headers.cpp
index 5231a47487bc7..52b954e921620 100644
--- a/clang-tools-extra/clangd/Headers.cpp
+++ b/clang-tools-extra/clangd/Headers.cpp
@@ -22,9 +22,17 @@
 namespace clang {
 namespace clangd {
 
-const char IWYUPragmaKeep[] = "// IWYU pragma: keep";
-const char IWYUPragmaExport[] = "// IWYU pragma: export";
-const char IWYUPragmaBeginExports[] = "// IWYU pragma: begin_exports";
+llvm::Optional parseIWYUPragma(const char *Text) {
+  // This gets called for every comment seen in the preamble, so it's quite 
hot.
+  constexpr llvm::StringLiteral IWYUPragma = "// IWYU pragma: ";
+  if (strncmp(Text, IWYUPragma.data(), IWYUPragma.size()))
+return llvm::None;
+  Text += IWYUPragma.size();
+  const char *End = Text;
+  while (*End != 0 && *End != '\n')
+++End;
+  return StringRef(Text, End - Text);
+}
 
 class IncludeStructure::RecordHeaders : public PPCallbacks,
 public CommentHandler {
@@ -129,10 +137,10 @@ class IncludeStructure::RecordHeaders : public 
PPCallbacks,
   }
 
   bool HandleComment(Preprocessor &PP, SourceRange Range) override {
-bool Err = false;
-llvm::StringRef Text = SM.getCharacterData(Range.getBegin(), &Err);
-if (Err)
+auto Pragma = parseIWYUPragma(SM.getCharacterData(Range.getBegin()));
+if (!Pragma)
   return false;
+
 if (inMainFile()) {
   // Given:
   //
@@ -150,8 +158,7 @@ class IncludeStructure::RecordHeaders : public PPCallbacks,
   // will know that the next inclusion is behind the IWYU pragma.
   // FIXME: Support "IWYU pragma: begin_exports" and "IWYU pragma:
   // end_exports".
-  if (!Text.startswith(IWYUPragmaExport) &&
-  !Text.startswith(IWYUPragmaKeep))
+  if (!Pragma->startswith("export") && !Pragma->startswith("keep"))
 return false;
   unsigned Offset = SM.getFileOffset(Range.getBegin());
   LastPragmaKeepInMainFileLine =
@@ -161,8 +168,7 @@ class IncludeStructure::RecordHeaders : public PPCallbacks,
   // does not support them properly yet, so they will be not marked as
   // unused.
   // FIXME: Once IncludeCleaner supports export pragmas, remove this.
-  if (!Text.startswith(IWYUPragmaExport) &&
-  !Text.startswith(IWYUPragmaBeginExports))
+  if (!Pragma->startswith("export") && 
!Pragma->startswith("begin_exports"))
 return false;
   Out->HasIWYUExport.insert(
   *Out->getID(SM.getFileEntryForID(SM.getFileID(Range.getBegin();

diff  --git a/clang-tools-extra/clangd/Headers.h 
b/clang-tools-extra/clangd/Headers.h
index ff3f063168325..ba72ad397bf8f 100644
--- a/clang-tools-extra/clangd/Headers.h
+++ b/clang-tools-extra/clangd/Headers.h
@@ -35,6 +35,12 @@ namespace clangd {
 /// Returns true if \p Include is literal include like "path" or .
 bool isLiteralInclude(llvm::StringRef Include);
 
+/// If Text begins an Include-What-You-Use directive, returns it.
+/// Given "// IWYU pragma: keep", returns "keep".
+/// Input is a null-terminated char* as provided by SM.getCharacterData().
+/// (This should not be StringRef as we do *not* want to scan for its length).
+llvm::Optional parseIWYUPragma(const char *Text);
+
 /// Represents a header file to be #include'd.
 struct HeaderFile {
   std::string File;

diff  --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp 
b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index b0ae42a96ecf4..8568079ca1adb 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -17,8 +17,6 @@
 namespace clang {
 namespace clangd {
 namespace {
-const char IWYUPragma[] = "// IWYU pragma: private, include ";
-
 const std::pair IncludeMappings[] = {
 {"include/__stddef_max_align_t.h", ""},
 {"include/__wmmintrin_aes.h", ""},
@@ -712,17 +710,17 @@ collectIWYUHeaderMaps(CanonicalIncludes *Includes)

[PATCH] D135314: [clangd] Avoid scanning up to end of file on each comment!

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
sammccall marked an inline comment as done.
Closed by commit rG5d2d527c32da: [clangd] Avoid scanning up to end of file on 
each comment! (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D135314?vs=465531&id=465684#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135314

Files:
  clang-tools-extra/clangd/Headers.cpp
  clang-tools-extra/clangd/Headers.h
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp
  clang-tools-extra/clangd/unittests/HeadersTests.cpp

Index: clang-tools-extra/clangd/unittests/HeadersTests.cpp
===
--- clang-tools-extra/clangd/unittests/HeadersTests.cpp
+++ clang-tools-extra/clangd/unittests/HeadersTests.cpp
@@ -9,6 +9,7 @@
 #include "Headers.h"
 
 #include "Compiler.h"
+#include "Matchers.h"
 #include "TestFS.h"
 #include "TestTU.h"
 #include "clang/Basic/TokenKinds.h"
@@ -30,6 +31,7 @@
 using ::testing::AllOf;
 using ::testing::Contains;
 using ::testing::ElementsAre;
+using ::testing::Eq;
 using ::testing::IsEmpty;
 using ::testing::Not;
 using ::testing::UnorderedElementsAre;
@@ -445,6 +447,18 @@
   EXPECT_FALSE(Includes.hasIWYUExport(getID("none.h", Includes)));
 }
 
+TEST(Headers, ParseIWYUPragma) {
+  EXPECT_THAT(parseIWYUPragma("// IWYU pragma: keep"), HasValue(Eq("keep")));
+  EXPECT_THAT(parseIWYUPragma("// IWYU pragma: keep\netc"),
+  HasValue(Eq("keep")));
+  EXPECT_EQ(parseIWYUPragma("/* IWYU pragma: keep"), llvm::None)
+  << "Only // comments supported!";
+  EXPECT_EQ(parseIWYUPragma("//  IWYU pragma: keep"), llvm::None)
+  << "Sensitive to whitespace";
+  EXPECT_EQ(parseIWYUPragma("// IWYU pragma:keep"), llvm::None)
+  << "Sensitive to whitespace";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -17,8 +17,6 @@
 namespace clang {
 namespace clangd {
 namespace {
-const char IWYUPragma[] = "// IWYU pragma: private, include ";
-
 const std::pair IncludeMappings[] = {
 {"include/__stddef_max_align_t.h", ""},
 {"include/__wmmintrin_aes.h", ""},
@@ -712,17 +710,17 @@
 PragmaCommentHandler(CanonicalIncludes *Includes) : Includes(Includes) {}
 
 bool HandleComment(Preprocessor &PP, SourceRange Range) override {
-  llvm::StringRef Text =
-  Lexer::getSourceText(CharSourceRange::getCharRange(Range),
-   PP.getSourceManager(), PP.getLangOpts());
-  if (!Text.consume_front(IWYUPragma))
+  auto Pragma = parseIWYUPragma(
+  PP.getSourceManager().getCharacterData(Range.getBegin()));
+  if (!Pragma || !Pragma->consume_front("private, include "))
 return false;
   auto &SM = PP.getSourceManager();
   // We always insert using the spelling from the pragma.
   if (auto *FE = SM.getFileEntryForID(SM.getFileID(Range.getBegin(
-Includes->addMapping(
-FE->getLastRef(),
-isLiteralInclude(Text) ? Text.str() : ("\"" + Text + "\"").str());
+Includes->addMapping(FE->getLastRef(),
+ isLiteralInclude(*Pragma)
+ ? Pragma->str()
+ : ("\"" + *Pragma + "\"").str());
   return false;
 }
 
Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -35,6 +35,12 @@
 /// Returns true if \p Include is literal include like "path" or .
 bool isLiteralInclude(llvm::StringRef Include);
 
+/// If Text begins an Include-What-You-Use directive, returns it.
+/// Given "// IWYU pragma: keep", returns "keep".
+/// Input is a null-terminated char* as provided by SM.getCharacterData().
+/// (This should not be StringRef as we do *not* want to scan for its length).
+llvm::Optional parseIWYUPragma(const char *Text);
+
 /// Represents a header file to be #include'd.
 struct HeaderFile {
   std::string File;
Index: clang-tools-extra/clangd/Headers.cpp
===
--- clang-tools-extra/clangd/Headers.cpp
+++ clang-tools-extra/clangd/Headers.cpp
@@ -22,9 +22,17 @@
 namespace clang {
 namespace clangd {
 
-const char IWYUPragmaKeep[] = "// IWYU pragma: keep";
-const char IWYUPragmaExport[] = "// IWYU pragma: export";
-const char IWYUPragmaBeginExports[] = "// IWYU pragma: begin_exports";
+llvm::Optional parseIWYUPragma(const char *Text) {
+  // This gets called for every comment seen in the preamble, so it's quite hot.
+  constexpr llvm::StringLiteral IWYUPragma = "// IWYU pr

[PATCH] D135161: [clang][Lex] Fix a crash on malformed string literals

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D135161#3839510 , @kadircet wrote:

> i'll reland the fix without the test case, as it's clearly fixing one of the 
> codepaths that'll lead to a crash. it's only the test case that's crashing, 
> because i don't think there are certain test cases that exposed literal 
> parser to invalid/incomplete input and i am not able to reproduce any of the 
> crashes i've seen in buildbots locally (even under asan/msan) and because the 
> bots don't have stack traces for whatever reason, it's impossible to chase 
> from my side. i've pinged buildbot owners in 
> https://discourse.llvm.org/t/buildbots-missing-stack-traces/65753 to see the 
> issue fixed.

Makes sense. The fix is correct and if it wasn't it would affect all bots 
equally (I highly doubt that code path is exercised anywhere but the lex test)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135161

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

[PATCH] D134529: [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM with a few minor NITs




Comment at: clang/include/clang/Sema/Overload.h:1024
   /// candidates for operator Op.
-  bool shouldAddReversed(OverloadedOperatorKind Op);
+  bool mayAddReversed(OverloadedOperatorKind Op);
 

usaxena95 wrote:
> ilya-biryukov wrote:
> > I am not sure the new name adds clarity.
> > It's unclear what the `true` return value means here. `should` clearly 
> > indicated returning true means the code has to proceed with adding the 
> > reversed operator. `may` means the code can choose to do so or not, I don't 
> > think that's quite right. `should` was really a better choice here.
> > 
> > That said, I don't think the rename itself is a bad idea, maybe there is a 
> > better name, but I couldn't come up with one.
> Just to be clear, it is possible to not reverse the args even if this returns 
> true now since we do not do full check for `==` here.
> 
> I renamed it `allowsReversed` on the same line of `AllowRewrittenCandidates`.
> 
> Intention is to convey that if this is true then reversing is allowed and you 
> can choose not to do so as well.
The new name LG, thanks.



Comment at: clang/lib/Sema/SemaOverload.cpp:976
 bool OverloadCandidateSet::OperatorRewriteInfo::shouldAddReversed(
-ASTContext &Ctx, const FunctionDecl *FD) {
-  if (!shouldAddReversed(FD->getDeclName().getCXXOverloadedOperator()))
+Sema &S, ArrayRef Args, const FunctionDecl *FD) {
+  auto Op = FD->getOverloadedOperator();

usaxena95 wrote:
> ilya-biryukov wrote:
> > NIT: same suggestion as before. Just pass `Expr* FirstOperand` as the 
> > parameter instead of an array.
> I would prefer not to use a `FirstOperand` for `shouldAddReversed` as it 
> serves more than just `operator==`. 
> 
> It might be confusing/error-prone for the users as to what is the "first 
> operand" here (Args[0] or Args[1] ? In reversed args or original args ?). I 
> think it would be a better API to just have original args as the parameter 
> let this function decide which is the `FirstOperand`.
> It might be confusing/error-prone for the users as to what is the "first 
> operand" here (Args[0] or Args[1] ? 
Yeah, good point. It's much better to have this choice in this function to 
avoid the complicated contract on each call site.
That being said...

> let this function decide which is the FirstOperand.
We should have a comment explaining why *first* operand is `Args[1]`. It's a 
bit non-obvious, but followed from the specification.

I also suggest to add an extra sanity check: `assert(OriginalArgs.size() == 2)`.




Comment at: 
clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp:269
+};
+bool b = B() == B(); // ok. No rewrite due to const.
+

usaxena95 wrote:
> ilya-biryukov wrote:
> > Also due to different parameter types (`T` vs `B`)?
> > So the description is incomplete or am I missing something?
> This verifies that adding `const` would solve the ambiguity without adding a 
> matching `!=`. The `!=` here does not match because it is non-template and, 
> yes, because of different param types.
However, the rewritten operator does get added in that case, right?
So my reading is that the 'rewrite' does happen, but since conversions are the 
same, the  
[(over.match.best.genera)p2.8](https://eel.is/c++draft/over.match#best.general-2.8)
  kick in and there is no ambiguity:
>Given these definitions, a viable function F1 is defined to be a better 
>function than another viable function F2 ...
> - F2 is a rewritten candidate ([over.match.oper]) and F1 is not


I suggest to:
- remove the `operator !=` completely, we check having it does not block adding 
rewritten operators in other tests,
- change the comment to say `No ambiguity due to const` as the rewrite actually 
does happen.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134529

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

[clang] 36f77e2 - Revert "Revert "[clang][Lex] Fix a crash on malformed string literals""

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2022-10-06T11:41:18+02:00
New Revision: 36f77e20d9aaaf93a9b00ec1bd6b7e3ceb4918b9

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

LOG: Revert "Revert "[clang][Lex] Fix a crash on malformed string literals""

This reverts commit feea7ef23cb1bef92d363cc613052f8f3a878fc2.
Drops the test case, see https://reviews.llvm.org/D135161#3839510

Added: 


Modified: 
clang/lib/Lex/LiteralSupport.cpp
clang/test/Lexer/char-escapes-delimited.c
clang/unittests/Lex/LexerTest.cpp

Removed: 




diff  --git a/clang/lib/Lex/LiteralSupport.cpp 
b/clang/lib/Lex/LiteralSupport.cpp
index 1a48a68c28b62..160240e49dd70 100644
--- a/clang/lib/Lex/LiteralSupport.cpp
+++ b/clang/lib/Lex/LiteralSupport.cpp
@@ -545,7 +545,6 @@ static bool ProcessNamedUCNEscape(const char *ThisTokBegin,
diag::err_delimited_escape_missing_brace)
   << StringRef(&ThisTokBuf[-1], 1);
 }
-ThisTokBuf++;
 return false;
   }
   ThisTokBuf++;

diff  --git a/clang/test/Lexer/char-escapes-delimited.c 
b/clang/test/Lexer/char-escapes-delimited.c
index 65e3dc740e3b4..43ade65a58309 100644
--- a/clang/test/Lexer/char-escapes-delimited.c
+++ b/clang/test/Lexer/char-escapes-delimited.c
@@ -94,7 +94,7 @@ void named(void) {
 
   unsigned h = U'\N{LOTUS}';  // ext-warning {{extension}} 
cxx2b-warning {{C++2b}}
   unsigned i = u'\N{GREEK CAPITAL LETTER DELTA}'; // ext-warning {{extension}} 
cxx2b-warning {{C++2b}}
-  char j = '\NN'; // expected-error {{expected 
'{' after '\N' escape sequence}}
+  char j = '\NN'; // expected-error {{expected 
'{' after '\N' escape sequence}} expected-warning {{multi-character character 
constant}}
   unsigned k = u'\N{LOTUS';   // expected-error 
{{incomplete universal character name}}
 }
 

diff  --git a/clang/unittests/Lex/LexerTest.cpp 
b/clang/unittests/Lex/LexerTest.cpp
index 0ad644ce71465..c5982b859289a 100644
--- a/clang/unittests/Lex/LexerTest.cpp
+++ b/clang/unittests/Lex/LexerTest.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
+#include "clang/Lex/LiteralSupport.h"
 #include "clang/Lex/MacroArgs.h"
 #include "clang/Lex/MacroInfo.h"
 #include "clang/Lex/ModuleLoader.h"



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

[PATCH] D126481: [analyzer] Handle SymbolCast in SValBuilder

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

I don't see this case different to the unary expressions. Consider the unary 
minus for example. Let's say, the symbol `a` is constrained to `[1, 1]` and 
then `-a` is constrained to `[-2, -2]`. This is clearly an infeasible state and 
the analyzer will terminate the path. So, no further call of SValBuilder should 
happen from that point. That is, it is meaningless to evaluate `-(-a)` if there 
is a contradiction in the constraints that are assigned to the sub-symbols of 
the the symbol tree of `-(-a)`.
Here is a test case that demonstrates this (this passes with latest llvm/main):

  // RUN: %clang_analyze_cc1 %s \
  // RUN:   -analyzer-checker=core \
  // RUN:   -analyzer-checker=debug.ExprInspection \
  // RUN:   -analyzer-config eagerly-assume=false \
  // RUN:   -verify
  
  extern void abort() __attribute__((__noreturn__));
  #define assert(expr) ((expr) ? (void)(0) : abort())
  
  void clang_analyzer_warnIfReached();
  void clang_analyzer_eval(int);
  
  void test(int a, int b) {
assert(-a == -1);
assert(a == 1);
clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
clang_analyzer_eval(-(-a) == 1); // expected-warning{{TRUE}}
  
assert(-b <= -2);
assert(b == 1);
clang_analyzer_warnIfReached(); // unreachable!, no-warning
clang_analyzer_eval(-(-b) == 1); // no-warning
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126481

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

[PATCH] D135247: [clang][analyzer] Add stream functions to StdLibraryFunctionsChecker.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> The two checkers work together and 'apiModeling.StdCLibraryFunctions'
> and its 'ModelPOSIX=true' option should be now a dependency of
> checker 'alpha.unix.Stream'.

Is there a way to stop the analysis with an error if "ModelPOSIX=true" is not 
set but the  'alpha.unix.Stream' checker is enabled? @Szelethus, maybe you have 
some insights with this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135247

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

[PATCH] D135115: [clang-format] update --files help description

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

HazardyKnusperkeks added a comment.

It's too long ago for me. Does the `--files` option take multiple file names on 
the command line, or a file containing the file names? If the latter the patch 
generally looks good.

Except for the `@` part. (Which also isn't handles by clang-format, or is 
it?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135115

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

[PATCH] D134947: [analyzer] Fix liveness of Symbols for values in regions reffered by LazyCompoundVal

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource marked 2 inline comments as done.
tomasz-kaminski-sonarsource added a comment.

Where should I add As a result of our internal test on around ~170 projects 
(~20 Widnows, ~150 Linux) that are compromised of several hundreds of millions 
of lines of code, the impact on the files that parsed correctly was: 5 issues 
disappearing and 4 new issues.  We investigated all of the reports, and the 
changes seemed justified:

- removing issues from impossible paths, that are now correctly recognized due 
to preserved constraints
- the value of location not being reported as garbage
- undefined behavior for overflowing shift operation, as we preserve constrain 
of the bits
- recognition of null pointer values


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134947

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

[PATCH] D134529: [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 465686.
usaxena95 marked 2 inline comments as done.
usaxena95 added a comment.

Addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134529

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Overload.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp

Index: clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
@@ -125,46 +125,204 @@
   bool b2 = 0 == ADL::type();
 }
 
-// Various C++17 cases that are known to be broken by the C++20 rules.
-namespace problem_cases {
-  // We can have an ambiguity between an operator and its reversed form. This
-  // wasn't intended by the original "consistent comparison" proposal, and we
-  // allow it as extension, picking the non-reversed form.
-  struct A {
-bool operator==(const A&); // expected-note {{ambiguity is between a regular call to this operator and a call with the argument order reversed}}
-  };
-  bool cmp_non_const = A() == A(); // expected-warning {{ambiguous}}
+namespace P2468R2 {
+// Problem cases prior to P2468R2 but now intentionally rejected.
+struct SymmetricNonConst {
+  bool operator==(const SymmetricNonConst&); // expected-note {{ambiguity is between a regular call to this operator and a call with the argument order reversed}}
+  // expected-note@-1 {{mark 'operator==' as const or add a matching 'operator!=' to resolve the ambiguity}}
+};
+bool cmp_non_const = SymmetricNonConst() == SymmetricNonConst(); // expected-warning {{ambiguous}}
 
-  struct B {
-virtual bool operator==(const B&) const;
-  };
-  struct D : B {
-bool operator==(const B&) const override; // expected-note {{operator}}
-  };
-  bool cmp_base_derived = D() == D(); // expected-warning {{ambiguous}}
+struct SymmetricConst {
+  bool operator==(const SymmetricConst&) const;
+};
+bool cmp_const = SymmetricConst() == SymmetricConst();
 
-  template struct CRTPBase {
-bool operator==(const T&) const; // expected-note {{operator}} expected-note {{reversed}}
-bool operator!=(const T&) const; // expected-note {{non-reversed}}
-  };
-  struct CRTP : CRTPBase {};
-  bool cmp_crtp = CRTP() == CRTP(); // expected-warning-re {{ambiguous despite there being a unique best viable function{{$}}
-  bool cmp_crtp2 = CRTP() != CRTP(); // expected-warning {{ambiguous despite there being a unique best viable function with non-reversed arguments}}
-
-  // Given a choice between a rewritten and non-rewritten function with the
-  // same parameter types, where the rewritten function is reversed and each
-  // has a better conversion for one of the two arguments, prefer the
-  // non-rewritten one.
-  using UBool = signed char; // ICU uses this.
-  struct ICUBase {
-virtual UBool operator==(const ICUBase&) const;
-UBool operator!=(const ICUBase &arg) const { return !operator==(arg); }
-  };
-  struct ICUDerived : ICUBase {
-UBool operator==(const ICUBase&) const override; // expected-note {{declared here}} expected-note {{ambiguity is between}}
-  };
-  bool cmp_icu = ICUDerived() != ICUDerived(); // expected-warning {{ambiguous}} expected-warning {{'bool', not 'UBool'}}
+struct SymmetricNonConstWithoutConstRef {
+  bool operator==(SymmetricNonConstWithoutConstRef);
+};
+bool cmp_non_const_wo_ref = SymmetricNonConstWithoutConstRef() == SymmetricNonConstWithoutConstRef();
+
+struct B {
+  virtual bool operator==(const B&) const;
+};
+struct D : B {
+  bool operator==(const B&) const override; // expected-note {{operator}}
+};
+bool cmp_base_derived = D() == D(); // expected-warning {{ambiguous}}
+
+// Reversed "3" not used because we find "2".
+// Rewrite != from "3" but warn that "chosen rewritten candidate must return cv-bool".
+using UBool = signed char;
+struct ICUBase {
+  virtual UBool operator==(const ICUBase&) const; // 1.
+  UBool operator!=(const ICUBase &arg) const { return !operator==(arg); } // 2.
+};
+struct ICUDerived : ICUBase {
+  // 3.
+  UBool operator==(const ICUBase&) const override; // expected-note {{declared here}}
+};
+bool cmp_icu = ICUDerived() != ICUDerived(); // expected-warning {{ISO C++20 requires return type of selected 'operator==' function for rewritten '!=' comparison to be 'bool', not 'UBool' (aka 'signed char')}}
+// Accepted by P2468R2.
+// 1
+struct S {
+  bool operator==(const S&) { return true; }
+  bool operator!=(const S&) { return false; }
+};
+bool ts = S{} != S{};
+// 2
+template struct CRTPBase {
+  bool operator==(const T&) const;
+  bool operator!=(const T&) const;
+};
+struct CRTP : CRTPBa

[PATCH] D135257: [clangd][Tweak] Make sure enclosing function doesnt have invalid children

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I don't think it's unreasonable to protect against `nullptr` in individual 
fields. We need **some** value for it and null is a reasonable choice.
However, it feels completely wrong to have `nullptr` in collections. Those 
should be filtered out upon creation, if possible.
It seems perfectly fine for a collection to have less elements if some of them 
are invalid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135257

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

[PATCH] D135257: [clangd][Tweak] Make sure enclosing function doesnt have invalid children

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I guess my question is: is there any fundamental reason why we think we 
**need** to allow `nullptr` children in `Stmt`? What are the places that 
actually need it?

A quick search shows there are quite a few places in our codebase (many 
google-internal) that don't check for `nullptr` and are subject to the same 
breakage.
It seems that having a standard primitive for iterating over only non-null 
children and using it in almost all of the places is appropriate. I am trying 
to understand what are the use-cases that have to see those null children.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135257

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

[clang] 89810ce - [RelativeVTablesABI] Convert tests to opaque pointers (NFC)

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-10-06T12:37:42+02:00
New Revision: 89810cee544db09fbf7d578c6e93db7a9cb0d9e5

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

LOG: [RelativeVTablesABI] Convert tests to opaque pointers (NFC)

Converted using https://gist.github.com/nikic/98357b71fd67756b0f064c9517b62a34
with manual fixup, primarily to drop check lines for types that
no longer appear with opaque pointers.

Added: 


Modified: 
clang/test/CodeGenCXX/RelativeVTablesABI/diamond-inheritance.cpp
clang/test/CodeGenCXX/RelativeVTablesABI/diamond-virtual-inheritance.cpp
clang/test/CodeGenCXX/RelativeVTablesABI/dynamic-cast.cpp
clang/test/CodeGenCXX/RelativeVTablesABI/multiple-inheritance.cpp
clang/test/CodeGenCXX/RelativeVTablesABI/type-info.cpp
clang/test/CodeGenCXX/RelativeVTablesABI/vbase-offset.cpp
clang/test/CodeGenCXX/RelativeVTablesABI/virtual-function-call.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/RelativeVTablesABI/diamond-inheritance.cpp 
b/clang/test/CodeGenCXX/RelativeVTablesABI/diamond-inheritance.cpp
index 6990a068b5ab..cd3ce22fc8e9 100644
--- a/clang/test/CodeGenCXX/RelativeVTablesABI/diamond-inheritance.cpp
+++ b/clang/test/CodeGenCXX/RelativeVTablesABI/diamond-inheritance.cpp
@@ -1,28 +1,23 @@
 // Diamond inheritance.
 // A more complicated multiple inheritance example that includes longer chain 
of inheritance and a common ancestor.
 
-// RUN: %clang_cc1 -no-opaque-pointers %s -triple=aarch64-unknown-fuchsia -O1 
-S -o - -emit-llvm -fhalf-no-semantic-interposition | FileCheck %s
-
-// CHECK-DAG: %class.B = type { %class.A }
-// CHECK-DAG: %class.A = type { i32 (...)** }
-// CHECK-DAG: %class.C = type { %class.A }
-// CHECK-DAG: %class.D = type { %class.B, %class.C }
+// RUN: %clang_cc1 %s -triple=aarch64-unknown-fuchsia -O1 -S -o - -emit-llvm 
-fhalf-no-semantic-interposition | FileCheck %s
 
 // VTable for B should contain offset to top (0), RTTI pointer, A::foo(), and 
B::barB().
-// CHECK: @_ZTV1B.local = private unnamed_addr constant { [4 x i32] } { [4 x 
i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint ({ i8*, i8*, i8* }** 
@_ZTI1B.rtti_proxy to i64), i64 ptrtoint (i32* getelementptr inbounds ({ [4 x 
i32] }, { [4 x i32] }* @_ZTV1B.local, i32 0, i32 0, i32 2) to i64)) to i32), 
i32 trunc (i64 sub (i64 ptrtoint (void (%class.A*)* dso_local_equivalent 
@_ZN1A3fooEv to i64), i64 ptrtoint (i32* getelementptr inbounds ({ [4 x i32] }, 
{ [4 x i32] }* @_ZTV1B.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc 
(i64 sub (i64 ptrtoint (void (%class.B*)* dso_local_equivalent @_ZN1B4barBEv to 
i64), i64 ptrtoint (i32* getelementptr inbounds ({ [4 x i32] }, { [4 x i32] }* 
@_ZTV1B.local, i32 0, i32 0, i32 2) to i64)) to i32)] }, align 4
+// CHECK: @_ZTV1B.local = private unnamed_addr constant { [4 x i32] } { [4 x 
i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1B.rtti_proxy to i64), 
i64 ptrtoint (ptr getelementptr inbounds ({ [4 x i32] }, ptr @_ZTV1B.local, i32 
0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr 
dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr getelementptr 
inbounds ({ [4 x i32] }, ptr @_ZTV1B.local, i32 0, i32 0, i32 2) to i64)) to 
i32), i32 trunc (i64 sub (i64 ptrtoint (ptr dso_local_equivalent @_ZN1B4barBEv 
to i64), i64 ptrtoint (ptr getelementptr inbounds ({ [4 x i32] }, ptr 
@_ZTV1B.local, i32 0, i32 0, i32 2) to i64)) to i32)] }, align 4
 
 // VTable for C should contain offset to top (0), RTTI pointer, A::foo(), and 
C::barC().
-// CHECK: @_ZTV1C.local = private unnamed_addr constant { [4 x i32] } { [4 x 
i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint ({ i8*, i8*, i8* }** 
@_ZTI1C.rtti_proxy to i64), i64 ptrtoint (i32* getelementptr inbounds ({ [4 x 
i32] }, { [4 x i32] }* @_ZTV1C.local, i32 0, i32 0, i32 2) to i64)) to i32), 
i32 trunc (i64 sub (i64 ptrtoint (void (%class.A*)* dso_local_equivalent 
@_ZN1A3fooEv to i64), i64 ptrtoint (i32* getelementptr inbounds ({ [4 x i32] }, 
{ [4 x i32] }* @_ZTV1C.local, i32 0, i32 0, i32 2) to i64)) to i32), i32 trunc 
(i64 sub (i64 ptrtoint (void (%class.C*)* dso_local_equivalent @_ZN1C4barCEv to 
i64), i64 ptrtoint (i32* getelementptr inbounds ({ [4 x i32] }, { [4 x i32] }* 
@_ZTV1C.local, i32 0, i32 0, i32 2) to i64)) to i32)] }, align 4
+// CHECK: @_ZTV1C.local = private unnamed_addr constant { [4 x i32] } { [4 x 
i32] [i32 0, i32 trunc (i64 sub (i64 ptrtoint (ptr @_ZTI1C.rtti_proxy to i64), 
i64 ptrtoint (ptr getelementptr inbounds ({ [4 x i32] }, ptr @_ZTV1C.local, i32 
0, i32 0, i32 2) to i64)) to i32), i32 trunc (i64 sub (i64 ptrtoint (ptr 
dso_local_equivalent @_ZN1A3fooEv to i64), i64 ptrtoint (ptr getelementptr 
inbounds ({ [4 x i32] }, ptr @_ZTV1C.local, i32 0, i32 0, i32 2) to i64)) to 
i32), i32 trunc (i64 sub (i64 pt

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

[Mikhail Goncharov via Phabricator via cfe-commits]

goncharov added subscribers: dexonsmith, goncharov.
goncharov added a comment.

That change might be problematic for content addressing storages. E.g. 
clang/test/Driver/cl-pch-showincludes.cpp started to fail as 
clang/test/Driver/header{0,1,3,4}.h are all identical and can be symlinked. cc 
@dexonsmith


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135220

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

[clang] 956f7f2 - [CodeGenCXX] Remove typed pointer check lines from test (NFC)

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2022-10-06T13:06:02+02:00
New Revision: 956f7f2b4f785271f7fde2a6ff83472d9451968f

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

LOG: [CodeGenCXX] Remove typed pointer check lines from test (NFC)

This test already has check lines for opaque pointers, remove the
unnecessary typed pointer check lines.

Added: 


Modified: 
clang/test/CodeGenCXX/threadlocal_address.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/threadlocal_address.cpp 
b/clang/test/CodeGenCXX/threadlocal_address.cpp
index f5af5c25facd..cb63bc275990 100644
--- a/clang/test/CodeGenCXX/threadlocal_address.cpp
+++ b/clang/test/CodeGenCXX/threadlocal_address.cpp
@@ -1,7 +1,6 @@
 // Test that the use of thread local variables would be wrapped by 
@llvm.threadlocal.address intrinsics.
 // RUN: %clang_cc1 -std=c++11 -emit-llvm -triple x86_64 -o - %s 
-disable-llvm-passes | FileCheck %s
 // RUN: %clang_cc1 -std=c++11 -emit-llvm -triple aarch64 -o - -O1 %s | 
FileCheck %s -check-prefix=CHECK-O1
-// RUN: %clang_cc1 -std=c++11 -no-opaque-pointers -emit-llvm -triple x86_64 -o 
- %s -disable-llvm-passes | FileCheck %s -check-prefix=CHECK-NOOPAQUE
 thread_local int i;
 int g() {
   i++;
@@ -29,16 +28,6 @@ int g() {
 // CHECK-O1-NEXT:   %[[INC:.+]] = add nsw i32 %[[VAL]], 1
 // CHECK-O1-NEXT:   store i32 %[[INC]], ptr %[[I_ADDR]]
 // CHECK-O1-NEXT:   ret i32 %[[INC]]
-//
-// CHECK-NOOPAQUE-LABEL: @_Z1gv
-// CHECK-NOOPAQUE-NEXT: entry:
-// CHECK-NOOPAQUE-NEXT:   %[[I_ADDR:.+]] = call align 4 i32* 
@llvm.threadlocal.address.p0i32(i32* align 4 @i)
-// CHECK-NOOPAQUE-NEXT:   %[[VAL:.+]] = load i32, i32* %[[I_ADDR]]
-// CHECK-NOOPAQUE-NEXT:   %[[INC:.+]] = add nsw i32 %[[VAL]], 1
-// CHECK-NOOPAQUE-NEXT:   store i32 %[[INC]], i32* %[[I_ADDR]]
-// CHECK-NOOPAQUE-NEXT:   %[[IA2:.+]] = call align 4 i32* 
@llvm.threadlocal.address.p0i32(i32* align 4 @i)
-// CHECK-NOOPAQUE-NEXT:   %[[RET:.+]] = load i32, i32* %[[IA2]], align 4
-// CHECK-NOOPAQUE-NEXT:   ret i32 %[[RET]]
 int f() {
   thread_local int j = 0;
   j++;
@@ -62,14 +51,4 @@ int f() {
 // CHECK-O1-NEXT:   store i32 %[[INC]], ptr %[[J_ADDR]]
 // CHECK-O1-NEXT:   ret i32 %[[INC]]
 //
-// CHECK-NOOPAQUE: @_Z1fv()
-// CHECK-NOOPAQUE-NEXT: entry
-// CHECK-NOOPAQUE-NEXT: %[[JA:.+]] = call align 4 i32* 
@llvm.threadlocal.address.p0i32(i32* align 4 @_ZZ1fvE1j)
-// CHECK-NOOPAQUE-NEXT: %[[VA:.+]] = load i32, i32* %[[JA]]
-// CHECK-NOOPAQUE-NEXT: %[[INC:.+]] = add nsw i32 %[[VA]], 1
-// CHECK-NOOPAQUE-NEXT: store i32 %[[INC]], i32* %[[JA]], align 4
-// CHECK-NOOPAQUE-NEXT: %[[JA2:.+]] = call align 4 i32* 
@llvm.threadlocal.address.p0i32(i32* align 4 @_ZZ1fvE1j)
-// CHECK-NOOPAQUE-NEXT: %[[RET:.+]] = load i32, i32* %[[JA2]], align 4
-// CHECK-NOOPAQUE-NEXT: ret i32 %[[RET]]
-//
 // CHECK: attributes #[[ATTR_NUM]] = { nocallback nofree nosync nounwind 
readnone speculatable willreturn }



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

[PATCH] D135356: [Format] Fix crash when hitting eof while lexing JS template string

[Sam McCall via Phabricator via cfe-commits]

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

Different loop termination conditions resulted in confusion of whether
*Offset was intended to be inside or outside the token.
This ultimately led to constructing an out-of-range SourceLocation.

Fix by making Offset consistently point *after* the token.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135356

Files:
  clang/lib/Format/FormatTokenLexer.cpp
  clang/unittests/Format/FormatTestJS.cpp


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2145,6 +2145,7 @@
 
   // Crashed at some point.
   verifyFormat("}");
+  verifyFormat("`");
 }
 
 TEST_F(FormatTestJS, TaggedTemplateStrings) {
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -760,6 +760,7 @@
   for (; Offset != Lex->getBuffer().end(); ++Offset) {
 if (Offset[0] == '`') {
   StateStack.pop();
+  ++Offset;
   break;
 }
 if (Offset[0] == '\\') {
@@ -769,11 +770,12 @@
   // '${' introduces an expression interpolation in the template string.
   StateStack.push(LexerState::NORMAL);
   ++Offset;
+  ++Offset;
   break;
 }
   }
 
-  StringRef LiteralText(TmplBegin, Offset - TmplBegin + 1);
+  StringRef LiteralText(TmplBegin, Offset - TmplBegin);
   BacktickToken->setType(TT_TemplateString);
   BacktickToken->Tok.setKind(tok::string_literal);
   BacktickToken->TokenText = LiteralText;
@@ -794,9 +796,7 @@
   StartColumn, Style.TabWidth, Encoding);
   }
 
-  SourceLocation loc = Offset < Lex->getBuffer().end()
-   ? Lex->getSourceLocation(Offset + 1)
-   : SourceMgr.getLocForEndOfFile(ID);
+  SourceLocation loc = Lex->getSourceLocation(Offset);
   resetLexer(SourceMgr.getFileOffset(loc));
 }
 


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2145,6 +2145,7 @@
 
   // Crashed at some point.
   verifyFormat("}");
+  verifyFormat("`");
 }
 
 TEST_F(FormatTestJS, TaggedTemplateStrings) {
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -760,6 +760,7 @@
   for (; Offset != Lex->getBuffer().end(); ++Offset) {
 if (Offset[0] == '`') {
   StateStack.pop();
+  ++Offset;
   break;
 }
 if (Offset[0] == '\\') {
@@ -769,11 +770,12 @@
   // '${' introduces an expression interpolation in the template string.
   StateStack.push(LexerState::NORMAL);
   ++Offset;
+  ++Offset;
   break;
 }
   }
 
-  StringRef LiteralText(TmplBegin, Offset - TmplBegin + 1);
+  StringRef LiteralText(TmplBegin, Offset - TmplBegin);
   BacktickToken->setType(TT_TemplateString);
   BacktickToken->Tok.setKind(tok::string_literal);
   BacktickToken->TokenText = LiteralText;
@@ -794,9 +796,7 @@
   StartColumn, Style.TabWidth, Encoding);
   }
 
-  SourceLocation loc = Offset < Lex->getBuffer().end()
-   ? Lex->getSourceLocation(Offset + 1)
-   : SourceMgr.getLocForEndOfFile(ID);
+  SourceLocation loc = Lex->getSourceLocation(Offset);
   resetLexer(SourceMgr.getFileOffset(loc));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134902: [clang] Implement -fstrict-flex-arrays=3

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/test/CodeGen/object-size-flex-array.c:45
+  // CHECK-STRICT-2: ret i32 -1
+  // CHECK-STRICT-3: ret i32 0
   return OBJECT_SIZE_BUILTIN(f->c, 1);

kees wrote:
> serge-sans-paille wrote:
> > This one worries me a bit, as an array of size 0 is invalid C, unless you 
> > consider the extension of it being a FAM. Shouldn't we emit a warning or 
> > something here? @kees what meaning would you give to that construct under 
> > `-fstrict-flex-arrays=3` ?
> ```
> type identifier[0]
> ```
> when not a fake FAM is the same as:
> 
> ```
> struct { } identifier
> ```
> 
> It's addressable with no size.
> 
> FWIW, this is how GCC is treating it, and opted for no warning. The warning 
> gains nothing and is likely an irritant: if someone is requesting =3, they 
> want this behavior. If they didn't, they'd just use `-Wzero-length-array`. In 
> particular, the Linux kernel is in the position of needing to have 
> zero-length arrays (legacy userspace API) alongside real FAMs, even if we 
> don't reference the zero-length members. So we cannot use 
> '-Wzero-length-array', but we want to make sure no zero-length arrays will 
> ever be used as fake FAMs, as a code quality/style enforcement. This is 
> especially true of FORTIFY_SOURCE, where `__bos()` needs to report 0 instead 
> of -1 for such a destination buffer size, so that we immediately trip 
> compile-time (or at worst, run-time) warnings, to keep any kernel internals 
> from using the deprecated members as a fake FAM.
> 
> Take this case:
> 
> ```
> struct broken {
> int foo;
> int fake_fam[0];
> struct something oops;
> };
> ```
> 
> There have been bugs where the above struct was created because "oops" got 
> added after "fake_fam" by someone not realizing. Under FORTIFY_SOURCE, doing:
> 
> ```
> memcpy(p->fake_fam, src, len);
> ```
> 
> raises no warning when `__bos(p->fake_fam, 1)` returns -1 and will happily 
> stomp on "oops". If `__bos()` returns 0, we can compile-time (or run-time) 
> block the memcpy. (And this holds for -fsanitize=bounds as well: if it is 
> considered to be unknown size, it won't trip on access, but if it's 0-sized, 
> it'll trip.) 
> 
> So, we can't keep zero-length arrays out of the kernel, but we want to be 
> able to enforce that if they DO show up, they will trip warnings quickly.
> 
Thanks for clarifying the situation, esp. the kernel background. This looks 
like a quite specific scenario, but it's fine with me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134902

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

[clang] 38b9d31 - [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2022-10-06T13:21:34+02:00
New Revision: 38b9d313e6945804fffc654f849cfa05ba2c713d

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

LOG: [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

Implement
https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2468r2.html.

Primarily we now accept
```
template struct CRTPBase {
  bool operator==(const T&) const;
  bool operator!=(const T&) const;
};
struct CRTP : CRTPBase {};
bool cmp_crtp = CRTP() == CRTP();
bool cmp_crtp2 = CRTP() != CRTP();
```

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Overload.h
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 93974e89eec14..1652c90337f6e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -415,6 +415,7 @@ C++20 Feature Support
   name is found via ordinary lookup so typedefs are found.
 - Implemented `P0634r3 
`_,
   which removes the requirement for the ``typename`` keyword in certain 
contexts.
+- Implemented The Equality Operator You Are Looking For (`P2468 
`_).
 
 C++2b Feature Support
 ^

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index fd6651ee5d25f..0c7b64c7a94b4 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4703,6 +4703,8 @@ def ext_ovl_ambiguous_oper_binary_reversed : ExtWarn<
 def note_ovl_ambiguous_oper_binary_reversed_self : Note<
   "ambiguity is between a regular call to this operator and a call with the "
   "argument order reversed">;
+def note_ovl_ambiguous_eqeq_reversed_self_non_const : Note<
+  "mark 'operator==' as const or add a matching 'operator!=' to resolve the 
ambiguity">;
 def note_ovl_ambiguous_oper_binary_selected_candidate : Note<
   "candidate function with non-reversed arguments">;
 def note_ovl_ambiguous_oper_binary_reversed_candidate : Note<

diff  --git a/clang/include/clang/Sema/Overload.h 
b/clang/include/clang/Sema/Overload.h
index eb935e0a8cbf3..29d0fa63369f0 100644
--- a/clang/include/clang/Sema/Overload.h
+++ b/clang/include/clang/Sema/Overload.h
@@ -977,12 +977,16 @@ class Sema;
 /// functions to a candidate set.
 struct OperatorRewriteInfo {
   OperatorRewriteInfo()
-  : OriginalOperator(OO_None), AllowRewrittenCandidates(false) {}
-  OperatorRewriteInfo(OverloadedOperatorKind Op, bool AllowRewritten)
-  : OriginalOperator(Op), AllowRewrittenCandidates(AllowRewritten) {}
+  : OriginalOperator(OO_None), OpLoc(), 
AllowRewrittenCandidates(false) {}
+  OperatorRewriteInfo(OverloadedOperatorKind Op, SourceLocation OpLoc,
+  bool AllowRewritten)
+  : OriginalOperator(Op), OpLoc(OpLoc),
+AllowRewrittenCandidates(AllowRewritten) {}
 
   /// The original operator as written in the source.
   OverloadedOperatorKind OriginalOperator;
+  /// The source location of the operator.
+  SourceLocation OpLoc;
   /// Whether we should include rewritten candidates in the overload set.
   bool AllowRewrittenCandidates;
 
@@ -1018,22 +1022,23 @@ class Sema;
   CRK = OverloadCandidateRewriteKind(CRK | CRK_Reversed);
 return CRK;
   }
-
   /// Determines whether this operator could be implemented by a function
   /// with reversed parameter order.
   bool isReversible() {
 return AllowRewrittenCandidates && OriginalOperator &&
(getRewrittenOverloadedOperator(OriginalOperator) != OO_None ||
-shouldAddReversed(OriginalOperator));
+allowsReversed(OriginalOperator));
   }
 
-  /// Determine whether we should consider looking for and adding reversed
-  /// candidates for operator Op.
-  bool shouldAddReversed(OverloadedOperatorKind Op);
+  /// Determine whether reversing parameter order is allowed for operator
+  /// Op.
+  bool allowsReversed(OverloadedOperatorKind Op);
 
   /// Determine whether we should add a rewritten candidate for \p FD with
   /// reversed parameter order.
-  bool shouldAddReversed(ASTContext &Ctx, const FunctionDecl *FD);
+  /// \param OriginalArgs are the original non reversed arguments.
+  bool shouldAddReversed(Sema &S, ArrayRef OriginalArgs,
+

[PATCH] D134529: [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

[Utkarsh Saxena via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG38b9d313e694: [C++20][Clang] P2468R2 The Equality Operator 
You Are Looking For (authored by usaxena95).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134529

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Overload.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp

Index: clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
===
--- clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
+++ clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
@@ -125,46 +125,204 @@
   bool b2 = 0 == ADL::type();
 }
 
-// Various C++17 cases that are known to be broken by the C++20 rules.
-namespace problem_cases {
-  // We can have an ambiguity between an operator and its reversed form. This
-  // wasn't intended by the original "consistent comparison" proposal, and we
-  // allow it as extension, picking the non-reversed form.
-  struct A {
-bool operator==(const A&); // expected-note {{ambiguity is between a regular call to this operator and a call with the argument order reversed}}
-  };
-  bool cmp_non_const = A() == A(); // expected-warning {{ambiguous}}
+namespace P2468R2 {
+// Problem cases prior to P2468R2 but now intentionally rejected.
+struct SymmetricNonConst {
+  bool operator==(const SymmetricNonConst&); // expected-note {{ambiguity is between a regular call to this operator and a call with the argument order reversed}}
+  // expected-note@-1 {{mark 'operator==' as const or add a matching 'operator!=' to resolve the ambiguity}}
+};
+bool cmp_non_const = SymmetricNonConst() == SymmetricNonConst(); // expected-warning {{ambiguous}}
 
-  struct B {
-virtual bool operator==(const B&) const;
-  };
-  struct D : B {
-bool operator==(const B&) const override; // expected-note {{operator}}
-  };
-  bool cmp_base_derived = D() == D(); // expected-warning {{ambiguous}}
+struct SymmetricConst {
+  bool operator==(const SymmetricConst&) const;
+};
+bool cmp_const = SymmetricConst() == SymmetricConst();
 
-  template struct CRTPBase {
-bool operator==(const T&) const; // expected-note {{operator}} expected-note {{reversed}}
-bool operator!=(const T&) const; // expected-note {{non-reversed}}
-  };
-  struct CRTP : CRTPBase {};
-  bool cmp_crtp = CRTP() == CRTP(); // expected-warning-re {{ambiguous despite there being a unique best viable function{{$}}
-  bool cmp_crtp2 = CRTP() != CRTP(); // expected-warning {{ambiguous despite there being a unique best viable function with non-reversed arguments}}
-
-  // Given a choice between a rewritten and non-rewritten function with the
-  // same parameter types, where the rewritten function is reversed and each
-  // has a better conversion for one of the two arguments, prefer the
-  // non-rewritten one.
-  using UBool = signed char; // ICU uses this.
-  struct ICUBase {
-virtual UBool operator==(const ICUBase&) const;
-UBool operator!=(const ICUBase &arg) const { return !operator==(arg); }
-  };
-  struct ICUDerived : ICUBase {
-UBool operator==(const ICUBase&) const override; // expected-note {{declared here}} expected-note {{ambiguity is between}}
-  };
-  bool cmp_icu = ICUDerived() != ICUDerived(); // expected-warning {{ambiguous}} expected-warning {{'bool', not 'UBool'}}
+struct SymmetricNonConstWithoutConstRef {
+  bool operator==(SymmetricNonConstWithoutConstRef);
+};
+bool cmp_non_const_wo_ref = SymmetricNonConstWithoutConstRef() == SymmetricNonConstWithoutConstRef();
+
+struct B {
+  virtual bool operator==(const B&) const;
+};
+struct D : B {
+  bool operator==(const B&) const override; // expected-note {{operator}}
+};
+bool cmp_base_derived = D() == D(); // expected-warning {{ambiguous}}
+
+// Reversed "3" not used because we find "2".
+// Rewrite != from "3" but warn that "chosen rewritten candidate must return cv-bool".
+using UBool = signed char;
+struct ICUBase {
+  virtual UBool operator==(const ICUBase&) const; // 1.
+  UBool operator!=(const ICUBase &arg) const { return !operator==(arg); } // 2.
+};
+struct ICUDerived : ICUBase {
+  // 3.
+  UBool operator==(const ICUBase&) const override; // expected-note {{declared here}}
+};
+bool cmp_icu = ICUDerived() != ICUDerived(); // expected-warning {{ISO C++20 requires return type of selected 'operator==' function for rewritten '!=' comparison to be 'bool', not 'UBool' (aka 'signed char')}}
+// Accepted by P2468R2.
+// 1
+struct S {
+  bool operator==(const S&) { return true; }
+  bool operator!=(const S&) { return false; }
+};
+bool ts = S{} != S{};
+// 2
+template st

[PATCH] D135356: [Format] Fix crash when hitting eof while lexing JS template string

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks!




Comment at: clang/lib/Format/FormatTokenLexer.cpp:767
 if (Offset[0] == '\\') {
   ++Offset; // Skip the escaped character.
 } else if (Offset + 1 < Lex->getBuffer().end() && Offset[0] == '$' &&

it's a separate concern, but feels like this could also trigger some crashes 
(e.g. an input like "`\")



Comment at: clang/lib/Format/FormatTokenLexer.cpp:772
   StateStack.push(LexerState::NORMAL);
   ++Offset;
+  ++Offset;

nit: `Offset += 2;` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135356

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

[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

[Hans Wennborg via Phabricator via cfe-commits]

hans accepted this revision.
hans 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/D134456/new/

https://reviews.llvm.org/D134456

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

[PATCH] D133647: [clang-format] Parse the else part of `#if 0`

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This seems to have regressed comment alignment on unrelated directives:

  #if 0
  #endif
  
  #if X
  int something_fairly_long;  // Align here please
  #endif  // Should be aligned

These comments were aligned before this patch, but are no longer.
I'm not sure why, maybe setting the taken branch to 1 has some negative 
side-effect when there's no branch 1?
Filed https://github.com/llvm/llvm-project/issues/58188


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133647

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

[PATCH] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134456#3827661 , @aaron.ballman 
wrote:

> Thank you (everyone!) for the discussion on this. To make sure we're all on 
> the same page for where we're at:
>
> 1. The changes in this review are reasonable and once review is finished, 
> we're clear to land it.
> 2. We should file an issue to track the feature request for adding opt 
> remarks for likelihood attribute disagreements.

I filed https://github.com/llvm/llvm-project/issues/58187 for this.

> 3. We should file a bug to improve the PGO documentation 
> (https://clang.llvm.org/docs/UsersManual.html#profile-guided-optimization) to 
> explicitly document our behavior around explicitly-provided user annotations 
> that disagree with PGO (this goes beyond `[[likely]]` and into things like 
> `__builtin_expect`, `inline`, etc).

I filed https://github.com/llvm/llvm-project/issues/58189 for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134456

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

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

tbaeder updated this revision to Diff 465702.
tbaeder added a comment.

Another try, this time without any underlying host type.

There are a few things I'm uncertain about. The tests are obviously not enough, 
especially now that I had to add another `CastFP` opcode to cast floats between 
different semantics.

The general approach seems much saner than before, but I'm sure you'll find 
enough problem with this one as well :)

BTW, I feel like there are a lot of comments on this review now, so if you'd 
like me to split the new version of the patch out into another ticket, I can do 
that of course.


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

https://reviews.llvm.org/D134859

Files:
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Interp/Boolean.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Floating.cpp
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpStack.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/PrimType.h
  clang/lib/AST/Interp/Primitives.h
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -270,3 +270,26 @@
   static_assert((1337 & -1) == 1337, "");
   static_assert((0 & gimme(12)) == 0, "");
 };
+
+namespace floats {
+  constexpr int i = 2;
+  constexpr float f = 1.0f;
+  static_assert(f == 1.0f, "");
+
+  constexpr float f2 = 1u * f;
+  static_assert(f2 == 1.0f, "");
+
+  static_assert(1.0f + 3u == 4, "");
+  static_assert(4.0f / 1.0f == 4, "");
+  static_assert(10.0f * false == 0, "");
+
+  constexpr float floats[] = {1.0f, 2.0f, 3.0f, 4.0f};
+
+  constexpr float m = 5.0f / 0.0f; // ref-error {{must be initialized by a constant expression}} \
+   // ref-note {{division by zero}} \
+   // expected-error {{must be initialized by a constant expression}} \
+   // expected-note {{division by zero}}
+
+  static_assert(~2.0f == 3, ""); // ref-error {{invalid argument type 'float' to unary expression}} \
+ // expected-error {{invalid argument type 'float' to unary expression}}
+};
Index: clang/lib/AST/Interp/Primitives.h
===
--- /dev/null
+++ clang/lib/AST/Interp/Primitives.h
@@ -0,0 +1,36 @@
+//===-- Primitives.h - Types for the constexpr VM ---*- 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
+//
+//===--===//
+//
+// Utilities and helper functions for all primitive types:
+//  - Integral
+//  - Floating
+//  - Boolean
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_INTERP_PRIMITIVES_H
+#define LLVM_CLANG_AST_INTERP_PRIMITIVES_H
+
+#include "clang/AST/ComparisonCategories.h"
+
+namespace clang {
+namespace interp {
+
+/// Helper to compare two comparable types.
+template  ComparisonCategoryResult Compare(const T &X, const T &Y) {
+  if (X < Y)
+return ComparisonCategoryResult::Less;
+  if (X > Y)
+return ComparisonCategoryResult::Greater;
+  return ComparisonCategoryResult::Equal;
+}
+
+} // namespace interp
+} // namespace clang
+
+#endif
Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -13,11 +13,12 @@
 #ifndef LLVM_CLANG_AST_INTERP_TYPE_H
 #define LLVM_CLANG_AST_INTERP_TYPE_H
 
+#include "Boolean.h"
+#include "Floating.h"
+#include "Integral.h"
 #include 
 #include 
 #include 
-#include "Boolean.h"
-#include "Integral.h"
 
 namespace clang {
 namespace interp {
@@ -35,6 +36,7 @@
   PT_Sint64,
   PT_Uint64,
   PT_Bool,
+  PT_Float,
   PT_Ptr,
 };
 
@@ -48,6 +50,7 @@
 template <> struct PrimConv { using T = Integral<32, false>; };
 template <> struct PrimConv { using T = Integral<64, true>; };
 template <> struct PrimConv { using T = Integral<64, false>; };
+template <> struct PrimConv { using T = Floating; };
 template <> struct PrimConv { using T = Boolean; };
 template <> struct PrimConv { using T = Pointer; };
 
@@ -70,6 +73,7 @@
   case PT_Uint32:
   case PT_Sint64:
   case PT_Uint64:
+  case PT_Float:
 return true;
   default:
 return false;
@@ -94,6 +98,7 @@
   TYPE_SWITCH_CASE(PT_Uint32, B)   \
   TYPE_SWITCH_CASE(PT_Sint64, B)

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Floating.h:33-34
+  /// Primitive representing limits.
+  // static constexpr auto Min = std::numeric_limits::min();
+  // static constexpr auto Max = std::numeric_limits::max();
+

This is currently commented out, but I //think// I can get the semantics of the 
`APFloat` and ask its semantics for min/max values.


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

https://reviews.llvm.org/D134859

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

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

tbaeder updated this revision to Diff 465703.

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

https://reviews.llvm.org/D134859

Files:
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Interp/Boolean.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Floating.cpp
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpStack.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/PrimType.h
  clang/lib/AST/Interp/Primitives.h
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -270,3 +270,26 @@
   static_assert((1337 & -1) == 1337, "");
   static_assert((0 & gimme(12)) == 0, "");
 };
+
+namespace floats {
+  constexpr int i = 2;
+  constexpr float f = 1.0f;
+  static_assert(f == 1.0f, "");
+
+  constexpr float f2 = 1u * f;
+  static_assert(f2 == 1.0f, "");
+
+  static_assert(1.0f + 3u == 4, "");
+  static_assert(4.0f / 1.0f == 4, "");
+  static_assert(10.0f * false == 0, "");
+
+  constexpr float floats[] = {1.0f, 2.0f, 3.0f, 4.0f};
+
+  constexpr float m = 5.0f / 0.0f; // ref-error {{must be initialized by a constant expression}} \
+   // ref-note {{division by zero}} \
+   // expected-error {{must be initialized by a constant expression}} \
+   // expected-note {{division by zero}}
+
+  static_assert(~2.0f == 3, ""); // ref-error {{invalid argument type 'float' to unary expression}} \
+ // expected-error {{invalid argument type 'float' to unary expression}}
+};
Index: clang/lib/AST/Interp/Primitives.h
===
--- /dev/null
+++ clang/lib/AST/Interp/Primitives.h
@@ -0,0 +1,36 @@
+//===-- Primitives.h - Types for the constexpr VM ---*- 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
+//
+//===--===//
+//
+// Utilities and helper functions for all primitive types:
+//  - Integral
+//  - Floating
+//  - Boolean
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_INTERP_PRIMITIVES_H
+#define LLVM_CLANG_AST_INTERP_PRIMITIVES_H
+
+#include "clang/AST/ComparisonCategories.h"
+
+namespace clang {
+namespace interp {
+
+/// Helper to compare two comparable types.
+template  ComparisonCategoryResult Compare(const T &X, const T &Y) {
+  if (X < Y)
+return ComparisonCategoryResult::Less;
+  if (X > Y)
+return ComparisonCategoryResult::Greater;
+  return ComparisonCategoryResult::Equal;
+}
+
+} // namespace interp
+} // namespace clang
+
+#endif
Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -13,11 +13,12 @@
 #ifndef LLVM_CLANG_AST_INTERP_TYPE_H
 #define LLVM_CLANG_AST_INTERP_TYPE_H
 
+#include "Boolean.h"
+#include "Floating.h"
+#include "Integral.h"
 #include 
 #include 
 #include 
-#include "Boolean.h"
-#include "Integral.h"
 
 namespace clang {
 namespace interp {
@@ -35,6 +36,7 @@
   PT_Sint64,
   PT_Uint64,
   PT_Bool,
+  PT_Float,
   PT_Ptr,
 };
 
@@ -48,6 +50,7 @@
 template <> struct PrimConv { using T = Integral<32, false>; };
 template <> struct PrimConv { using T = Integral<64, true>; };
 template <> struct PrimConv { using T = Integral<64, false>; };
+template <> struct PrimConv { using T = Floating; };
 template <> struct PrimConv { using T = Boolean; };
 template <> struct PrimConv { using T = Pointer; };
 
@@ -70,6 +73,7 @@
   case PT_Uint32:
   case PT_Sint64:
   case PT_Uint64:
+  case PT_Float:
 return true;
   default:
 return false;
@@ -94,6 +98,7 @@
   TYPE_SWITCH_CASE(PT_Uint32, B)   \
   TYPE_SWITCH_CASE(PT_Sint64, B)   \
   TYPE_SWITCH_CASE(PT_Uint64, B)   \
+  TYPE_SWITCH_CASE(PT_Float, B)\
   TYPE_SWITCH_CASE(PT_Bool, B) \
   TYPE_SWITCH_CASE(PT_Ptr, B)  \
 }  \
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AS

[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods

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

tbaeder added a comment.

@aaron.ballman This one has working precommit CI \o/ (array filler patch is not 
needed for it though)


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

https://reviews.llvm.org/D134699

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

[PATCH] D135314: [clangd] Avoid scanning up to end of file on each comment!

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

BTW it looks like the 8% profile I saw was an outlier, 3-4% seems more typical.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135314

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

[PATCH] D135287: Disallow dereferencing of void* in C++.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 465707.
erichkeane marked an inline comment as done.
erichkeane added a comment.

fix the release note spelling of permanent.


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

https://reviews.llvm.org/D135287

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
  clang/test/SemaCXX/disallow_void_deref.cpp
  clang/test/SemaCXX/reinterpret-cast.cpp

Index: clang/test/SemaCXX/reinterpret-cast.cpp
===
--- clang/test/SemaCXX/reinterpret-cast.cpp
+++ clang/test/SemaCXX/reinterpret-cast.cpp
@@ -214,11 +214,11 @@
   (void)*reinterpret_cast(v_ptr);
 
   // Casting to void pointer
-  (void)*reinterpret_cast(&a); // expected-warning {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(&b); // expected-warning {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(&l); // expected-warning {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(&d); // expected-warning {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(&f); // expected-warning {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&a); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&b); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&l); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&d); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&f); // expected-error {{ISO C++ does not allow}}
 }
 
 void reinterpret_cast_allowlist () {
Index: clang/test/SemaCXX/disallow_void_deref.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/disallow_void_deref.cpp
@@ -0,0 +1,15 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+
+void f(void* p) {
+  (void)*p; // expected-error{{ISO C++ does not allow indirection on operand of type 'void *'}}
+}
+
+template
+concept deref = requires (T& t) {
+  { *t }; // #FAILED_REQ
+};
+
+static_assert(deref);
+// expected-error@-1{{static assertion failed}}
+// expected-note@-2{{because 'void *' does not satisfy 'deref'}}
+// expected-note@#FAILED_REQ{{because '*t' would be invalid: ISO C++ does not allow indirection on operand of type 'void *'}}
Index: clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
+++ clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
@@ -8,7 +8,7 @@
 
 template
 void X0::f(T *t, const U &u) {
-  *t = u; // expected-warning{{indirection on operand of type 'void *'}} expected-error{{not assignable}}
+  *t = u; // expected-error{{indirection on operand of type 'void *'}} expected-error{{not assignable}}
 }
 
 void test_f(X0 xfi, X0 xvi, float *fp, void *vp, int i) {
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14536,7 +14536,10 @@
 //   [...] the expression to which [the unary * operator] is applied shall
 //   be a pointer to an object type, or a pointer to a function type
 LangOptions LO = S.getLangOpts();
-if (LO.CPlusPlus || !(LO.C99 && (IsAfterAmp || S.isUnevaluatedContext(
+if (LO.CPlusPlus)
+  S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer_cpp)
+  << OpTy << Op->getSourceRange();
+else if (!(LO.C99 && (IsAfterAmp || S.isUnevaluatedContext(
   S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer)
   << LO.CPlusPlus << OpTy << Op->getSourceRange();
   }
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6934,6 +6934,14 @@
 def ext_typecheck_indirection_through_void_pointer : ExtWarn<
   "ISO %select{C|C++}0 does not allow indirection on operand of type %1">,
   InGroup>;
+def ext_typecheck_indirection_through_void_pointer_cpp
+: ExtWarn<"ISO C++ does not allow indirection on operand of type %0">,
+  // Note: This uses a different diagnostics group than the C diagnostic
+  // so that projects that have disabled the above will get this diagnostic,
+  // and be aware of the deprecation.
+  InGroup>,
+  DefaultError,
+  SFINAEFailure;
 def warn_indirection_through_null : Warning<
   "indirection of non-volatile null pointer will be deleted, not trap">,
   InGroup;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -86,6 +86,16 @@
   typedef char int8_a16 __attribute__((aligned(16)));
   in

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

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

tbaeder updated this revision to Diff 465708.

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

https://reviews.llvm.org/D134859

Files:
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Interp/Boolean.h
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Floating.cpp
  clang/lib/AST/Interp/Floating.h
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpStack.h
  clang/lib/AST/Interp/Opcodes.td
  clang/lib/AST/Interp/PrimType.h
  clang/lib/AST/Interp/Primitives.h
  clang/test/AST/Interp/literals.cpp

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -270,3 +270,26 @@
   static_assert((1337 & -1) == 1337, "");
   static_assert((0 & gimme(12)) == 0, "");
 };
+
+namespace floats {
+  constexpr int i = 2;
+  constexpr float f = 1.0f;
+  static_assert(f == 1.0f, "");
+
+  constexpr float f2 = 1u * f;
+  static_assert(f2 == 1.0f, "");
+
+  static_assert(1.0f + 3u == 4, "");
+  static_assert(4.0f / 1.0f == 4, "");
+  static_assert(10.0f * false == 0, "");
+
+  constexpr float floats[] = {1.0f, 2.0f, 3.0f, 4.0f};
+
+  constexpr float m = 5.0f / 0.0f; // ref-error {{must be initialized by a constant expression}} \
+   // ref-note {{division by zero}} \
+   // expected-error {{must be initialized by a constant expression}} \
+   // expected-note {{division by zero}}
+
+  static_assert(~2.0f == 3, ""); // ref-error {{invalid argument type 'float' to unary expression}} \
+ // expected-error {{invalid argument type 'float' to unary expression}}
+};
Index: clang/lib/AST/Interp/Primitives.h
===
--- /dev/null
+++ clang/lib/AST/Interp/Primitives.h
@@ -0,0 +1,36 @@
+//===-- Primitives.h - Types for the constexpr VM ---*- 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
+//
+//===--===//
+//
+// Utilities and helper functions for all primitive types:
+//  - Integral
+//  - Floating
+//  - Boolean
+//
+//===--===//
+
+#ifndef LLVM_CLANG_AST_INTERP_PRIMITIVES_H
+#define LLVM_CLANG_AST_INTERP_PRIMITIVES_H
+
+#include "clang/AST/ComparisonCategories.h"
+
+namespace clang {
+namespace interp {
+
+/// Helper to compare two comparable types.
+template  ComparisonCategoryResult Compare(const T &X, const T &Y) {
+  if (X < Y)
+return ComparisonCategoryResult::Less;
+  if (X > Y)
+return ComparisonCategoryResult::Greater;
+  return ComparisonCategoryResult::Equal;
+}
+
+} // namespace interp
+} // namespace clang
+
+#endif
Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -13,11 +13,12 @@
 #ifndef LLVM_CLANG_AST_INTERP_TYPE_H
 #define LLVM_CLANG_AST_INTERP_TYPE_H
 
+#include "Boolean.h"
+#include "Floating.h"
+#include "Integral.h"
 #include 
 #include 
 #include 
-#include "Boolean.h"
-#include "Integral.h"
 
 namespace clang {
 namespace interp {
@@ -35,6 +36,7 @@
   PT_Sint64,
   PT_Uint64,
   PT_Bool,
+  PT_Float,
   PT_Ptr,
 };
 
@@ -48,6 +50,7 @@
 template <> struct PrimConv { using T = Integral<32, false>; };
 template <> struct PrimConv { using T = Integral<64, true>; };
 template <> struct PrimConv { using T = Integral<64, false>; };
+template <> struct PrimConv { using T = Floating; };
 template <> struct PrimConv { using T = Boolean; };
 template <> struct PrimConv { using T = Pointer; };
 
@@ -70,6 +73,7 @@
   case PT_Uint32:
   case PT_Sint64:
   case PT_Uint64:
+  case PT_Float:
 return true;
   default:
 return false;
@@ -94,6 +98,7 @@
   TYPE_SWITCH_CASE(PT_Uint32, B)   \
   TYPE_SWITCH_CASE(PT_Sint64, B)   \
   TYPE_SWITCH_CASE(PT_Uint64, B)   \
+  TYPE_SWITCH_CASE(PT_Float, B)\
   TYPE_SWITCH_CASE(PT_Bool, B) \
   TYPE_SWITCH_CASE(PT_Ptr, B)  \
 }  \
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AS

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaConcept.cpp:614
+
+  static auto UnifyConstraintDepth(Sema &Self,
+  const NamedDecl *Old,

`SemaRef` or `S` is our common nomenclature.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134128

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

[PATCH] D135175: [clang] adds `__is_bounded_array` and `__is_unbounded_array` as builtins

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaExprCXX.cpp:24
 #include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/Type.h"
 #include "clang/AST/TypeLoc.h"

cjdb wrote:
> shafik wrote:
> > Why also `Type.h`?
> The most likely reason is that it's come from D116280, and is actually 
> important for one of the child patches (I'm not sure which off the top of my 
> head). I can investigate and move it to the correct one before merging.
I'd say it isn't particularly important to figure out whichone.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:4910
+if (T->isVariableArrayType())
+  Self.Diag(KeyLoc, diag::err_vla_unsupported)
+  << 1 << tok::kw___is_bounded_array;

This should make this return 'false' in this case, rather than a success?  Not 
sure about this, but wanted to make sure you were thinking about this.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:4915
+if (T->isVariableArrayType())
+  Self.Diag(KeyLoc, diag::err_vla_unsupported)
+  << 1 << tok::kw___is_unbounded_array;

Same here, not clear if this is the logic you meant.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135175

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

[PATCH] D135360: [clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker.

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

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

Add support for functions 'fgetpos', 'fsetpos', 'ftell', 'rewind'.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135360

Files:
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/test/Analysis/stream-errno-note.c
  clang/test/Analysis/stream-errno.c
  clang/test/Analysis/stream-noopen.c

Index: clang/test/Analysis/stream-noopen.c
===
--- clang/test/Analysis/stream-noopen.c
+++ clang/test/Analysis/stream-noopen.c
@@ -77,6 +77,49 @@
   clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
 }
 
+void check_fgetpos(FILE *F) {
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fgetpos(F, &Pos);
+  if (Ret)
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_fsetpos(FILE *F) {
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fsetpos(F, &Pos);
+  if (Ret)
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
+void check_ftell(FILE *F) {
+  errno = 0;
+  long Ret = ftell(F);
+  if (Ret == -1) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  } else {
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+ // expected-warning@-1{{FALSE}}
+clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+  }
+  if (errno) {} // no-warning
+  clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
+  clang_analyzer_eval(ferror(F)); // expected-warning {{UNKNOWN}}
+}
+
 void freadwrite_zerosize(FILE *F) {
   fwrite(WBuf, 1, 0, F);
   clang_analyzer_eval(feof(F)); // expected-warning {{UNKNOWN}}
Index: clang/test/Analysis/stream-errno.c
===
--- clang/test/Analysis/stream-errno.c
+++ clang/test/Analysis/stream-errno.c
@@ -123,6 +123,64 @@
   fclose(F);
 }
 
+void check_fgetpos(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fgetpos(F, &Pos);
+  if (Ret)
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+  if (errno) {} // no-warning
+  fclose(F);
+}
+
+void check_fsetpos(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  errno = 0;
+  fpos_t Pos;
+  int Ret = fsetpos(F, &Pos);
+  if (Ret)
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  else
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+  if (errno) {} // no-warning
+  fclose(F);
+}
+
+void check_ftell(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  errno = 0;
+  long Ret = ftell(F);
+  if (Ret == -1) {
+clang_analyzer_eval(errno != 0); // expected-warning{{TRUE}}
+  } else {
+clang_analyzer_eval(errno == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(Ret >= 0); // expected-warning{{TRUE}}
+  }
+  if (errno) {} // no-warning
+  fclose(F);
+}
+
+void check_rewind(void) {
+  FILE *F = tmpfile();
+  if (!F)
+return;
+  errno = 0;
+  rewind(F);
+  clang_analyzer_eval(errno == 0);
+  // expected-warning@-1{{FALSE}}
+  // expected-warning@-2{{TRUE}}
+  fclose(F);
+}
+
 void check_fileno(void) {
   FILE *F = tmpfile();
   if (!F)
Index: clang/test/Analysis/stream-errno-note.c
===
--- clang/test/Analysis/stream-errno-note.c
+++ clang/test/Analysis/stream-errno-note.c
@@ -98,6 +98,18 @@
   (void)fclose(F);
 }
 
+void check_rewind_errnocheck(void) {
+  FILE *F = tmpfile();
+  // expected-note@+2{{'F' is non-null}}
+  // expected-note@+1{{Taking false branch}}
+  if (!F)
+return;
+  errno = 0;
+  rewind(F); // expected-note{{Function 'rewind' indicates failure only by setting of 'errno'}}
+  fclose(F); // expected-warning{{Value of 'errno' wa

[PATCH] D135361: [clang][Interp] Implement bitwise Or operations

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

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135361

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Integral.h
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -271,6 +271,15 @@
   static_assert((0 & gimme(12)) == 0, "");
 };
 
+namespace bitOr {
+  static_assert((10 | 1) == 11, "");
+  static_assert((10 | 10) == 10, "");
+
+  static_assert((1337 | -1) == -1, "");
+  static_assert((0 | gimme(12)) == 12, "");
+  static_assert((12 | true) == 13, "");
+};
+
 namespace floats {
   constexpr int i = 2;
   constexpr float f = 1.0f;
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -432,6 +432,7 @@
   let HasGroup = 1;
 }
 def BitAnd : IntegerOpcode;
+def BitOr : IntegerOpcode;
 def Div : Opcode {
   let Types = [NumberTypeClass];
   let HasGroup = 1;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -175,6 +175,23 @@
   return false;
 }
 
+/// 1) Pops the RHS from the stack.
+/// 2) Pops the LHS from the stack.
+/// 3) Pushes 'LHS | RHS' on the stack
+template ::T>
+bool BitOr(InterpState &S, CodePtr OpPC) {
+  const T &RHS = S.Stk.pop();
+  const T &LHS = S.Stk.pop();
+
+  unsigned Bits = RHS.bitWidth();
+  T Result;
+  if (!T::bitOr(LHS, RHS, Bits, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+  return false;
+}
+
 /// 1) Pops the RHS from the stack.
 /// 2) Pops the LHS from the stack.
 /// 3) Pushes 'LHS % RHS' on the stack (the remainder of dividing LHS by RHS).
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -217,6 +217,11 @@
 return false;
   }
 
+  static bool bitOr(Integral A, Integral B, unsigned OpBits, Integral *R) {
+*R = Integral(A.V | B.V);
+return false;
+  }
+
   static bool neg(Integral A, Integral *R) {
 *R = -A;
 return false;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -273,6 +273,7 @@
 case BO_And:
   return Discard(this->emitBitAnd(*T, BO));
 case BO_Or:
+  return Discard(this->emitBitOr(*T, BO));
 case BO_LAnd:
 case BO_LOr:
 default:


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -271,6 +271,15 @@
   static_assert((0 & gimme(12)) == 0, "");
 };
 
+namespace bitOr {
+  static_assert((10 | 1) == 11, "");
+  static_assert((10 | 10) == 10, "");
+
+  static_assert((1337 | -1) == -1, "");
+  static_assert((0 | gimme(12)) == 12, "");
+  static_assert((12 | true) == 13, "");
+};
+
 namespace floats {
   constexpr int i = 2;
   constexpr float f = 1.0f;
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -432,6 +432,7 @@
   let HasGroup = 1;
 }
 def BitAnd : IntegerOpcode;
+def BitOr : IntegerOpcode;
 def Div : Opcode {
   let Types = [NumberTypeClass];
   let HasGroup = 1;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -175,6 +175,23 @@
   return false;
 }
 
+/// 1) Pops the RHS from the stack.
+/// 2) Pops the LHS from the stack.
+/// 3) Pushes 'LHS | RHS' on the stack
+template ::T>
+bool BitOr(InterpState &S, CodePtr OpPC) {
+  const T &RHS = S.Stk.pop();
+  const T &LHS = S.Stk.pop();
+
+  unsigned Bits = RHS.bitWidth();
+  T Result;
+  if (!T::bitOr(LHS, RHS, Bits, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+  return false;
+}
+
 /// 1) Pops the RHS from the stack.
 /// 2) Pops the LHS from the stack.
 /// 3) Pushes 'LHS % RHS' on the stack (the remainder of dividing LHS by RHS).
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -217,6 +217,11 @@
 return false;
   }
 
+  static bool bitOr(Integral A, Inte

[PATCH] D135177: [clang] adds `__is_scoped_enum`, `__is_nullptr`, and `__is_referenceable`

[Erich Keane via Phabricator via cfe-commits]

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

LGTM... Please do the re-ordering separately here, unless it was necessary here.




Comment at: clang/lib/Parse/ParseExpr.cpp:1071
   REVERTIBLE_TYPE_TRAIT(__is_base_of);
+  REVERTIBLE_TYPE_TRAIT(__is_bounded_array);
   REVERTIBLE_TYPE_TRAIT(__is_class);

Can you just put this in the right place in the other commit?



Comment at: clang/lib/Parse/ParseExpr.cpp:1078
   REVERTIBLE_TYPE_TRAIT(__is_convertible_to);
+  REVERTIBLE_TYPE_TRAIT(__is_convertible);
   REVERTIBLE_TYPE_TRAIT(__is_destructible);

Why are the rest of these re-ordered?  Feel free to do an NFC re-ordering 
these, but please dont do them here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135177

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

[PATCH] D135238: [clang] adds copy-constructible type-trait builtins

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Please make sure these are going to be OK with libc++ here.  "Triviality" is a 
bit of a hard nut, the standards have re-defined what they mean quite a few 
times, so this ends up being pretty worthless if it doesn't match the 'version' 
of this check that the library is trying to look at.




Comment at: clang/include/clang/Basic/TokenKinds.def:528
+TYPE_TRAIT_1(__is_nothrow_copy_constructible, IsNothrowCopyConstructible, 
KEYCXX)
+TYPE_TRAIT_1(__is_trivially_copy_constructible, IsTriviallyCopyConstructible, 
KEYCXX)
 TYPE_TRAIT_2(__reference_binds_to_temporary, ReferenceBindsToTemporary, KEYCXX)

So this one is a whole 'thing'.  The Clang definition of 'trivially copy 
constructible' is a few DRs behind.  We should probably discuss this with 
libcxx to make sure use of this wouldn't be broken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135238

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

[PATCH] D135239: [clang] adds copy-assignable type-trait builtins

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Same comment as the previous patch on triviality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135239

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

[PATCH] D135240: [clang] adds move-constructible type-trait builtins

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Same comments on triviality here unfortunately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135240

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

[PATCH] D135338: [clang] adds move-assignable type-trait builtins

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Once again, concerns about triviality.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135338

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

[PATCH] D135362: [clang] Make variables of undeduced types to have dependent alignment

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX created this revision.
ArcsinX added reviewers: aaron.ballman, rsmith, mizvekov.
Herald added a project: All.
ArcsinX requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Without this patch `VarDecl::hasDependent()` checks only undeduced auto types, 
so can give false negatives result for other undeduced types.
This lead to crashes in sequence `!VarDecl::hasDepentent()` => `getDeclAlign()`.

It seems this problem appeared since D105380 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135362

Files:
  clang/lib/AST/Decl.cpp
  clang/test/Sema/tls_alignment.cpp


Index: clang/test/Sema/tls_alignment.cpp
===
--- clang/test/Sema/tls_alignment.cpp
+++ clang/test/Sema/tls_alignment.cpp
@@ -1,6 +1,6 @@
 // TLS variable cannot be aligned to more than 32 bytes on PS4.
 
-// RUN: %clang_cc1 -triple x86_64-scei-ps4 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 -std=c++17 -fsyntax-only -verify %s
 
 
 // A non-aligned type.
@@ -18,6 +18,12 @@
 int some_aligned_data[12] __attribute__(( aligned(64) )); // 48 bytes of 
stuff, aligned to 64.
 };
 
+// A templated type
+template 
+struct templated_struct {};
+// expected-note@-1{{candidate template ignored: couldn't infer template 
argument ''}}
+// expected-note@-2{{candidate function template not viable: requires 1 
argument, but 0 were provided}}
+
 // A typedef of the aligned struct.
 typedef aligned_struct another_aligned_struct;
 
@@ -42,6 +48,9 @@
 // Variable aligned because of typedef, second case.
 __thread yet_another_aligned_structbar5; // expected-error{{alignment 
(64) of thread-local variable}}
 
+// No crash for undeduced type.
+__thread templated_struct  bar6; // expected-error{{no viable 
constructor or deduction guide for deduction of template arguments of 
'templated_struct'}}
+
 int baz ()
 {
 return foo.some_data[0] + bar.some_data[1] + bar2.some_data[2] +
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2581,7 +2581,7 @@
 
 bool VarDecl::hasDependentAlignment() const {
   QualType T = getType();
-  return T->isDependentType() || T->isUndeducedAutoType() ||
+  return T->isDependentType() || T->isUndeducedType() ||
  llvm::any_of(specific_attrs(), [](const AlignedAttr *AA) 
{
return AA->isAlignmentDependent();
  });


Index: clang/test/Sema/tls_alignment.cpp
===
--- clang/test/Sema/tls_alignment.cpp
+++ clang/test/Sema/tls_alignment.cpp
@@ -1,6 +1,6 @@
 // TLS variable cannot be aligned to more than 32 bytes on PS4.
 
-// RUN: %clang_cc1 -triple x86_64-scei-ps4 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-scei-ps4 -std=c++17 -fsyntax-only -verify %s
 
 
 // A non-aligned type.
@@ -18,6 +18,12 @@
 int some_aligned_data[12] __attribute__(( aligned(64) )); // 48 bytes of stuff, aligned to 64.
 };
 
+// A templated type
+template 
+struct templated_struct {};
+// expected-note@-1{{candidate template ignored: couldn't infer template argument ''}}
+// expected-note@-2{{candidate function template not viable: requires 1 argument, but 0 were provided}}
+
 // A typedef of the aligned struct.
 typedef aligned_struct another_aligned_struct;
 
@@ -42,6 +48,9 @@
 // Variable aligned because of typedef, second case.
 __thread yet_another_aligned_structbar5; // expected-error{{alignment (64) of thread-local variable}}
 
+// No crash for undeduced type.
+__thread templated_struct  bar6; // expected-error{{no viable constructor or deduction guide for deduction of template arguments of 'templated_struct'}}
+
 int baz ()
 {
 return foo.some_data[0] + bar.some_data[1] + bar2.some_data[2] +
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -2581,7 +2581,7 @@
 
 bool VarDecl::hasDependentAlignment() const {
   QualType T = getType();
-  return T->isDependentType() || T->isUndeducedAutoType() ||
+  return T->isDependentType() || T->isUndeducedType() ||
  llvm::any_of(specific_attrs(), [](const AlignedAttr *AA) {
return AA->isAlignmentDependent();
  });
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135339: [clang] makes `__is_destructible` KEYCXX instead of KEYMS

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

This doesn't cause us to lose this in Microsoft C mode, does it?  Otherwise, 
LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135339

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

[PATCH] D135360: [clang][analyzer] Add some more functions to StreamChecker and StdLibraryFunctionsChecker.

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

balazske added a reviewer: martong.
balazske added a comment.
Herald added a subscriber: rnkovacs.

I found some anomalies during development:

- If the checker **StdCLibraryFunctions** is added as dependency for 
**alpha.unix.Stream** in //checkers.td// I get some "unexplainable" test 
failures.
- In the comments section at CERT ERR30-C 

 is pointed out that function `ftell` can change the value of `errno` if it is 
successful. This is the "normal" expected behavior of all standard functions 
unless the documentation tells other. But this place (search for ftell) 
 tells that `ftell` should 
not change `errno` if successful. The persons commented at the CERT rule 
probably checked only one or more of the C standards. But there should be no 
conflict between POSIX and C ("Any conflict between the requirements described 
here and the ISO C standard is unintentional."). It would be unconventional if 
different check rules are needed (and add of a global analyzer option for POSIX 
or C mode).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135360

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

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thanks!




Comment at: clang/lib/Tooling/Inclusions/STL/CMakeLists.txt:2
+add_clang_library(clangToolingInclusionsSTL
+  StandardLibrary.cpp
+

kadircet wrote:
> sammccall wrote:
> > This means the implementation files and the header files have a different 
> > directory structure, which may be confusing to people trying to work out 
> > which library to link against based on the headers they included.
> > 
> > On the other hand, I think the cascading effect of dependencies => 
> > libraries => directory structure => header structure is pretty unfortunate 
> > leaking of llvm's sad cmake structure. Up to you
> > 
> > 
> right, i was also torn between moving the headers around vs not. but i 
> finally managed to convince myself that the implementation being in a 
> different subdirectory is actually an unfortunate detail about the way LLVM 
> is build (I didn't want to have PARTIAL_SOURCES_INTENDED, either) and 
> shouldn't matter for the applications that want to use it.
FWIW, I'd move the header.

FWIW, I also don't think llvm's cmake setup is particularly unfortunate here 
either. It makes it easy to see where library boundaries are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135245

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

[PATCH] D135245: [clang][Tooling] Move STL recognizer to its own library

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(Alternatively, it looks like the only client is clangd, so maybe the file 
could just live in there instead of being in a dedicated library with a single 
client.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135245

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

[PATCH] D132855: [OpenMP] Extend the lit test for uses_allocators in target region

[Johannes Doerfert via Phabricator via cfe-commits]

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

LG




Comment at: clang/test/OpenMP/target_map_codegen_10.cpp:19
+// RUN: %clang_cc1 -no-opaque-pointers -DCK11 -fopenmp 
-fopenmp-targets=i386-pc-linux-gnu -x c++ -std=c++11 -triple 
i386-unknown-unknown -emit-pch -o %t %s
+// RUN: %clang_cc1 -no-opaque-pointers -fopenmp 
-fopenmp-targets=i386-pc-linux-gnu -x c++ -triple i386-unknown-unknown 
-std=c++11 -include-pch %t -verify %s -emit-llvm -o - | FileCheck 
-allow-deprecated-dag-overlap  %s  --check-prefixes CK11,CK11_5
 

animeshk-amd wrote:
> jdoerfert wrote:
> > What does this test?
> This test was originally testing the respective directives only for the 
> OpenMP version 4.5. I have updated new RUN lines for the default version(5.0 
> as of now) because there were differences in the generated IR in case of the 
> default version.
I see. Makes sense.



Comment at: clang/test/OpenMP/target_uses_allocators.c:42
+  #pragma omp target uses_allocators(omp_thread_mem_alloc) 
allocate(omp_thread_mem_alloc: x) firstprivate(x) // expected-warning 
{{allocator with the 'thread' trait access has unspecified behavior on 'target' 
directive}}
+  {}
 }

animeshk-amd wrote:
> jdoerfert wrote:
> > This should go into the _messages test case
> The code needs to be tested for the generated IR(but as a side effect it also 
> generates a warning), that's why I had to put the expected-warning clause. 
> Since I had to write CHECK lines, I didn't put this in the _messages test. 
> That said, should I still put this into the _messages test along with the 
> required CHECK lines?
I am not really sure about the warning to begin with but I get your point about 
placement.
I'm ok with this for now and we revisit this as the next OpenMP TR is going to 
clarify the allocate traits anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132855

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

[PATCH] D135300: [PowerPC] Fix types for vcipher builtins.

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision as: amyk.
amyk 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/D135300/new/

https://reviews.llvm.org/D135300

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

[PATCH] D135287: Disallow dereferencing of void* in C++.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D135287#3837617 , @erichkeane 
wrote:

> In D135287#3837584 , @jrtc27 wrote:
>
>> What about `__typeof__(*p)`?
>
> Yes, that would also be an error in C++, as it is on all other compilers.

This actually relates to a use case I brought up in the RFC -- in an 
unevaluated context where the expression is only used to determine a type, it 
doesn't seem particularly harmful to allow the dereference. There might be 
generic programming cases where this comes up for people who exclusively use 
Clang, but I'm not 100% certain.




Comment at: clang/docs/ReleaseNotes.rst:89-98
+- Clang now diagnoses a Warning-As-Default-Error on indirection of void* in C++
+  mode for ISO C++, GCC, ICC, and MSVC Compatibility. This is also now a SFINAE
+  error so constraint checking and SFINAE checking can be compatible with other
+  compilers. It is likely that this will be made a permanent error in Clang 17.
+
+  .. code-block:: c
+

Wordsmithing



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6934-6935
   "indirection requires pointer operand (%0 invalid)">;
 def ext_typecheck_indirection_through_void_pointer : ExtWarn<
   "ISO %select{C|C++}0 does not allow indirection on operand of type %1">,
   InGroup>;

We no longer use this diagnostic in C++, so might as well simplify it.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6939-6942
+  // Note: This uses a different diagnostics group than the C diagnostic
+  // so that projects that have disabled the above will get this 
diagnostic,
+  // and be aware of the deprecation.
+  InGroup>,

Based on the code search for people using that diagnostic flag, I don't think 
we need to do this -- it seems like only a very small number of projects 
disable that warning (at least from a code search on sourcegraph). We still 
need a separate diagnostic (because of the `DefaultError`, but I think we can 
re-use the old diagnostic group. WDYT?



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6942-6944
+  InGroup>,
+  DefaultError,
+  SFINAEFailure;





Comment at: clang/test/SemaCXX/disallow_void_deref.cpp:1
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+

You should also add a RUN line showing the behavior when the diagnostic is not 
an error and is disabled entirely.


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

https://reviews.llvm.org/D135287

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

[PATCH] D135238: [clang] adds copy-constructible type-trait builtins

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

TBH I don't think adding these builtins is worth the extra maintenance cost. 
libc++'s implementation is already really simple, and actually //uses// 
`__is_constructible`, contrary to the statement in the summary. This is the 
whole implementation currently:

  template 
  struct _LIBCPP_TEMPLATE_VIS is_copy_constructible
  : public integral_constant<
bool,
__is_constructible(_Tp, __add_lvalue_reference_t::type>)> {};
  
  #if _LIBCPP_STD_VER > 14
  template 
  inline constexpr bool is_copy_constructible_v = 
is_copy_constructible<_Tp>::value;
  #endif

I don't think adding an extra `#if __has_builtin(...)` is worth it in this 
case, since we already use builtins for most of it. IMO the effort would be 
better spent adding a builtin for `add_const`; that would probably make the 
implementation about as efficient as adding a builtin specifically for 
`is_copy_constructible`. It would effectively just be `__is_constructible(_Tp, 
__add_lvalue_reference(__add_const(_Tp)))`. The `trivially` and `nothrow` 
versions look very similar, just with `__is_trivially_constructible` and 
`__is_nothrow_constructible` respectively.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135238

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

[clang] 364003e - [clang][C++20] Note github issue in the FIXME matching requires clause.

[Utkarsh Saxena via cfe-commits]

Author: Utkarsh Saxena
Date: 2022-10-06T16:00:54+02:00
New Revision: 364003e2da7f233725d6200ff469f80cc65b17c5

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

LOG: [clang][C++20] Note github issue in the FIXME matching requires clause.

Added: 


Modified: 
clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp

Removed: 




diff  --git 
a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp 
b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
index e5b271cb9872..5c6804eb7726 100644
--- a/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
+++ b/clang/test/CXX/over/over.match/over.match.funcs/over.match.oper/p3-2a.cpp
@@ -302,7 +302,7 @@ bool c = 0 == C(); // Rewrite not possible. expected-error 
{{invalid operands to
 } // templated
 } // using_decls
 
-// FIXME: Match requires clause.
+// FIXME(GH58185): Match requires clause.
 namespace match_requires_clause {
 template
 struct A {



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

[PATCH] D135362: [clang] Make variables of undeduced types to have dependent alignment

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov accepted this revision.
mizvekov 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/D135362/new/

https://reviews.llvm.org/D135362

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

[PATCH] D135045: [Frontend] Recognize environment variable SOURCE_DATE_EPOCH

[Ed Maste via Phabricator via cfe-commits]

emaste added a comment.

> The time_t value may be negative

True, but we do not need to allow a negative `SOURCE_DATE_EPOCH` though. The 
spec implies that it must be nonnegative, although doesn't say so explicitly. 
https://reproducible-builds.org/specs/source-date-epoch/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135045

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

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added a comment.

> That change might be problematic for content addressing storages. E.g. 
> clang/test/Driver/cl-pch-showincludes.cpp started to fail in my setup as 
> clang/test/Driver/header{0,1,3,4}.h are all identical and can be symlinked

What is the failure you're seeing?  I would expect this change to make clang 
more consistent about symlinks, because it preserves which path was originally 
accessed.  But maybe there's an edge case somewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135220

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

[PATCH] D135287: Disallow dereferencing of void* in C++.

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6939-6942
+  // Note: This uses a different diagnostics group than the C diagnostic
+  // so that projects that have disabled the above will get this 
diagnostic,
+  // and be aware of the deprecation.
+  InGroup>,

aaron.ballman wrote:
> Based on the code search for people using that diagnostic flag, I don't think 
> we need to do this -- it seems like only a very small number of projects 
> disable that warning (at least from a code search on sourcegraph). We still 
> need a separate diagnostic (because of the `DefaultError`, but I think we can 
> re-use the old diagnostic group. WDYT?
SGTM.  I'm not attached to it.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6942-6944
+  InGroup>,
+  DefaultError,
+  SFINAEFailure;

aaron.ballman wrote:
> 
This is actually the result of clang-format!  I'll undo it.


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

https://reviews.llvm.org/D135287

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

[clang] f717050 - Silence a duplicate diagnostic about K&R C function definitions

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-10-06T10:08:23-04:00
New Revision: f7170500cf10ea9d685eb655c76a7c393fb18708

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

LOG: Silence a duplicate diagnostic about K&R C function definitions

We would issue the same diagnostic twice in the case that the K&R C
function definition is preceded by a static declaration of the function
with a prototype.

Fixes #58181

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaDecl.cpp
clang/test/Sema/warn-deprecated-non-prototype.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1652c90337f6e..a1aea7e2753d8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -231,6 +231,10 @@ Improvements to Clang's diagnostics
   be selected.
 - Add a fix-it hint for the ``-Wdefaulted-function-deleted`` warning to
   explicitly delete the function.
+- Fixed an accidental duplicate diagnostic involving the declaration of a
+  function definition without a prototype which is preceded by a static
+  declaration of the function with a prototype. Fixes
+  `Issue 58181 `_.
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 8e7a989219da6..60b8d1c3c59c7 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -14657,6 +14657,21 @@ void Sema::ActOnFinishInlineFunctionDef(FunctionDecl 
*D) {
   Consumer.HandleInlineFunctionDefinition(D);
 }
 
+static bool FindPossiblePrototype(const FunctionDecl *FD,
+  const FunctionDecl *&PossiblePrototype) {
+  for (const FunctionDecl *Prev = FD->getPreviousDecl(); Prev;
+   Prev = Prev->getPreviousDecl()) {
+// Ignore any declarations that occur in function or method
+// scope, because they aren't visible from the header.
+if (Prev->getLexicalDeclContext()->isFunctionOrMethod())
+  continue;
+
+PossiblePrototype = Prev;
+return Prev->getType()->isFunctionProtoType();
+  }
+  return false;
+}
+
 static bool
 ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
 const FunctionDecl *&PossiblePrototype) {
@@ -14703,16 +14718,9 @@ ShouldWarnAboutMissingPrototype(const FunctionDecl *FD,
   if (!FD->isExternallyVisible())
 return false;
 
-  for (const FunctionDecl *Prev = FD->getPreviousDecl();
-   Prev; Prev = Prev->getPreviousDecl()) {
-// Ignore any declarations that occur in function or method
-// scope, because they aren't visible from the header.
-if (Prev->getLexicalDeclContext()->isFunctionOrMethod())
-  continue;
-
-PossiblePrototype = Prev;
-return Prev->getType()->isFunctionNoProtoType();
-  }
+  // If we were able to find a potential prototype, don't warn.
+  if (FindPossiblePrototype(FD, PossiblePrototype))
+return false;
 
   return true;
 }
@@ -15280,6 +15288,12 @@ Decl *Sema::ActOnFinishFunctionBody(Decl *dcl, Stmt 
*Body,
 }
   }
 
+  // We might not have found a prototype because we didn't wish to warn on
+  // the lack of a missing prototype. Try again without the checks for
+  // whether we want to warn on the missing prototype.
+  if (!PossiblePrototype)
+(void)FindPossiblePrototype(FD, PossiblePrototype);
+
   // If the function being defined does not have a prototype, then we may
   // need to diagnose it as changing behavior in C2x because we now know
   // whether the function accepts arguments or not. This only handles the

diff  --git a/clang/test/Sema/warn-deprecated-non-prototype.c 
b/clang/test/Sema/warn-deprecated-non-prototype.c
index 351745e49069f..47994934baed6 100644
--- a/clang/test/Sema/warn-deprecated-non-prototype.c
+++ b/clang/test/Sema/warn-deprecated-non-prototype.c
@@ -105,3 +105,14 @@ void calls(void) {
   func(1, 2); // OK
   func(1, 2, 3); // both-warning {{too many arguments in call to 'func'}}
 }
+
+// Issue 58181 -- we would issue the warning about the function without a
+// prototype twice when the function was declared static in the following
+// example.
+static int GH58181(int x, int y);
+static int GH58181(x, y) // both-warning {{a function definition without a 
prototype is deprecated in all versions of C and is not supported in C2x}}
+int x;
+int y;
+{
+return x + y;
+}



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

[PATCH] D134749: [clang][Interp] Implement Div opcode

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a simple refactoring (feel free to apply it when landing or do 
it post-commit as an NFC change).




Comment at: clang/lib/AST/Interp/Interp.h:203-211
+  if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
+APSInt LHSInt = LHS.toAPSInt();
+SmallString<32> Trunc;
+(-LHSInt.extend(LHSInt.getBitWidth() + 1)).toString(Trunc, 10);
+const SourceInfo &Loc = S.Current->getSource(OpPC);
+const Expr *E = S.Current->getExpr(OpPC);
+S.CCEDiag(Loc, diag::note_constexpr_overflow) << Trunc << E->getType();

We should factor this out into a helper function so we don't have duplication 
between Div and Rem. Same for the above block checking for division by zero.


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

https://reviews.llvm.org/D134749

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

[PATCH] D135287: Disallow dereferencing of void* in C++.

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 465725.
erichkeane marked 2 inline comments as done.
erichkeane added a comment.

Fix based on Aaron's comments.


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

https://reviews.llvm.org/D135287

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaExpr.cpp
  clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
  clang/test/SemaCXX/disallow_void_deref.cpp
  clang/test/SemaCXX/reinterpret-cast.cpp

Index: clang/test/SemaCXX/reinterpret-cast.cpp
===
--- clang/test/SemaCXX/reinterpret-cast.cpp
+++ clang/test/SemaCXX/reinterpret-cast.cpp
@@ -214,11 +214,11 @@
   (void)*reinterpret_cast(v_ptr);
 
   // Casting to void pointer
-  (void)*reinterpret_cast(&a); // expected-warning {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(&b); // expected-warning {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(&l); // expected-warning {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(&d); // expected-warning {{ISO C++ does not allow}}
-  (void)*reinterpret_cast(&f); // expected-warning {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&a); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&b); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&l); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&d); // expected-error {{ISO C++ does not allow}}
+  (void)*reinterpret_cast(&f); // expected-error {{ISO C++ does not allow}}
 }
 
 void reinterpret_cast_allowlist () {
Index: clang/test/SemaCXX/disallow_void_deref.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/disallow_void_deref.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=enabled,sfinae -std=c++20 %s
+// RUN: %clang_cc1 -fsyntax-only -verify=sfinae -std=c++20 -Wno-void-ptr-dereference %s
+
+void f(void* p) {
+  (void)*p; // enabled-error{{ISO C++ does not allow indirection on operand of type 'void *'}}
+}
+
+template
+concept deref = requires (T& t) {
+  { *t }; // #FAILED_REQ
+};
+
+static_assert(deref);
+// sfinae-error@-1{{static assertion failed}}
+// sfinae-note@-2{{because 'void *' does not satisfy 'deref'}}
+// sfinae-note@#FAILED_REQ{{because '*t' would be invalid: ISO C++ does not allow indirection on operand of type 'void *'}}
Index: clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
+++ clang/test/CXX/temp/temp.decls/temp.class/temp.mem.func/p1inst.cpp
@@ -8,7 +8,7 @@
 
 template
 void X0::f(T *t, const U &u) {
-  *t = u; // expected-warning{{indirection on operand of type 'void *'}} expected-error{{not assignable}}
+  *t = u; // expected-error{{indirection on operand of type 'void *'}} expected-error{{not assignable}}
 }
 
 void test_f(X0 xfi, X0 xvi, float *fp, void *vp, int i) {
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14536,9 +14536,12 @@
 //   [...] the expression to which [the unary * operator] is applied shall
 //   be a pointer to an object type, or a pointer to a function type
 LangOptions LO = S.getLangOpts();
-if (LO.CPlusPlus || !(LO.C99 && (IsAfterAmp || S.isUnevaluatedContext(
+if (LO.CPlusPlus)
+  S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer_cpp)
+  << OpTy << Op->getSourceRange();
+else if (!(LO.C99 && (IsAfterAmp || S.isUnevaluatedContext(
   S.Diag(OpLoc, diag::ext_typecheck_indirection_through_void_pointer)
-  << LO.CPlusPlus << OpTy << Op->getSourceRange();
+  << OpTy << Op->getSourceRange();
   }
 
   // Dereferences are usually l-values...
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6932,8 +6932,11 @@
 def err_typecheck_indirection_requires_pointer : Error<
   "indirection requires pointer operand (%0 invalid)">;
 def ext_typecheck_indirection_through_void_pointer : ExtWarn<
-  "ISO %select{C|C++}0 does not allow indirection on operand of type %1">,
-  InGroup>;
+  "ISO C does not allow indirection on operand of type %0">,
+  InGroup;
+def ext_typecheck_indirection_through_void_pointer_cpp
+: ExtWarn<"ISO C++ does not allow indirection on operand of type %0">,
+  InGroup, DefaultError, SFINAEFailure;
 def warn_indirection_through_null : Warning<
   "indirection of non-volatile null pointer will be deleted, not trap">,
   InGroup;
Index: clang/include/clang/Basic/DiagnosticGroups.td
==

[PATCH] D135305: [Clang] Fix using LTO with the new driver in RDC-mode

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135305

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

[PATCH] D134788: [ARM64EC][clang-cl] Add arm64EC test; NFC

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added a comment.

This (and it's followup?) has been landed, right? Please close the revision if 
so.


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

https://reviews.llvm.org/D134788

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

[PATCH] D135306: [CUDA] Add support for CUDA-11.8 and sm_{87,89,90} GPUs.

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135306

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

[clang] 8788374 - [ASTMatchers][NFC] Fix wrong code ending command in documentation comments

[via cfe-commits]

Author: oToToT
Date: 2022-10-06T22:32:15+08:00
New Revision: 87883740ebb3f733985d47e7b3d6b0a4e6670c78

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

LOG: [ASTMatchers][NFC] Fix wrong code ending command in documentation comments

Added: 


Modified: 
clang/include/clang/ASTMatchers/ASTMatchers.h

Removed: 




diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 5bcf1d3896e89..dfea432c16adb 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -3954,14 +3954,14 @@ AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
 ///
 /// \code
 /// auto x = int(3);
-/// \code
+/// \endcode
 /// cxxTemporaryObjectExpr(hasTypeLoc(loc(asString("int"
 ///   matches int(3)
 ///
 /// \code
 /// struct Foo { Foo(int, int); };
 /// auto x = Foo(1, 2);
-/// \code
+/// \endcode
 /// cxxFunctionalCastExpr(hasTypeLoc(loc(asString("struct Foo"
 ///   matches Foo(1, 2)
 ///
@@ -4175,7 +4175,7 @@ AST_MATCHER_P(DeclRefExpr, to, internal::Matcher,
 ///   namespace a { class X{}; }
 ///   using a::X;
 ///   X x;
-/// \code
+/// \endcode
 /// typeLoc(loc(usingType(throughUsingDecl(anything()
 ///   matches \c X
 ///



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

[clang] ac135f9 - [Clang] Fix using LTO with the new driver in RDC-mode

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-10-06T09:36:09-05:00
New Revision: ac135f9ee574e7451088926c667d93d51a3d6940

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

LOG: [Clang] Fix using LTO with the new driver in RDC-mode

The new driver supports LTO for RDC-mode compilations. However, this was
not correctly handled for non-LTO compilations. HIP can handle this as
it is fed to `lld` which will perform the LTO itself. CUDA however would
require every work which is wholly useless in non-RDC mode so it should
report an error.

Reviewed By: yaxunl

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/cuda-bindings.cu
clang/test/Driver/cuda-phases.cu
clang/test/Driver/hip-binding.hip
clang/test/Driver/hip-phases.hip

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index be62ce5de4b3..21dc180d26c3 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4375,7 +4375,9 @@ Action *Driver::BuildOffloadingActions(Compilation &C,
 
 // Compiling HIP in non-RDC mode requires linking each action individually.
 for (Action *&A : DeviceActions) {
-  if (A->getType() != types::TY_Object || Kind != Action::OFK_HIP ||
+  if ((A->getType() != types::TY_Object &&
+   A->getType() != types::TY_LTO_BC) ||
+  Kind != Action::OFK_HIP ||
   Args.hasFlag(options::OPT_fgpu_rdc, options::OPT_fno_gpu_rdc, false))
 continue;
   ActionList LinkerInput = {A};

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index 4f880c27b2f1..1a3ee0964835 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -4842,7 +4842,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
   CmdArgs.push_back("-emit-llvm-uselists");
 
 if (IsUsingLTO) {
-  // Only AMDGPU supports device-side LTO.
   if (IsDeviceOffloadAction && !JA.isDeviceOffloading(Action::OFK_OpenMP) 
&&
   !Args.hasFlag(options::OPT_offload_new_driver,
 options::OPT_no_offload_new_driver, false) &&
@@ -4852,6 +4851,12 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
options::OPT_foffload_lto_EQ)
->getAsString(Args)
 << Triple.getTriple();
+  } else if (Triple.isNVPTX() && !IsRDCMode) {
+D.Diag(diag::err_drv_unsupported_opt_for_language_mode)
+<< Args.getLastArg(options::OPT_foffload_lto,
+   options::OPT_foffload_lto_EQ)
+   ->getAsString(Args)
+<< "-fno-gpu-rdc";
   } else {
 assert(LTOMode == LTOK_Full || LTOMode == LTOK_Thin);
 CmdArgs.push_back(Args.MakeArgString(

diff  --git a/clang/test/Driver/cuda-bindings.cu 
b/clang/test/Driver/cuda-bindings.cu
index 78c9bb975c6f..7f2d60421cc3 100644
--- a/clang/test/Driver/cuda-bindings.cu
+++ b/clang/test/Driver/cuda-bindings.cu
@@ -234,3 +234,11 @@
 // SAVE-TEMPS: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-target-cpu" 
"sm_52"
 // SAVE-TEMPS: "-cc1" "-triple" "nvptx64-nvidia-cuda"{{.*}}"-target-cpu" 
"sm_70"
 // SAVE-TEMPS: "-cc1" "-triple" "powerpc64le-ibm-linux-gnu"
+
+//
+// Check to ensure that we cannot use '-foffload' when not operating in 
RDC-mode.
+//
+// RUN: %clang -### -target powerpc64le-ibm-linux-gnu -fno-gpu-rdc 
--offload-new-driver \
+// RUN:-foffload-lto --offload-arch=sm_70 --offload-arch=sm_52 -c %s 
2>&1 \
+// RUN: | FileCheck -check-prefix=LTO-NO-RDC %s
+// LTO-NO-RDC: error: unsupported option '-foffload-lto' for language mode 
'-fno-gpu-rdc'

diff  --git a/clang/test/Driver/cuda-phases.cu 
b/clang/test/Driver/cuda-phases.cu
index 0230d30f8388..2622c3a1bf55 100644
--- a/clang/test/Driver/cuda-phases.cu
+++ b/clang/test/Driver/cuda-phases.cu
@@ -294,3 +294,27 @@
 // NON-CUDA-INPUT-NEXT: 22: backend, {21}, assembler, (host-cuda)
 // NON-CUDA-INPUT-NEXT: 23: assembler, {22}, object, (host-cuda)
 // NON-CUDA-INPUT-NEXT: 24: clang-linker-wrapper, {18, 23}, image, (host-cuda)
+
+//
+// Test the phases using the new driver in LTO-mode.
+//
+// RUN: %clang -### -target powerpc64le-ibm-linux-gnu --offload-new-driver 
-ccc-print-phases \
+// RUN:--offload-arch=sm_70 --offload-arch=sm_52 -foffload-lto 
-fgpu-rdc -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix=LTO %s
+//  LTO: 0: input, "[[INPUT:.+]]", cuda, (host-cuda)
+// LTO-NEXT: 1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
+// LTO-NEXT: 2: compiler, {1}, ir, (host-cuda)
+// LTO-NEXT: 3: input, "[[INPUT]]", cuda, (device-cuda, sm_52)
+// LTO-NEXT: 4: preprocessor, {3}, cu

[PATCH] D135305: [Clang] Fix using LTO with the new driver in RDC-mode

[Joseph Huber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGac135f9ee574: [Clang] Fix using LTO with the new driver in 
RDC-mode (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135305

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cuda-bindings.cu
  clang/test/Driver/cuda-phases.cu
  clang/test/Driver/hip-binding.hip
  clang/test/Driver/hip-phases.hip

Index: clang/test/Driver/hip-phases.hip
===
--- clang/test/Driver/hip-phases.hip
+++ clang/test/Driver/hip-phases.hip
@@ -546,3 +546,27 @@
 
 // CHECK: [[L0:[0-9]+]]: linker, {[[A3]], [[B3]]}, ir, (device-hip, [[ARCH]])
 // CHECK: offload, "device-hip (amdgcn-amd-amdhsa:[[ARCH]])" {[[L0]]}, ir
+
+//
+// Test the bindings using the new driver in LTO-mode.
+//
+// RUN: %clang -### --target=x86_64-linux-gnu --offload-new-driver -ccc-print-phases \
+// RUN:--offload-arch=gfx90a --offload-arch=gfx908 -foffload-lto -fgpu-rdc -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix=LTO %s
+//  LTO: 0: input, "[[INPUT:.+]]", hip, (host-hip)
+// LTO-NEXT: 1: preprocessor, {0}, hip-cpp-output, (host-hip)
+// LTO-NEXT: 2: compiler, {1}, ir, (host-hip)
+// LTO-NEXT: 3: input, "[[INPUT]]", hip, (device-hip, gfx908)
+// LTO-NEXT: 4: preprocessor, {3}, hip-cpp-output, (device-hip, gfx908)
+// LTO-NEXT: 5: compiler, {4}, ir, (device-hip, gfx908)
+// LTO-NEXT: 6: backend, {5}, lto-bc, (device-hip, gfx908)
+// LTO-NEXT: 7: offload, "device-hip (amdgcn-amd-amdhsa:gfx908)" {6}, lto-bc
+// LTO-NEXT: 8: input, "[[INPUT]]", hip, (device-hip, gfx90a)
+// LTO-NEXT: 9: preprocessor, {8}, hip-cpp-output, (device-hip, gfx90a)
+// LTO-NEXT: 10: compiler, {9}, ir, (device-hip, gfx90a)
+// LTO-NEXT: 11: backend, {10}, lto-bc, (device-hip, gfx90a)
+// LTO-NEXT: 12: offload, "device-hip (amdgcn-amd-amdhsa:gfx90a)" {11}, lto-bc
+// LTO-NEXT: 13: clang-offload-packager, {7, 12}, image, (device-hip)
+// LTO-NEXT: 14: offload, "host-hip (x86_64-unknown-linux-gnu)" {2}, "device-hip (x86_64-unknown-linux-gnu)" {13}, ir
+// LTO-NEXT: 15: backend, {14}, assembler, (host-hip)
+// LTO-NEXT: 16: assembler, {15}, object, (host-hip)
Index: clang/test/Driver/hip-binding.hip
===
--- clang/test/Driver/hip-binding.hip
+++ clang/test/Driver/hip-binding.hip
@@ -79,3 +79,15 @@
 // SYNTAX-ONLY: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-fsyntax-only"
 // SYNTAX-ONLY: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-fsyntax-only"
 // SYNTAX-ONLY: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-fsyntax-only"
+
+//
+// Check to ensure that we can use '-foffload' when not operating in RDC-mode.
+//
+// RUN: %clang -### --target=x86_64-linux-gnu -fno-gpu-rdc --offload-new-driver -ccc-print-bindings \
+// RUN:-foffload-lto --offload-arch=gfx90a --offload-arch=gfx908 -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix=LTO-NO-RDC %s
+//  LTO-NO-RDC: # "amdgcn-amd-amdhsa" - "clang", inputs: ["[[INPUT:.+]]"], output: "[[LTO_908:.+]]"
+// LTO-NO-RDC-NEXT: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[LTO_908]]"], output: "[[OBJ_908:.+]]"
+// LTO-NO-RDC-NEXT: # "amdgcn-amd-amdhsa" - "clang", inputs: ["[[INPUT]]"], output: "[[LTO_90A:.+]]"
+// LTO-NO-RDC-NEXT: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[LTO_90A]]"], output: "[[OBJ_90A:.+]]"
+// LTO-NO-RDC-NEXT: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[OBJ_908]]", "[[OBJ_90A]]"], output: "[[HIPFB:.+]]"
Index: clang/test/Driver/cuda-phases.cu
===
--- clang/test/Driver/cuda-phases.cu
+++ clang/test/Driver/cuda-phases.cu
@@ -294,3 +294,27 @@
 // NON-CUDA-INPUT-NEXT: 22: backend, {21}, assembler, (host-cuda)
 // NON-CUDA-INPUT-NEXT: 23: assembler, {22}, object, (host-cuda)
 // NON-CUDA-INPUT-NEXT: 24: clang-linker-wrapper, {18, 23}, image, (host-cuda)
+
+//
+// Test the phases using the new driver in LTO-mode.
+//
+// RUN: %clang -### -target powerpc64le-ibm-linux-gnu --offload-new-driver -ccc-print-phases \
+// RUN:--offload-arch=sm_70 --offload-arch=sm_52 -foffload-lto -fgpu-rdc -c %s 2>&1 \
+// RUN: | FileCheck -check-prefix=LTO %s
+//  LTO: 0: input, "[[INPUT:.+]]", cuda, (host-cuda)
+// LTO-NEXT: 1: preprocessor, {0}, cuda-cpp-output, (host-cuda)
+// LTO-NEXT: 2: compiler, {1}, ir, (host-cuda)
+// LTO-NEXT: 3: input, "[[INPUT]]", cuda, (device-cuda, sm_52)
+// LTO-NEXT: 4: preprocessor, {3}, cuda-cpp-output, (device-cuda, sm_52)
+// LTO-NEXT: 5: compiler, {4}, ir, (device-cuda, sm_52)
+// LTO-NEXT: 6: backend, {5}, lto-bc, (device-cuda, sm_52)
+// LTO-NEXT: 7: offload, "device-cuda (nvptx64-nvidia-cuda:sm_52)" {6}, lto-bc
+// LTO-NEXT: 8: input, "[[INPUT]]", cuda, (device-cuda, sm_70)
+// LTO-NEXT: 9: preprocessor, {8}, cuda-cpp-output, (device-

[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeEmitter.cpp:47
+  bool HasThisPointer = false;
+  if (const auto *MD = dyn_cast(F); MD && !MD->isStatic()) {
+HasThisPointer = true;





Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:638
   if (const auto CtorExpr = dyn_cast(Initializer)) {
-const CXXConstructorDecl *Ctor = CtorExpr->getConstructor();
-const RecordDecl *RD = Ctor->getParent();
-const Record *R = getRecord(RD);
-
-for (const auto *Init : Ctor->inits()) {
-  const FieldDecl *Member = Init->getMember();
-  const Expr *InitExpr = Init->getInit();
-
-  if (Optional T = classify(InitExpr->getType())) {
-const Record::Field *F = R->getField(Member);
-
-if (!this->emitDupPtr(Initializer))
-  return false;
+const Function *Func = getFunction(CtorExpr->getConstructor());
 

What happens if `Func` is null?



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:668
 const Decl *Callee = CE->getCalleeDecl();
-const Function *Func = P.getFunction(dyn_cast(Callee));
+const Function *Func = getFunction(dyn_cast(Callee));
+

Any reason not to use `cast` here instead, given that `getFunction()` expects a 
nonnull pointer anyway?



Comment at: clang/test/AST/Interp/records.cpp:106
+
+constexpr C RVOAndParams(const C *c) {
+  return C();

We're missing a fair amount of test coverage here in terms of calling member 
functions. Can you add some tests for that, as well as a test where the this 
pointer is invalid and causes UB? e.g.,
```
struct S {
  constexpr void member() {}
};

constexpr void func(S *s) {
  s->member(); // Should not be a constant expression
}

int main() {
  func(nullptr);
}
```


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

https://reviews.llvm.org/D134699

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

[PATCH] D135269: [AMDGPU] Disable bool range metadata to workaround backend issue

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 465731.
yaxunl marked an inline comment as done.
yaxunl edited the summary of this revision.
yaxunl added a comment.

update comments with issue link


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

https://reviews.llvm.org/D135269

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/test/CodeGenCUDA/bool-range.cu


Index: clang/test/CodeGenCUDA/bool-range.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/bool-range.cu
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm %s -O3 -o - -fcuda-is-device \
+// RUN:   -triple nvptx64-unknown-unknown | FileCheck %s -check-prefixes=NV
+// RUN: %clang_cc1 -emit-llvm %s -O3 -o - -fcuda-is-device \
+// RUN:   -triple amdgcn-amd-amdhsa | FileCheck %s -check-prefixes=AMD
+
+#include "Inputs/cuda.h"
+
+// NV:  %[[LD:[0-9]+]] = load i8, ptr %x,{{.*}} !range ![[MD:[0-9]+]]
+// NV:  store i8 %[[LD]], ptr %y
+// NV: ![[MD]] = !{i8 0, i8 2}
+
+// Make sure bool loaded from memory is truncated and
+// range metadata is not emitted.
+// TODO: Re-enable range metadata after issue
+// https://github.com/llvm/llvm-project/issues/58176 is fixed.
+
+// AMD:  %[[LD:[0-9]+]] = load i8, ptr addrspace(1) %x.global
+// AMD-NOT: !range
+// AMD:  %[[AND:[0-9]+]] = and i8 %[[LD]], 1
+// AMD:  store i8 %[[AND]], ptr addrspace(1) %y.global
+__global__ void test1(bool *x, bool *y) {
+  *y = *x != false;
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -1746,7 +1746,10 @@
   if (EmitScalarRangeCheck(Load, Ty, Loc)) {
 // In order to prevent the optimizer from throwing away the check, don't
 // attach range metadata to the load.
-  } else if (CGM.getCodeGenOpts().OptimizationLevel > 0)
+// TODO: Enable range metadata for AMDGCN after issue
+// https://github.com/llvm/llvm-project/issues/58176 is fixed.
+  } else if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
+ !CGM.getTriple().isAMDGCN())
 if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))
   Load->setMetadata(llvm::LLVMContext::MD_range, RangeInfo);
 


Index: clang/test/CodeGenCUDA/bool-range.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/bool-range.cu
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -emit-llvm %s -O3 -o - -fcuda-is-device \
+// RUN:   -triple nvptx64-unknown-unknown | FileCheck %s -check-prefixes=NV
+// RUN: %clang_cc1 -emit-llvm %s -O3 -o - -fcuda-is-device \
+// RUN:   -triple amdgcn-amd-amdhsa | FileCheck %s -check-prefixes=AMD
+
+#include "Inputs/cuda.h"
+
+// NV:  %[[LD:[0-9]+]] = load i8, ptr %x,{{.*}} !range ![[MD:[0-9]+]]
+// NV:  store i8 %[[LD]], ptr %y
+// NV: ![[MD]] = !{i8 0, i8 2}
+
+// Make sure bool loaded from memory is truncated and
+// range metadata is not emitted.
+// TODO: Re-enable range metadata after issue
+// https://github.com/llvm/llvm-project/issues/58176 is fixed.
+
+// AMD:  %[[LD:[0-9]+]] = load i8, ptr addrspace(1) %x.global
+// AMD-NOT: !range
+// AMD:  %[[AND:[0-9]+]] = and i8 %[[LD]], 1
+// AMD:  store i8 %[[AND]], ptr addrspace(1) %y.global
+__global__ void test1(bool *x, bool *y) {
+  *y = *x != false;
+}
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -1746,7 +1746,10 @@
   if (EmitScalarRangeCheck(Load, Ty, Loc)) {
 // In order to prevent the optimizer from throwing away the check, don't
 // attach range metadata to the load.
-  } else if (CGM.getCodeGenOpts().OptimizationLevel > 0)
+// TODO: Enable range metadata for AMDGCN after issue
+// https://github.com/llvm/llvm-project/issues/58176 is fixed.
+  } else if (CGM.getCodeGenOpts().OptimizationLevel > 0 &&
+ !CGM.getTriple().isAMDGCN())
 if (llvm::MDNode *RangeInfo = getRangeForLoadFromType(Ty))
   Load->setMetadata(llvm::LLVMContext::MD_range, RangeInfo);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Joshua Cranmer via Phabricator via cfe-commits]

jcranmer-intel added inline comments.



Comment at: clang/lib/AST/Interp/Floating.h:33-34
+  /// Primitive representing limits.
+  // static constexpr auto Min = std::numeric_limits::min();
+  // static constexpr auto Max = std::numeric_limits::max();
+

tbaeder wrote:
> This is currently commented out, but I //think// I can get the semantics of 
> the `APFloat` and ask its semantics for min/max values.
`APFloat::get{Largest,Smallest}` will do the trick.



Comment at: clang/lib/AST/Interp/Floating.h:89
+  bool isMin() const { return false; } // TODO
+  bool isMinusOne() const { return F == APFloat(-1.0f); }
+  bool isNan() const { return F.isNaN(); }

FYI, `operator==` on `APFloat` requires the two types to have the same 
semantics. Probably the fastest way to check if it's -1 is either `ilogb(F) == 
0 && F.isNegative()` or `F.isExactlyValue(-1.0)`.



Comment at: clang/lib/AST/Interp/Interp.cpp:421
 
+bool CastFP(InterpState &S, CodePtr OpPC, const llvm::fltSemantics *Sem) {
+  Floating F = S.Stk.pop();

FWIW, `const llvm::fltSemantics &` is the usual way it's used.


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

https://reviews.llvm.org/D134859

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

[PATCH] D135356: [Format] Fix crash when hitting eof while lexing JS template string

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/FormatTokenLexer.cpp:767
 if (Offset[0] == '\\') {
   ++Offset; // Skip the escaped character.
 } else if (Offset + 1 < Lex->getBuffer().end() && Offset[0] == '$' &&

kadircet wrote:
> it's a separate concern, but feels like this could also trigger some crashes 
> (e.g. an input like "`\")
Good catch. I think you're right, but:
- I'm not sure
- that example doesn't crash under asan, and I didn't find a crashing one
- that pattern occurs a bunch more times in this file
- I really want to have an isolated minimal fix + test as this bug is causing 
an outage
- I don't really have spare time now to dive deeper into this


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135356

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

[clang] 882a05a - [Format] Fix crash when hitting eof while lexing JS template string

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-10-06T17:00:41+02:00
New Revision: 882a05afa17f2a8863978027f562934cd7a7d179

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

LOG: [Format] Fix crash when hitting eof while lexing JS template string

Different loop termination conditions resulted in confusion of whether
*Offset was intended to be inside or outside the token.
This ultimately led to constructing an out-of-range SourceLocation.

Fix by making Offset consistently point *after* the token.

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

Added: 


Modified: 
clang/lib/Format/FormatTokenLexer.cpp
clang/unittests/Format/FormatTestJS.cpp

Removed: 




diff  --git a/clang/lib/Format/FormatTokenLexer.cpp 
b/clang/lib/Format/FormatTokenLexer.cpp
index 313ea673ca757..f8f5f7112188c 100644
--- a/clang/lib/Format/FormatTokenLexer.cpp
+++ b/clang/lib/Format/FormatTokenLexer.cpp
@@ -760,6 +760,7 @@ void FormatTokenLexer::handleTemplateStrings() {
   for (; Offset != Lex->getBuffer().end(); ++Offset) {
 if (Offset[0] == '`') {
   StateStack.pop();
+  ++Offset;
   break;
 }
 if (Offset[0] == '\\') {
@@ -768,12 +769,12 @@ void FormatTokenLexer::handleTemplateStrings() {
Offset[1] == '{') {
   // '${' introduces an expression interpolation in the template string.
   StateStack.push(LexerState::NORMAL);
-  ++Offset;
+  Offset += 2;
   break;
 }
   }
 
-  StringRef LiteralText(TmplBegin, Offset - TmplBegin + 1);
+  StringRef LiteralText(TmplBegin, Offset - TmplBegin);
   BacktickToken->setType(TT_TemplateString);
   BacktickToken->Tok.setKind(tok::string_literal);
   BacktickToken->TokenText = LiteralText;
@@ -794,9 +795,7 @@ void FormatTokenLexer::handleTemplateStrings() {
   StartColumn, Style.TabWidth, Encoding);
   }
 
-  SourceLocation loc = Offset < Lex->getBuffer().end()
-   ? Lex->getSourceLocation(Offset + 1)
-   : SourceMgr.getLocForEndOfFile(ID);
+  SourceLocation loc = Lex->getSourceLocation(Offset);
   resetLexer(SourceMgr.getFileOffset(loc));
 }
 

diff  --git a/clang/unittests/Format/FormatTestJS.cpp 
b/clang/unittests/Format/FormatTestJS.cpp
index 9883aae62191a..f2a5559e871dc 100644
--- a/clang/unittests/Format/FormatTestJS.cpp
+++ b/clang/unittests/Format/FormatTestJS.cpp
@@ -2145,6 +2145,8 @@ TEST_F(FormatTestJS, NestedTemplateStrings) {
 
   // Crashed at some point.
   verifyFormat("}");
+  verifyFormat("`");
+  verifyFormat("`\\");
 }
 
 TEST_F(FormatTestJS, TaggedTemplateStrings) {



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

[PATCH] D135356: [Format] Fix crash when hitting eof while lexing JS template string

[Sam McCall 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 rG882a05afa17f: [Format] Fix crash when hitting eof while 
lexing JS template string (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D135356?vs=465699&id=465742#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135356

Files:
  clang/lib/Format/FormatTokenLexer.cpp
  clang/unittests/Format/FormatTestJS.cpp


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2145,6 +2145,8 @@
 
   // Crashed at some point.
   verifyFormat("}");
+  verifyFormat("`");
+  verifyFormat("`\\");
 }
 
 TEST_F(FormatTestJS, TaggedTemplateStrings) {
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -760,6 +760,7 @@
   for (; Offset != Lex->getBuffer().end(); ++Offset) {
 if (Offset[0] == '`') {
   StateStack.pop();
+  ++Offset;
   break;
 }
 if (Offset[0] == '\\') {
@@ -768,12 +769,12 @@
Offset[1] == '{') {
   // '${' introduces an expression interpolation in the template string.
   StateStack.push(LexerState::NORMAL);
-  ++Offset;
+  Offset += 2;
   break;
 }
   }
 
-  StringRef LiteralText(TmplBegin, Offset - TmplBegin + 1);
+  StringRef LiteralText(TmplBegin, Offset - TmplBegin);
   BacktickToken->setType(TT_TemplateString);
   BacktickToken->Tok.setKind(tok::string_literal);
   BacktickToken->TokenText = LiteralText;
@@ -794,9 +795,7 @@
   StartColumn, Style.TabWidth, Encoding);
   }
 
-  SourceLocation loc = Offset < Lex->getBuffer().end()
-   ? Lex->getSourceLocation(Offset + 1)
-   : SourceMgr.getLocForEndOfFile(ID);
+  SourceLocation loc = Lex->getSourceLocation(Offset);
   resetLexer(SourceMgr.getFileOffset(loc));
 }
 


Index: clang/unittests/Format/FormatTestJS.cpp
===
--- clang/unittests/Format/FormatTestJS.cpp
+++ clang/unittests/Format/FormatTestJS.cpp
@@ -2145,6 +2145,8 @@
 
   // Crashed at some point.
   verifyFormat("}");
+  verifyFormat("`");
+  verifyFormat("`\\");
 }
 
 TEST_F(FormatTestJS, TaggedTemplateStrings) {
Index: clang/lib/Format/FormatTokenLexer.cpp
===
--- clang/lib/Format/FormatTokenLexer.cpp
+++ clang/lib/Format/FormatTokenLexer.cpp
@@ -760,6 +760,7 @@
   for (; Offset != Lex->getBuffer().end(); ++Offset) {
 if (Offset[0] == '`') {
   StateStack.pop();
+  ++Offset;
   break;
 }
 if (Offset[0] == '\\') {
@@ -768,12 +769,12 @@
Offset[1] == '{') {
   // '${' introduces an expression interpolation in the template string.
   StateStack.push(LexerState::NORMAL);
-  ++Offset;
+  Offset += 2;
   break;
 }
   }
 
-  StringRef LiteralText(TmplBegin, Offset - TmplBegin + 1);
+  StringRef LiteralText(TmplBegin, Offset - TmplBegin);
   BacktickToken->setType(TT_TemplateString);
   BacktickToken->Tok.setKind(tok::string_literal);
   BacktickToken->TokenText = LiteralText;
@@ -794,9 +795,7 @@
   StartColumn, Style.TabWidth, Encoding);
   }
 
-  SourceLocation loc = Offset < Lex->getBuffer().end()
-   ? Lex->getSourceLocation(Offset + 1)
-   : SourceMgr.getLocForEndOfFile(ID);
+  SourceLocation loc = Lex->getSourceLocation(Offset);
   resetLexer(SourceMgr.getFileOffset(loc));
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

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

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135366

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/test/AST/Interp/arrays.cpp


Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -118,6 +118,22 @@
// expected-note {{cannot refer to 
element -2 of array of 10}}
 };
 
+namespace strings {
+  constexpr const char *S = "abc";
+  static_assert(S[0] == 'a', "");
+  static_assert(S[1] == 'b', "");
+  static_assert(S[2] == 'c', "");
+  static_assert(S[3] == '\0', "");
+
+  static_assert("foobar"[2] == 'o', "");
+
+  constexpr const wchar_t *wide = L"bar";
+  static_assert(wide[0] == L'b', "");
+
+  constexpr const char32_t *u32 = U"abc";
+  static_assert(u32[1] == U'b', "");
+};
+
 namespace fillers {
   constexpr int k[3] = {1337};
   constexpr int foo(int m) {
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -90,6 +90,8 @@
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
+  bool VisitStringLiteral(const StringLiteral *E);
+  bool VisitCharacterLiteral(const CharacterLiteral *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -432,6 +432,23 @@
   return true;
 }
 
+template 
+bool ByteCodeExprGen::VisitStringLiteral(const StringLiteral *E) {
+  unsigned StringIndex = P.createGlobalString(E);
+  return this->emitGetPtrGlobal(StringIndex, E);
+}
+
+template 
+bool ByteCodeExprGen::VisitCharacterLiteral(
+const CharacterLiteral *E) {
+  QualType LitType = E->getType();
+  if (Optional T = classify(LitType)) {
+APInt Val(getIntWidth(LitType), E->getValue());
+return this->emitConst(*T, 0, Val, E);
+  }
+  return this->bail(E);
+}
+
 template  bool ByteCodeExprGen::discard(const Expr *E) 
{
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);


Index: clang/test/AST/Interp/arrays.cpp
===
--- clang/test/AST/Interp/arrays.cpp
+++ clang/test/AST/Interp/arrays.cpp
@@ -118,6 +118,22 @@
// expected-note {{cannot refer to element -2 of array of 10}}
 };
 
+namespace strings {
+  constexpr const char *S = "abc";
+  static_assert(S[0] == 'a', "");
+  static_assert(S[1] == 'b', "");
+  static_assert(S[2] == 'c', "");
+  static_assert(S[3] == '\0', "");
+
+  static_assert("foobar"[2] == 'o', "");
+
+  constexpr const wchar_t *wide = L"bar";
+  static_assert(wide[0] == L'b', "");
+
+  constexpr const char32_t *u32 = U"abc";
+  static_assert(u32[1] == U'b', "");
+};
+
 namespace fillers {
   constexpr int k[3] = {1337};
   constexpr int foo(int m) {
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -90,6 +90,8 @@
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
   bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *E);
+  bool VisitStringLiteral(const StringLiteral *E);
+  bool VisitCharacterLiteral(const CharacterLiteral *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -432,6 +432,23 @@
   return true;
 }
 
+template 
+bool ByteCodeExprGen::VisitStringLiteral(const StringLiteral *E) {
+  unsigned StringIndex = P.createGlobalString(E);
+  return this->emitGetPtrGlobal(StringIndex, E);
+}
+
+template 
+bool ByteCodeExprGen::VisitCharacterLiteral(
+const CharacterLiteral *E) {
+  QualType LitType = E->getType();
+  if (Optional T = classify(LitType)) {
+APInt Val(getIntWidth(LitType), E->getValue());
+return this->emitConst(*T, 0, Val, E);
+  }
+  return this->bail(E);
+}
+
 template  bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);

[PATCH] D135367: [clang-tidy] Dump effective diagnostics level in YAML output

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin created this revision.
DmitryPolukhin added reviewers: njames93, klimek.
DmitryPolukhin added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
DmitryPolukhin requested review of this revision.
Herald added a subscriber: cfe-commits.

Before this patch YAML output had default diagnostic level instead of effective 
level reported to the user on stdout. Wrapper scripts for clang-tidy usually 
use YAML output and they pick wrong diagnostics level without this patch.

Test Plan: check-clang-tools


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D135367

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp


Index: clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
@@ -1,5 +1,5 @@
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t-input.cpp
-// RUN: not clang-tidy %t-input.cpp 
-checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes,clang-diagnostic-zero-length-array'
 -export-fixes=%t.yaml -- -Wmissing-prototypes -Wzero-length-array > %t.msg 2>&1
+// RUN: not clang-tidy %t-input.cpp 
-checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes,clang-diagnostic-zero-length-array'
 --warnings-as-errors='clang-diagnostic-missing-prototypes' 
-export-fixes=%t.yaml -- -Wmissing-prototypes -Wzero-length-array > %t.msg 2>&1
 // RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s 
-implicit-check-not='{{warning|error|note}}:'
 // RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s
 #define X(n) void n ## n() {}
@@ -12,7 +12,7 @@
   member;
 };
 
-// CHECK-MESSAGES: -input.cpp:2:1: warning: no previous prototype for function 
'ff' [clang-diagnostic-missing-prototypes]
+// CHECK-MESSAGES: -input.cpp:2:1: error: no previous prototype for function 
'ff' [clang-diagnostic-missing-prototypes,-warnings-as-errors]
 // CHECK-MESSAGES: -input.cpp:1:19: note: expanded from macro 'X'
 // CHECK-MESSAGES: {{^}}note: expanded from here{{$}}
 // CHECK-MESSAGES: -input.cpp:2:1: note: declare 'static' if the function is 
not intended to be used outside of this translation unit
@@ -52,7 +52,7 @@
 // CHECK-YAML-NEXT: FilePath:'{{.*}}-input.cpp'
 // CHECK-YAML-NEXT: FileOffset:  13
 // CHECK-YAML-NEXT: Replacements:[]
-// CHECK-YAML-NEXT: Level:   Warning
+// CHECK-YAML-NEXT: Level:   Error
 // CHECK-YAML-NEXT: BuildDirectory:  '{{.*}}'
 // CHECK-YAML-NEXT:   - DiagnosticName:  clang-diagnostic-error
 // CHECK-YAML-NEXT: DiagnosticMessage:
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
@@ -403,6 +403,8 @@
 
 bool IsWarningAsError = DiagLevel == DiagnosticsEngine::Warning &&
 Context.treatAsError(CheckName);
+if (IsWarningAsError)
+  Level = ClangTidyError::Error;
 Errors.emplace_back(CheckName, Level, Context.getCurrentBuildDirectory(),
 IsWarningAsError);
   }


Index: clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
===
--- clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
+++ clang-tools-extra/test/clang-tidy/infrastructure/export-diagnostics.cpp
@@ -1,5 +1,5 @@
 // RUN: grep -Ev "// *[A-Z-]+:" %s > %t-input.cpp
-// RUN: not clang-tidy %t-input.cpp -checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes,clang-diagnostic-zero-length-array' -export-fixes=%t.yaml -- -Wmissing-prototypes -Wzero-length-array > %t.msg 2>&1
+// RUN: not clang-tidy %t-input.cpp -checks='-*,google-explicit-constructor,clang-diagnostic-missing-prototypes,clang-diagnostic-zero-length-array' --warnings-as-errors='clang-diagnostic-missing-prototypes' -export-fixes=%t.yaml -- -Wmissing-prototypes -Wzero-length-array > %t.msg 2>&1
 // RUN: FileCheck -input-file=%t.msg -check-prefix=CHECK-MESSAGES %s -implicit-check-not='{{warning|error|note}}:'
 // RUN: FileCheck -input-file=%t.yaml -check-prefix=CHECK-YAML %s
 #define X(n) void n ## n() {}
@@ -12,7 +12,7 @@
   member;
 };
 
-// CHECK-MESSAGES: -input.cpp:2:1: warning: no previous prototype for function 'ff' [clang-diagnostic-missing-prototypes]
+// CHECK-MESSAGES: -input.cpp:2:1: error: no previous prototype for function 'ff' [clang-diagnostic-missing-prototypes,-warnings-as-errors]
 // CHECK-MESSAGES: -input.cpp:1:19: note: expanded from macro 'X'
 // CHECK-MESSAGES: {{^

[PATCH] D134749: [clang][Interp] Implement Div opcode

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

tbaeder added a comment.

In D134749#3839980 , @aaron.ballman 
wrote:

> LGTM aside from a simple refactoring (feel free to apply it when landing or 
> do it post-commit as an NFC change).

I actually suggested something like this review :P I'll write it down and 
provide a follow-up NFC commit, thanks.


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

https://reviews.llvm.org/D134749

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/Interp.cpp:421
 
+bool CastFP(InterpState &S, CodePtr OpPC, const llvm::fltSemantics *Sem) {
+  Floating F = S.Stk.pop();

jcranmer-intel wrote:
> FWIW, `const llvm::fltSemantics &` is the usual way it's used.
I saw, but this is going through a `char[]` stack a gain and I don't think 
using a reference works there.


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

https://reviews.llvm.org/D134859

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

[PATCH] D135220: [clang] Update ModuleMap::getModuleMapFile* to use FileEntryRef

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D135220#3839671 , @goncharov wrote:

> That change might be problematic for content addressing storages. E.g. 
> clang/test/Driver/cl-pch-showincludes.cpp started to fail in my setup as 
> clang/test/Driver/header{0,1,3,4}.h are all identical and can be symlinked. 
> cc @dexonsmith

FWIW, I agree with Ben that this change seems like it should improve 
consistency for symlinked content. Knowing the failure mode / having 
reproduction steps would be helpful to track down your corner case!

(There are many subtle interactions around this stuff in clang so it’s hard to 
make forward progress.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135220

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

[PATCH] D134699: [clang][Interp] Implement This pointer passing to methods

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

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



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:638
   if (const auto CtorExpr = dyn_cast(Initializer)) {
-const CXXConstructorDecl *Ctor = CtorExpr->getConstructor();
-const RecordDecl *RD = Ctor->getParent();
-const Record *R = getRecord(RD);
-
-for (const auto *Init : Ctor->inits()) {
-  const FieldDecl *Member = Init->getMember();
-  const Expr *InitExpr = Init->getInit();
-
-  if (Optional T = classify(InitExpr->getType())) {
-const Record::Field *F = R->getField(Member);
-
-if (!this->emitDupPtr(Initializer))
-  return false;
+const Function *Func = getFunction(CtorExpr->getConstructor());
 

aaron.ballman wrote:
> What happens if `Func` is null?
We need to bail out!



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:668
 const Decl *Callee = CE->getCalleeDecl();
-const Function *Func = P.getFunction(dyn_cast(Callee));
+const Function *Func = getFunction(dyn_cast(Callee));
+

aaron.ballman wrote:
> Any reason not to use `cast` here instead, given that `getFunction()` expects 
> a nonnull pointer anyway?
Not particularly, I guess the generated assertion output is a little nicer if 
it reaches the one in `getFunction()`.


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

https://reviews.llvm.org/D134699

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

[PATCH] D135356: [Format] Fix crash when hitting eof while lexing JS template string

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks tests: http://45.33.8.238/linux/88341/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135356

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

[PATCH] D135226: [clangd] Optimize Dex::generateProximityURIs().

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG561443818a15: [clangd] Optimize 
Dex::generateProximityURIs(). (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135226

Files:
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h

Index: clang-tools-extra/clangd/index/dex/Dex.h
===
--- clang-tools-extra/clangd/index/dex/Dex.h
+++ clang-tools-extra/clangd/index/dex/Dex.h
@@ -132,7 +132,7 @@
 /// Should be used within the index build process.
 ///
 /// This function is exposed for testing only.
-std::vector generateProximityURIs(llvm::StringRef URIPath);
+llvm::SmallVector generateProximityURIs(llvm::StringRef);
 
 } // namespace dex
 } // namespace clangd
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -163,8 +163,8 @@
   llvm::StringMap Sources;
   for (const auto &Path : ProximityPaths) {
 Sources[Path] = SourceParams();
-auto PathURI = URI::create(Path);
-const auto PathProximityURIs = generateProximityURIs(PathURI.toString());
+auto PathURI = URI::create(Path).toString();
+const auto PathProximityURIs = generateProximityURIs(PathURI.c_str());
 for (const auto &ProximityURI : PathProximityURIs)
   ParentURIs.insert(ProximityURI);
   }
@@ -353,30 +353,49 @@
   return Bytes + BackingDataSize;
 }
 
-std::vector generateProximityURIs(llvm::StringRef URIPath) {
-  std::vector Result;
-  auto ParsedURI = URI::parse(URIPath);
-  assert(ParsedURI &&
- "Non-empty argument of generateProximityURIs() should be a valid "
- "URI.");
-  llvm::StringRef Body = ParsedURI->body();
-  // FIXME(kbobyrev): Currently, this is a heuristic which defines the maximum
-  // size of resulting vector. Some projects might want to have higher limit if
-  // the file hierarchy is deeper. For the generic case, it would be useful to
-  // calculate Limit in the index build stage by calculating the maximum depth
-  // of the project source tree at runtime.
-  size_t Limit = 5;
-  // Insert original URI before the loop: this would save a redundant iteration
-  // with a URI parse.
-  Result.emplace_back(ParsedURI->toString());
-  while (!Body.empty() && --Limit > 0) {
-// FIXME(kbobyrev): Parsing and encoding path to URIs is not necessary and
-// could be optimized.
-Body = llvm::sys::path::parent_path(Body, llvm::sys::path::Style::posix);
-if (!Body.empty())
-  Result.emplace_back(
-  URI(ParsedURI->scheme(), ParsedURI->authority(), Body).toString());
+// Given foo://bar/one/two
+// Returns  (or empty for bad URI)
+llvm::StringRef findPathInURI(llvm::StringRef S) {
+  S = S.split(':').second;   // Eat scheme.
+  if (S.consume_front("//")) // Eat authority.
+S = S.drop_until([](char C) { return C == '/'; });
+  return S;
+}
+
+// FIXME(kbobyrev): Currently, this is a heuristic which defines the maximum
+// size of resulting vector. Some projects might want to have higher limit if
+// the file hierarchy is deeper. For the generic case, it would be useful to
+// calculate Limit in the index build stage by calculating the maximum depth
+// of the project source tree at runtime.
+constexpr unsigned ProximityURILimit = 5;
+
+llvm::SmallVector
+generateProximityURIs(llvm::StringRef URI) {
+  // This function is hot when indexing, so don't parse/reserialize URIPath,
+  // just emit substrings of it instead.
+  //
+  // foo://bar/one/two
+  //  
+  //Path
+  llvm::StringRef Path = findPathInURI(URI);
+  if (Path.empty())
+return {}; // Bad URI.
+  assert(Path.begin() >= URI.begin() && Path.begin() < URI.end() &&
+ Path.end() == URI.end());
+
+  // The original is a proximity URI.
+  llvm::SmallVector Result = {URI};
+  // Form proximity URIs by chopping before each slash in the path part.
+  for (auto Slash = Path.rfind('/'); Slash > 0 && Slash != StringRef::npos;
+   Slash = Path.rfind('/')) {
+Path = Path.substr(0, Slash);
+Result.push_back(URI.substr(0, Path.end() - URI.data()));
+if (Result.size() == ProximityURILimit)
+  return Result;
   }
+  // The root foo://bar/ is a proximity URI.
+  if (Path.startswith("/"))
+Result.push_back(URI.substr(0, Path.begin() + 1 - URI.data()));
   return Result;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 5614438 - [clangd] Optimize Dex::generateProximityURIs().

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-10-06T17:01:04+02:00
New Revision: 561443818a15e7b368ddbb58207a14c60ba55c58

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

LOG: [clangd] Optimize Dex::generateProximityURIs().

Production profiles show that generateProximityURIs is roughly 3.8% of
buildPreamble. Of this, the majority (3% of buildPreamble) is parsing
and reserializing URIs.

We can do this with ugly string manipulation instead.

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

Added: 


Modified: 
clang-tools-extra/clangd/index/dex/Dex.cpp
clang-tools-extra/clangd/index/dex/Dex.h

Removed: 




diff  --git a/clang-tools-extra/clangd/index/dex/Dex.cpp 
b/clang-tools-extra/clangd/index/dex/Dex.cpp
index 2d778c13157f2..c66e7f0d936ce 100644
--- a/clang-tools-extra/clangd/index/dex/Dex.cpp
+++ b/clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -163,8 +163,8 @@ std::unique_ptr Dex::createFileProximityIterator(
   llvm::StringMap Sources;
   for (const auto &Path : ProximityPaths) {
 Sources[Path] = SourceParams();
-auto PathURI = URI::create(Path);
-const auto PathProximityURIs = generateProximityURIs(PathURI.toString());
+auto PathURI = URI::create(Path).toString();
+const auto PathProximityURIs = generateProximityURIs(PathURI.c_str());
 for (const auto &ProximityURI : PathProximityURIs)
   ParentURIs.insert(ProximityURI);
   }
@@ -353,30 +353,49 @@ size_t Dex::estimateMemoryUsage() const {
   return Bytes + BackingDataSize;
 }
 
-std::vector generateProximityURIs(llvm::StringRef URIPath) {
-  std::vector Result;
-  auto ParsedURI = URI::parse(URIPath);
-  assert(ParsedURI &&
- "Non-empty argument of generateProximityURIs() should be a valid "
- "URI.");
-  llvm::StringRef Body = ParsedURI->body();
-  // FIXME(kbobyrev): Currently, this is a heuristic which defines the maximum
-  // size of resulting vector. Some projects might want to have higher limit if
-  // the file hierarchy is deeper. For the generic case, it would be useful to
-  // calculate Limit in the index build stage by calculating the maximum depth
-  // of the project source tree at runtime.
-  size_t Limit = 5;
-  // Insert original URI before the loop: this would save a redundant iteration
-  // with a URI parse.
-  Result.emplace_back(ParsedURI->toString());
-  while (!Body.empty() && --Limit > 0) {
-// FIXME(kbobyrev): Parsing and encoding path to URIs is not necessary and
-// could be optimized.
-Body = llvm::sys::path::parent_path(Body, llvm::sys::path::Style::posix);
-if (!Body.empty())
-  Result.emplace_back(
-  URI(ParsedURI->scheme(), ParsedURI->authority(), Body).toString());
+// Given foo://bar/one/two
+// Returns  (or empty for bad URI)
+llvm::StringRef findPathInURI(llvm::StringRef S) {
+  S = S.split(':').second;   // Eat scheme.
+  if (S.consume_front("//")) // Eat authority.
+S = S.drop_until([](char C) { return C == '/'; });
+  return S;
+}
+
+// FIXME(kbobyrev): Currently, this is a heuristic which defines the maximum
+// size of resulting vector. Some projects might want to have higher limit if
+// the file hierarchy is deeper. For the generic case, it would be useful to
+// calculate Limit in the index build stage by calculating the maximum depth
+// of the project source tree at runtime.
+constexpr unsigned ProximityURILimit = 5;
+
+llvm::SmallVector
+generateProximityURIs(llvm::StringRef URI) {
+  // This function is hot when indexing, so don't parse/reserialize URIPath,
+  // just emit substrings of it instead.
+  //
+  // foo://bar/one/two
+  //  
+  //Path
+  llvm::StringRef Path = findPathInURI(URI);
+  if (Path.empty())
+return {}; // Bad URI.
+  assert(Path.begin() >= URI.begin() && Path.begin() < URI.end() &&
+ Path.end() == URI.end());
+
+  // The original is a proximity URI.
+  llvm::SmallVector Result = {URI};
+  // Form proximity URIs by chopping before each slash in the path part.
+  for (auto Slash = Path.rfind('/'); Slash > 0 && Slash != StringRef::npos;
+   Slash = Path.rfind('/')) {
+Path = Path.substr(0, Slash);
+Result.push_back(URI.substr(0, Path.end() - URI.data()));
+if (Result.size() == ProximityURILimit)
+  return Result;
   }
+  // The root foo://bar/ is a proximity URI.
+  if (Path.startswith("/"))
+Result.push_back(URI.substr(0, Path.begin() + 1 - URI.data()));
   return Result;
 }
 

diff  --git a/clang-tools-extra/clangd/index/dex/Dex.h 
b/clang-tools-extra/clangd/index/dex/Dex.h
index f29e7496946c1..69e161d51135b 100644
--- a/clang-tools-extra/clangd/index/dex/Dex.h
+++ b/clang-tools-extra/clangd/index/dex/Dex.h
@@ -132,7 +132,7 @@ class Dex : public SymbolIndex {
 /// Should be used within the index build