[PATCH] D134804: [clang][Interp] Implement bitwise Not operations

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

tbaeder updated this revision to Diff 464155.
tbaeder marked 4 inline comments as done.

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

https://reviews.llvm.org/D134804

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
@@ -3,6 +3,10 @@
 // RUN: %clang_cc1 -std=c++11 -verify=ref %s
 // RUN: %clang_cc1 -std=c++20 -verify=ref %s
 
+#define INT_MIN (~__INT_MAX__)
+#define INT_MAX __INT_MAX__
+
+
 static_assert(true, "");
 static_assert(false, ""); // expected-error{{failed}} ref-error{{failed}}
 static_assert(nullptr == nullptr, "");
@@ -66,6 +70,18 @@
 static_assert(-true, "");
 static_assert(-false, ""); //expected-error{{failed}} ref-error{{failed}}
 
+static_assert(~0 == -1, "");
+static_assert(~1 == -2, "");
+static_assert(~-1 == 0, "");
+static_assert(~255 == -256, "");
+static_assert(~INT_MIN == INT_MAX, "");
+static_assert(~INT_MAX == INT_MIN, "");
+
+enum E {};
+constexpr E e = static_cast(0);
+static_assert(~e == -1, "");
+
+
 constexpr int m = 10;
 constexpr const int *p = &m;
 static_assert(p != nullptr, "");
@@ -146,7 +162,6 @@
 #endif
 };
 
-#define INT_MIN (~__INT_MAX__)
 
 namespace rem {
   static_assert(2 % 2 == 0, "");
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -422,6 +422,12 @@
   let HasGroup = 1;
 }
 
+// [Real] -> [Real]
+def Comp: Opcode {
+  let Types = [NumberTypeClass];
+  let HasGroup = 1;
+}
+
 //===--===//
 // Cast.
 //===--===//
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -238,6 +238,20 @@
   return true;
 }
 
+/// 1) Pops the value from the stack.
+/// 2) Pushes the bitwise complemented value on the stack (~V).
+template ::T>
+bool Comp(InterpState &S, CodePtr OpPC) {
+  const T &Val = S.Stk.pop();
+  T Result;
+  if (!T::comp(Val, &Result)) {
+S.Stk.push(Result);
+return true;
+  }
+
+  return false;
+}
+
 //===--===//
 // EQ, NE, GT, GE, LT, LE
 //===--===//
Index: clang/lib/AST/Interp/Integral.h
===
--- clang/lib/AST/Interp/Integral.h
+++ clang/lib/AST/Interp/Integral.h
@@ -225,6 +225,11 @@
 return false;
   }
 
+  static bool comp(Integral A, Integral *R) {
+*R = Integral(~A.V);
+return false;
+  }
+
 private:
   template  static bool CheckAddUB(T A, T B, T &R) {
 if constexpr (std::is_signed_v) {
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1048,6 +1048,11 @@
   return DiscardResult ? this->emitPop(T, E) : true;
 });
   case UO_Not:// ~x
+if (!this->Visit(SubExpr))
+  return false;
+if (Optional T = classify(E->getType()))
+  return this->emitComp(*T, E);
+return false;
   case UO_Real:   // __real x
   case UO_Imag:   // __imag x
   case UO_Extension:
___
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 a comment.

I think this is missing a change in SemaChecking.cpp too. It would probably be 
better to first merge https://reviews.llvm.org/D134791 so that you don't have 
to patch (too much) multiple places :-)


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

[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 464156.
tbaeder marked an inline comment as done.

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

https://reviews.llvm.org/D134859

Files:
  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/Floating.h
  clang/lib/AST/Interp/Integral.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
@@ -253,3 +253,27 @@
 #endif
 
 };
+
+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,34 @@
+//===-- 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
+
+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_Float32,
   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<32>; };
 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_Float32:
 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_Float32, B)  \
   TYPE_SWITCH_CASE(PT_Bool, B) \
   TYPE_SWITCH_CASE(PT_Ptr, B)  \
 }  \
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -25,6 +25,7 @@
 def Uint32 : Type;
 def Sint64 : Type;
 def Uint64 : Type;
+def Float32 : Type;
 def Ptr : Type;
 
 //===---

[clang] 924996e - [clang] [Driver] Disable default configs via envvar during testing

[Michał Górny via cfe-commits]

Author: Michał Górny
Date: 2022-09-30T09:11:50+02:00
New Revision: 924996e0a011eb833a72d2a2cac9b40fa8a42e34

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

LOG: [clang] [Driver] Disable default configs via envvar during testing

Add support for a CLANG_NO_DEFAULT_CONFIG envvar that works like
the --no-default-config option when set to a non-empty value.  Use it
to disable loading system configuration files during the test suite
runs.

Configuration files can change the driver behavior in extensive ways,
and it is neither really possible nor feasible to account for or undo
the effects of even the most common configuration uses.  Therefore,
the most reasonable option seems to be to ignore configuration files
while running the majority of tests (with the notable exception of tests
for configuration file support).

Due to the diversity of ways that %clang is used in the test suite,
including using it to copy or symlink the clang executable, as well to
call -cc1 and -cc1as modes, it is not feasible to pass the explicit
options to disable config loading either.  Using an environment variable
has the advantage of being easily applied across the test suite
and easily unset for default configuration file loading tests.

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/test/Driver/config-file3.c
clang/test/Driver/env.c
clang/test/Unit/lit.cfg.py
clang/test/lit.cfg.py

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index b8ef37904574c..a4bd2ded3c17a 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -94,6 +94,7 @@
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
+#include  // ::getenv
 #include 
 #include 
 #include 
@@ -1066,6 +1067,12 @@ bool Driver::loadConfigFiles() {
 }
 
 bool Driver::loadDefaultConfigFiles(ArrayRef CfgFileSearchDirs) {
+  // Disable default config if CLANG_NO_DEFAULT_CONFIG is set to a non-empty
+  // value.
+  if (const char *NoConfigEnv = ::getenv("CLANG_NO_DEFAULT_CONFIG")) {
+if (*NoConfigEnv)
+  return false;
+  }
   if (CLOptions && CLOptions->hasArg(options::OPT_no_default_config))
 return false;
 

diff  --git a/clang/test/Driver/config-file3.c 
b/clang/test/Driver/config-file3.c
index 633ad740c0401..5f712020e79fd 100644
--- a/clang/test/Driver/config-file3.c
+++ b/clang/test/Driver/config-file3.c
@@ -1,6 +1,7 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
+// RUN: unset CLANG_NO_DEFAULT_CONFIG
 // RUN: rm -rf %t && mkdir %t
 
 //--- If config file is specified by relative path (workdir/cfg-s2), it is 
searched for by that path.

diff  --git a/clang/test/Driver/env.c b/clang/test/Driver/env.c
index 413b04e2cb629..0a60739db8718 100644
--- a/clang/test/Driver/env.c
+++ b/clang/test/Driver/env.c
@@ -4,13 +4,13 @@
 //
 // REQUIRES: shell
 // The PATH variable is heavily used when trying to find a linker.
-// RUN: env -i LC_ALL=C LD_LIBRARY_PATH="$LD_LIBRARY_PATH" \
+// RUN: env -i LC_ALL=C LD_LIBRARY_PATH="$LD_LIBRARY_PATH" 
CLANG_NO_DEFAULT_CONFIG=1 \
 // RUN:   %clang %s -### -o %t.o --target=i386-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN: --rtlib=platform -no-pie \
 // RUN: --gcc-toolchain="" 2>&1 | FileCheck --check-prefix=CHECK-LD-32 %s
 //
-// RUN: env -i LC_ALL=C PATH="" LD_LIBRARY_PATH="$LD_LIBRARY_PATH" \
+// RUN: env -i LC_ALL=C PATH="" LD_LIBRARY_PATH="$LD_LIBRARY_PATH" 
CLANG_NO_DEFAULT_CONFIG=1 \
 // RUN:   %clang %s -### -o %t.o --target=i386-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN: --rtlib=platform -no-pie \

diff  --git a/clang/test/Unit/lit.cfg.py b/clang/test/Unit/lit.cfg.py
index 8e3e6f2e862d7..bd75b3bb39682 100644
--- a/clang/test/Unit/lit.cfg.py
+++ b/clang/test/Unit/lit.cfg.py
@@ -71,3 +71,8 @@ def find_shlibpath_var():
 else:
 lit_config.warning("unable to inject shared library path on '{}'"
.format(platform.system()))
+
+# It is not realistically possible to account for all options that could
+# possibly be present in system and user configuration files, so disable
+# default configs for the test runs.
+config.environment["CLANG_NO_DEFAULT_CONFIG"] = "1"

diff  --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index 590b73a702572..278fbdc79c601 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -284,3 +284,8 @@ def exclude_unsupported_files_for_aix(dirname):
 
 if 'system-aix' in config.available_features:
 config.substitutions.append(('llvm-nm', 'env OBJECT_MODE=any llvm-nm'))
+
+# It is not realistically possible to account for all options that could
+# possi

[PATCH] D134905: [clang] [Driver] Disable default configs via envvar during testing

[Michał Górny 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 rG924996e0a011: [clang] [Driver] Disable default configs via 
envvar during testing (authored by mgorny).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134905

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/config-file3.c
  clang/test/Driver/env.c
  clang/test/Unit/lit.cfg.py
  clang/test/lit.cfg.py


Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -284,3 +284,8 @@
 
 if 'system-aix' in config.available_features:
 config.substitutions.append(('llvm-nm', 'env OBJECT_MODE=any llvm-nm'))
+
+# It is not realistically possible to account for all options that could
+# possibly be present in system and user configuration files, so disable
+# default configs for the test runs.
+config.environment["CLANG_NO_DEFAULT_CONFIG"] = "1"
Index: clang/test/Unit/lit.cfg.py
===
--- clang/test/Unit/lit.cfg.py
+++ clang/test/Unit/lit.cfg.py
@@ -71,3 +71,8 @@
 else:
 lit_config.warning("unable to inject shared library path on '{}'"
.format(platform.system()))
+
+# It is not realistically possible to account for all options that could
+# possibly be present in system and user configuration files, so disable
+# default configs for the test runs.
+config.environment["CLANG_NO_DEFAULT_CONFIG"] = "1"
Index: clang/test/Driver/env.c
===
--- clang/test/Driver/env.c
+++ clang/test/Driver/env.c
@@ -4,13 +4,13 @@
 //
 // REQUIRES: shell
 // The PATH variable is heavily used when trying to find a linker.
-// RUN: env -i LC_ALL=C LD_LIBRARY_PATH="$LD_LIBRARY_PATH" \
+// RUN: env -i LC_ALL=C LD_LIBRARY_PATH="$LD_LIBRARY_PATH" 
CLANG_NO_DEFAULT_CONFIG=1 \
 // RUN:   %clang %s -### -o %t.o --target=i386-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN: --rtlib=platform -no-pie \
 // RUN: --gcc-toolchain="" 2>&1 | FileCheck --check-prefix=CHECK-LD-32 %s
 //
-// RUN: env -i LC_ALL=C PATH="" LD_LIBRARY_PATH="$LD_LIBRARY_PATH" \
+// RUN: env -i LC_ALL=C PATH="" LD_LIBRARY_PATH="$LD_LIBRARY_PATH" 
CLANG_NO_DEFAULT_CONFIG=1 \
 // RUN:   %clang %s -### -o %t.o --target=i386-unknown-linux \
 // RUN: --sysroot=%S/Inputs/basic_linux_tree \
 // RUN: --rtlib=platform -no-pie \
Index: clang/test/Driver/config-file3.c
===
--- clang/test/Driver/config-file3.c
+++ clang/test/Driver/config-file3.c
@@ -1,6 +1,7 @@
 // REQUIRES: shell
 // REQUIRES: x86-registered-target
 
+// RUN: unset CLANG_NO_DEFAULT_CONFIG
 // RUN: rm -rf %t && mkdir %t
 
 //--- If config file is specified by relative path (workdir/cfg-s2), it is 
searched for by that path.
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -94,6 +94,7 @@
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
+#include  // ::getenv
 #include 
 #include 
 #include 
@@ -1066,6 +1067,12 @@
 }
 
 bool Driver::loadDefaultConfigFiles(ArrayRef CfgFileSearchDirs) {
+  // Disable default config if CLANG_NO_DEFAULT_CONFIG is set to a non-empty
+  // value.
+  if (const char *NoConfigEnv = ::getenv("CLANG_NO_DEFAULT_CONFIG")) {
+if (*NoConfigEnv)
+  return false;
+  }
   if (CLOptions && CLOptions->hasArg(options::OPT_no_default_config))
 return false;
 


Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -284,3 +284,8 @@
 
 if 'system-aix' in config.available_features:
 config.substitutions.append(('llvm-nm', 'env OBJECT_MODE=any llvm-nm'))
+
+# It is not realistically possible to account for all options that could
+# possibly be present in system and user configuration files, so disable
+# default configs for the test runs.
+config.environment["CLANG_NO_DEFAULT_CONFIG"] = "1"
Index: clang/test/Unit/lit.cfg.py
===
--- clang/test/Unit/lit.cfg.py
+++ clang/test/Unit/lit.cfg.py
@@ -71,3 +71,8 @@
 else:
 lit_config.warning("unable to inject shared library path on '{}'"
.format(platform.system()))
+
+# It is not realistically possible to account for all options that could
+# possibly be present in system and user configuration files, so disable
+# default configs for the test runs.
+config.environment["CLANG_NO_DEFAULT_CONFIG"] = "1"
Index: clang/test/Driver/env.c
=

[PATCH] D125944: Template instantiation error recovery

[Purva Chaudhari via Phabricator via cfe-commits]

Purva-Chaudhari updated this revision to Diff 464159.
Purva-Chaudhari added a comment.

rebase, test passing locally


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

https://reviews.llvm.org/D125944

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/test/Interpreter/template-recovery.cpp


Index: clang/test/Interpreter/template-recovery.cpp
===
--- /dev/null
+++ clang/test/Interpreter/template-recovery.cpp
@@ -0,0 +1,18 @@
+// clang-format off
+// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
+// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck 
--check-prefix=CHECK-DRIVER %s
+// UNSUPPORTED: system-aix
+// CHECK-DRIVER: i = 10
+// RUN: cat %s | clang-repl | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
+
+template T f() { return T(); }
+auto ptu2 = f(); err;
+auto ptu2 = f();
+
+extern "C" int printf(const char *, ...);
+int i = 10;
+auto r1 = printf("i = %d\n", i);
+// CHECK: i = 10
+
+%quit
Index: clang/lib/Interpreter/IncrementalParser.cpp
===
--- clang/lib/Interpreter/IncrementalParser.cpp
+++ clang/lib/Interpreter/IncrementalParser.cpp
@@ -150,6 +150,7 @@
   llvm::CrashRecoveryContextCleanupRegistrar CleanupSema(&S);
   Sema::GlobalEagerInstantiationScope GlobalInstantiations(S, 
/*Enabled=*/true);
   Sema::LocalEagerInstantiationScope LocalInstantiations(S);
+  Sema::PerformPendingInstantiationsRAII PerformPendingInstantiations(S);
 
   PTUs.emplace_back(PartialTranslationUnit());
   PartialTranslationUnit &LastPTU = PTUs.back();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -9719,6 +9719,19 @@
 SavedPendingLocalImplicitInstantiations;
   };
 
+  class PerformPendingInstantiationsRAII {
+  public:
+PerformPendingInstantiationsRAII(Sema &S): S(S) {} ;
+
+~PerformPendingInstantiationsRAII() {
+  S.PerformPendingInstantiations();
+  assert(S.PendingInstantiations.empty() &&
+ "there shouldn't be any pending instantiations");
+}
+  private:
+Sema &S;
+  };  
+
   /// A helper class for building up ExtParameterInfos.
   class ExtParameterInfoBuilder {
 SmallVector Infos;


Index: clang/test/Interpreter/template-recovery.cpp
===
--- /dev/null
+++ clang/test/Interpreter/template-recovery.cpp
@@ -0,0 +1,18 @@
+// clang-format off
+// RUN: clang-repl "int i = 10;" 'extern "C" int printf(const char*,...);' \
+// RUN:'auto r1 = printf("i = %d\n", i);' | FileCheck --check-prefix=CHECK-DRIVER %s
+// UNSUPPORTED: system-aix
+// CHECK-DRIVER: i = 10
+// RUN: cat %s | clang-repl | FileCheck %s
+// RUN: cat %s | clang-repl -Xcc -O2 | FileCheck %s
+
+template T f() { return T(); }
+auto ptu2 = f(); err;
+auto ptu2 = f();
+
+extern "C" int printf(const char *, ...);
+int i = 10;
+auto r1 = printf("i = %d\n", i);
+// CHECK: i = 10
+
+%quit
Index: clang/lib/Interpreter/IncrementalParser.cpp
===
--- clang/lib/Interpreter/IncrementalParser.cpp
+++ clang/lib/Interpreter/IncrementalParser.cpp
@@ -150,6 +150,7 @@
   llvm::CrashRecoveryContextCleanupRegistrar CleanupSema(&S);
   Sema::GlobalEagerInstantiationScope GlobalInstantiations(S, /*Enabled=*/true);
   Sema::LocalEagerInstantiationScope LocalInstantiations(S);
+  Sema::PerformPendingInstantiationsRAII PerformPendingInstantiations(S);
 
   PTUs.emplace_back(PartialTranslationUnit());
   PartialTranslationUnit &LastPTU = PTUs.back();
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -9719,6 +9719,19 @@
 SavedPendingLocalImplicitInstantiations;
   };
 
+  class PerformPendingInstantiationsRAII {
+  public:
+PerformPendingInstantiationsRAII(Sema &S): S(S) {} ;
+
+~PerformPendingInstantiationsRAII() {
+  S.PerformPendingInstantiations();
+  assert(S.PendingInstantiations.empty() &&
+ "there shouldn't be any pending instantiations");
+}
+  private:
+Sema &S;
+  };  
+
   /// A helper class for building up ExtParameterInfos.
   class ExtParameterInfoBuilder {
 SmallVector Infos;
___
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 464161.

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

https://reviews.llvm.org/D134859

Files:
  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/Floating.h
  clang/lib/AST/Interp/Integral.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
@@ -253,3 +253,27 @@
 #endif
 
 };
+
+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,34 @@
+//===-- 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
+
+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_Float32,
   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<32>; };
 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_Float32:
 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_Float32, B)  \
   TYPE_SWITCH_CASE(PT_Bool, B) \
   TYPE_SWITCH_CASE(PT_Ptr, B)  \
 }  \
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -25,6 +25,7 @@
 def Uint32 : Type;
 def Sint64 : Type;
 def Uint64 : Type;
+def Float32 : Type;
 def Ptr : Type;
 
 //===--===//
@@ -40,6 +41,7 @@
 def ArgUin

[PATCH] D134801: [clang][Interp] Implement ConditionalOperators

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

tbaeder updated this revision to Diff 464165.
tbaeder marked 3 inline comments as done.

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

https://reviews.llvm.org/D134801

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/test/AST/Interp/literals.cpp
  clang/test/SemaCXX/constexpr-factorial.cpp

Index: clang/test/SemaCXX/constexpr-factorial.cpp
===
--- clang/test/SemaCXX/constexpr-factorial.cpp
+++ clang/test/SemaCXX/constexpr-factorial.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++11 -fsyntax-only %s
+// RUN: %clang_cc1 -std=c++11 -fsyntax-only -fexperimental-new-constant-interpreter %s
 
 constexpr unsigned oddfac(unsigned n) {
   return n == 1 ? 1 : n * oddfac(n-2);
Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -190,3 +190,54 @@
   constexpr long unsigned RHS = 3;
   static_assert(LHS / RHS == 4, "");
 };
+
+namespace cond {
+  constexpr bool isEven(int n) {
+return n % 2 == 0 ? true : false;
+  }
+  static_assert(isEven(2), "");
+  static_assert(!isEven(3), "");
+  static_assert(isEven(100), "");
+
+  constexpr int M = 5 ? 10 : 20;
+  static_assert(M == 10, "");
+
+  static_assert(5 ? 13 : 16 == 13, "");
+  static_assert(0 ? 13 : 16 == 16, "");
+
+  static_assert(number ?: -15 == number, "");
+  static_assert(0 ?: 100 == 100 , "");
+
+#if __cplusplus >= 201402L
+  constexpr int N = 20;
+  constexpr int foo() {
+int m = N > 0 ? 5 : 10;
+
+return m == 5 ? isEven(m) : true;
+  }
+  static_assert(foo() == false, "");
+
+  constexpr int dontCallMe(unsigned m) {
+if (m == 0) return 0;
+return dontCallMe(m - 2);
+  }
+
+  // Can't call this because it will run into infinite recursion.
+  constexpr int assertNotReached() {
+return dontCallMe(3);
+  }
+
+  constexpr int testCond() {
+return true ? 5 : assertNotReached();
+  }
+
+  constexpr int testCond2() {
+return false ? assertNotReached() : 10;
+  }
+
+  static_assert(testCond() == 5, "");
+  static_assert(testCond2() == 10, "");
+
+#endif
+
+};
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -88,6 +88,7 @@
   bool VisitMemberExpr(const MemberExpr *E);
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E);
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
+  bool VisitAbstractConditionalOperator(const AbstractConditionalOperator *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
@@ -344,6 +344,37 @@
   return this->visit(E->getSourceExpr());
 }
 
+template 
+bool ByteCodeExprGen::VisitAbstractConditionalOperator(
+const AbstractConditionalOperator *E) {
+  const Expr *Condition = E->getCond();
+  const Expr *TrueExpr = E->getTrueExpr();
+  const Expr *FalseExpr = E->getFalseExpr();
+
+  LabelTy LabelEnd = this->getLabel();   // Label after the operator.
+  LabelTy LabelFalse = this->getLabel(); // Label for the false expr.
+
+  if (!this->visit(Condition))
+return false;
+  if (!this->jumpFalse(LabelFalse))
+return false;
+
+  if (!this->visit(TrueExpr))
+return false;
+  if (!this->jump(LabelEnd))
+return false;
+
+  this->emitLabel(LabelFalse);
+
+  if (!this->visit(FalseExpr))
+return false;
+
+  this->fallthrough(LabelEnd);
+  this->emitLabel(LabelEnd);
+
+  return true;
+}
+
 template  bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134801: [clang][Interp] Implement ConditionalOperators

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:90
   bool VisitOpaqueValueExpr(const OpaqueValueExpr *E);
+  bool VisitConditionalOperator(const ConditionalOperator *E);
 

aaron.ballman wrote:
> Do we want to handle `BinaryConditionalOperator` at the same time? (IIRC, you 
> should be able to use `AbstractConditionalOperator` here and the logic is 
> handled for you automagically for both constructs.) e.g.,
> ```
> foo() ? 12 : bar  // C conditional operator, has the value 12 if foo() is 
> nonzero and bar otherwise
> foo() ? : bar // GNU "missing middle" conditional operator, has the value of 
> foo() if nonzero and bar otherwise
> ```
Yep, that just worked


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

https://reviews.llvm.org/D134801

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

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

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler updated this revision to Diff 464170.
ckandeler marked 3 inline comments as done.
ckandeler added a comment.

Adapted according to latest comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/semantic-tokens.test
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
@@ -18,18 +18,18 @@
 TEST_F(AnnotateHighlightingsTest, Test) {
   EXPECT_AVAILABLE("^vo^id^ ^f(^) {^}^"); // available everywhere.
   EXPECT_AVAILABLE("[[int a; int b;]]");
-  EXPECT_EQ("void /* Function [decl] [globalScope] */f() {}",
+  EXPECT_EQ("void /* Function [decl] [def] [globalScope] */f() {}",
 apply("void ^f() {}"));
 
   EXPECT_EQ(apply("[[int f1(); const int x = f1();]]"),
 "int /* Function [decl] [globalScope] */f1(); "
-"const int /* Variable [decl] [readonly] [fileScope] */x = "
+"const int /* Variable [decl] [def] [readonly] [fileScope] */x = "
 "/* Function [globalScope] */f1();");
 
   // Only the targeted range is annotated.
   EXPECT_EQ(apply("void f1(); void f2() {^}"),
 "void f1(); "
-"void /* Function [decl] [globalScope] */f2() {}");
+"void /* Function [decl] [def] [globalScope] */f2() {}");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -48,12 +48,20 @@
 assert(StartOffset <= EndOffset);
 assert(NextChar <= StartOffset);
 
+bool hasDef =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Definition));
+bool hasDecl =
+T.Modifiers & (1 << uint32_t(HighlightingModifier::Declaration));
+EXPECT_TRUE(!hasDef || hasDecl);
+
 OS << Input.substr(NextChar, StartOffset - NextChar);
 OS << '$' << T.Kind;
 for (unsigned I = 0;
  I <= static_cast(HighlightingModifier::LastModifier); ++I) {
-  if (T.Modifiers & (1 << I))
-OS << '_' << static_cast(I);
+  if (T.Modifiers & (1 << I)) {
+if (I != uint32_t(HighlightingModifier::Declaration) || !hasDef)
+  OS << '_' << static_cast(I);
+  }
 }
 OS << "[[" << Input.substr(StartOffset, EndOffset - StartOffset) << "]]";
 NextChar = EndOffset;
@@ -96,52 +104,52 @@
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
   R"cpp(
-  struct $Class_decl[[AS]] {
+  struct $Class_def[[AS]] {
 double $Field_decl[[SomeMember]];
   };
   struct {
-  } $Variable_decl[[S]];
-  void $Function_decl[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
-$Class[[AS]] $LocalVariable_decl[[AA]];
-$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
-auto $LocalVariable_decl[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
+  } $Variable_def[[S]];
+  void $Function_def[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[VeryLongVariableName]] = 12312;
+$Class[[AS]] $LocalVariable_def[[AA]];
+$Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_def[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+auto $LocalVariable_def[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
 $LocalVariable[[FN]](12312);
   }
 )cpp",
   R"cpp(
   void $Function_decl[[foo]](int);
   void $Function_decl[[Gah]]();
-  void $Function_decl[[foo]]() {
-auto $LocalVariable_decl[[Bou]] = $Function[[Gah]];
+  void $Function_def[[foo]]() {
+auto $LocalVariable_def[[Bou]] = $Function[[Gah]];
   }
-  struct $Class_decl[[A]] {
+  struct $Class_def[[A]] {
 void $Method_decl[[abc]]();
   };
 )cpp",
   R"cpp(
   namespace $Namespace_decl[[abc]] {
 template
-struct $Class_decl[[A]] {
+struct $Class_def[[A]] {
   $TemplateParameter[[T]] $

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

[Christian Kandeler via Phabricator via cfe-commits]

ckandeler added inline comments.



Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:79
+  if (auto *Var = dyn_cast(Decl))
+return !isa(Var) && Var->isThisDeclarationADefinition();
+  return isa(Decl) || isa(Decl);

I'm not 100% sure about this one, by the way. I've just never heard anyone talk 
about a "parameter definition".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403

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

[PATCH] D134885: [Clang] Fix variant crashes from GH58028, GH57370

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 464172.
royjacobson added a comment.

Slightly modify the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134885

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/specialization-diagnose-crash.cpp


Index: clang/test/SemaCXX/specialization-diagnose-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/specialization-diagnose-crash.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// This is a reduction of GH57370 and GH58028, originally appearing
+// in libstdc++'s variant code.
+
+struct V1 {};
+struct V2 : V1 {
+  int &a;
+};
+
+template  using void_t = void;
+
+template  struct X { T x; };
+
+template  struct Variant {
+  Variant() = delete; // expected-note {{deleted here}}
+};
+
+template 
+struct Variant{T1()})>> {};
+
+void f() {
+  Variant();
+  Variant(); // expected-error {{call to deleted constructor}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -695,10 +695,10 @@
 //   member of reference type uninitialized, the program is
 //   ill-formed.
 SemaRef.Diag(Loc, diag::err_init_reference_member_uninitialized)
-  << Field->getType()
-  << ILE->getSyntacticForm()->getSourceRange();
-SemaRef.Diag(Field->getLocation(),
- diag::note_uninit_reference_member);
+<< Field->getType()
+<< (ILE->isSyntacticForm() ? ILE : ILE->getSyntacticForm())
+   ->getSourceRange();
+SemaRef.Diag(Field->getLocation(), diag::note_uninit_reference_member);
   }
   hadError = true;
   return;


Index: clang/test/SemaCXX/specialization-diagnose-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/specialization-diagnose-crash.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// This is a reduction of GH57370 and GH58028, originally appearing
+// in libstdc++'s variant code.
+
+struct V1 {};
+struct V2 : V1 {
+  int &a;
+};
+
+template  using void_t = void;
+
+template  struct X { T x; };
+
+template  struct Variant {
+  Variant() = delete; // expected-note {{deleted here}}
+};
+
+template 
+struct Variant{T1()})>> {};
+
+void f() {
+  Variant();
+  Variant(); // expected-error {{call to deleted constructor}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -695,10 +695,10 @@
 //   member of reference type uninitialized, the program is
 //   ill-formed.
 SemaRef.Diag(Loc, diag::err_init_reference_member_uninitialized)
-  << Field->getType()
-  << ILE->getSyntacticForm()->getSourceRange();
-SemaRef.Diag(Field->getLocation(),
- diag::note_uninit_reference_member);
+<< Field->getType()
+<< (ILE->isSyntacticForm() ? ILE : ILE->getSyntacticForm())
+   ->getSourceRange();
+SemaRef.Diag(Field->getLocation(), diag::note_uninit_reference_member);
   }
   hadError = true;
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134885: [Clang] Fix variant crashes from GH58028, GH57370

[Roy Jacobson via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/specialization-diagnose-crash.cpp:2
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// expected-no-diagnostics
+// This is a reduction of GH57370 and GH58028, originally appearing

shafik wrote:
> Shouldn't we see a diagnostic in order to exercise the code that is being 
> fixed?
Since this fixes an ICE I'm not sure we actually need to, but I modified it a 
bit so we have assurance that the specialization code runs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134885

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

[clang] 9706bb3 - [Clang] Fix variant crashes from GH58028, GH57370

[Roy Jacobson via cfe-commits]

Author: Roy Jacobson
Date: 2022-09-30T11:09:02+03:00
New Revision: 9706bb3165f5e508d5e2247ad8a3f45077df546d

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

LOG: [Clang] Fix variant crashes from GH58028, GH57370

Fixes a null dereference in some diagnostic issuing code.

Closes https://github.com/llvm/llvm-project/issues/57370
Closes https://github.com/llvm/llvm-project/issues/58028

Reviewed By: shafik

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

Added: 
clang/test/SemaCXX/specialization-diagnose-crash.cpp

Modified: 
clang/lib/Sema/SemaInit.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 7097b9deb8ed6..ee6fee0ac7324 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -695,10 +695,10 @@ void InitListChecker::FillInEmptyInitForField(unsigned 
Init, FieldDecl *Field,
 //   member of reference type uninitialized, the program is
 //   ill-formed.
 SemaRef.Diag(Loc, diag::err_init_reference_member_uninitialized)
-  << Field->getType()
-  << ILE->getSyntacticForm()->getSourceRange();
-SemaRef.Diag(Field->getLocation(),
- diag::note_uninit_reference_member);
+<< Field->getType()
+<< (ILE->isSyntacticForm() ? ILE : ILE->getSyntacticForm())
+   ->getSourceRange();
+SemaRef.Diag(Field->getLocation(), diag::note_uninit_reference_member);
   }
   hadError = true;
   return;

diff  --git a/clang/test/SemaCXX/specialization-diagnose-crash.cpp 
b/clang/test/SemaCXX/specialization-diagnose-crash.cpp
new file mode 100644
index 0..ceb3c4655f745
--- /dev/null
+++ b/clang/test/SemaCXX/specialization-diagnose-crash.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// This is a reduction of GH57370 and GH58028, originally appearing
+// in libstdc++'s variant code.
+
+struct V1 {};
+struct V2 : V1 {
+  int &a;
+};
+
+template  using void_t = void;
+
+template  struct X { T x; };
+
+template  struct Variant {
+  Variant() = delete; // expected-note {{deleted here}}
+};
+
+template 
+struct Variant{T1()})>> {};
+
+void f() {
+  Variant();
+  Variant(); // expected-error {{call to deleted constructor}}
+}



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

[PATCH] D134885: [Clang] Fix variant crashes from GH58028, GH57370

[Roy Jacobson via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
royjacobson marked an inline comment as done.
Closed by commit rG9706bb3165f5: [Clang] Fix variant crashes from GH58028, 
GH57370 (authored by royjacobson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134885

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/specialization-diagnose-crash.cpp


Index: clang/test/SemaCXX/specialization-diagnose-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/specialization-diagnose-crash.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// This is a reduction of GH57370 and GH58028, originally appearing
+// in libstdc++'s variant code.
+
+struct V1 {};
+struct V2 : V1 {
+  int &a;
+};
+
+template  using void_t = void;
+
+template  struct X { T x; };
+
+template  struct Variant {
+  Variant() = delete; // expected-note {{deleted here}}
+};
+
+template 
+struct Variant{T1()})>> {};
+
+void f() {
+  Variant();
+  Variant(); // expected-error {{call to deleted constructor}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -695,10 +695,10 @@
 //   member of reference type uninitialized, the program is
 //   ill-formed.
 SemaRef.Diag(Loc, diag::err_init_reference_member_uninitialized)
-  << Field->getType()
-  << ILE->getSyntacticForm()->getSourceRange();
-SemaRef.Diag(Field->getLocation(),
- diag::note_uninit_reference_member);
+<< Field->getType()
+<< (ILE->isSyntacticForm() ? ILE : ILE->getSyntacticForm())
+   ->getSourceRange();
+SemaRef.Diag(Field->getLocation(), diag::note_uninit_reference_member);
   }
   hadError = true;
   return;


Index: clang/test/SemaCXX/specialization-diagnose-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/specialization-diagnose-crash.cpp
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+// This is a reduction of GH57370 and GH58028, originally appearing
+// in libstdc++'s variant code.
+
+struct V1 {};
+struct V2 : V1 {
+  int &a;
+};
+
+template  using void_t = void;
+
+template  struct X { T x; };
+
+template  struct Variant {
+  Variant() = delete; // expected-note {{deleted here}}
+};
+
+template 
+struct Variant{T1()})>> {};
+
+void f() {
+  Variant();
+  Variant(); // expected-error {{call to deleted constructor}}
+}
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -695,10 +695,10 @@
 //   member of reference type uninitialized, the program is
 //   ill-formed.
 SemaRef.Diag(Loc, diag::err_init_reference_member_uninitialized)
-  << Field->getType()
-  << ILE->getSyntacticForm()->getSourceRange();
-SemaRef.Diag(Field->getLocation(),
- diag::note_uninit_reference_member);
+<< Field->getType()
+<< (ILE->isSyntacticForm() ? ILE : ILE->getSyntacticForm())
+   ->getSourceRange();
+SemaRef.Diag(Field->getLocation(), diag::note_uninit_reference_member);
   }
   hadError = true;
   return;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134898: [Clang] define __cpp_named_character_escapes

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson updated this revision to Diff 464174.
royjacobson added a comment.

Remove literal suffix in accordance with the rest of the test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134898

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Lexer/cxx-features.cpp


Index: clang/test/Lexer/cxx-features.cpp
===
--- clang/test/Lexer/cxx-features.cpp
+++ clang/test/Lexer/cxx-features.cpp
@@ -51,7 +51,7 @@
 #error "wrong value for __cpp_static_call_operator"
 #endif
 
-#if check(named_character_escapes, 0, 0, 0, 0, 0, 0)
+#if check(named_character_escapes, 202207, 202207, 202207, 202207, 202207, 
202207)
 #error "wrong value for __cpp_named_character_escapes"
 #endif
 
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -697,10 +697,11 @@
 Builder.defineMacro("__cpp_multidimensional_subscript", "202110L");
   }
 
-  // We provide this as an extension in earlier language modes, so we
-  // also define the macro.
+  // We provide those C++2b features as extensions in earlier language modes, 
so
+  // we also define their feature test macros.
   if (LangOpts.CPlusPlus11)
 Builder.defineMacro("__cpp_static_call_operator", "202207L");
+  Builder.defineMacro("__cpp_named_character_escapes", "202207L");
 
   if (LangOpts.Char8)
 Builder.defineMacro("__cpp_char8_t", "201811L");


Index: clang/test/Lexer/cxx-features.cpp
===
--- clang/test/Lexer/cxx-features.cpp
+++ clang/test/Lexer/cxx-features.cpp
@@ -51,7 +51,7 @@
 #error "wrong value for __cpp_static_call_operator"
 #endif
 
-#if check(named_character_escapes, 0, 0, 0, 0, 0, 0)
+#if check(named_character_escapes, 202207, 202207, 202207, 202207, 202207, 202207)
 #error "wrong value for __cpp_named_character_escapes"
 #endif
 
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -697,10 +697,11 @@
 Builder.defineMacro("__cpp_multidimensional_subscript", "202110L");
   }
 
-  // We provide this as an extension in earlier language modes, so we
-  // also define the macro.
+  // We provide those C++2b features as extensions in earlier language modes, so
+  // we also define their feature test macros.
   if (LangOpts.CPlusPlus11)
 Builder.defineMacro("__cpp_static_call_operator", "202207L");
+  Builder.defineMacro("__cpp_named_character_escapes", "202207L");
 
   if (LangOpts.Char8)
 Builder.defineMacro("__cpp_char8_t", "201811L");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134898: [Clang] define __cpp_named_character_escapes

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D134898#3825255 , @cor3ntin wrote:

> Thanks for catching that, I missed there was a feature test macro for it.
> It doesn't really make sense for it to be a feature test macro (given WG21's 
> current policies) so I'll make raise an issue about that but I'm happy to 
> approve the change in the meantime

Then I hope we won't have to revert this in two years or something :)

But in the meantime I agree this is what users will expect so it's better to 
define it than not to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134898

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

[clang] bd1bb8c - [Clang] define __cpp_named_character_escapes

[Roy Jacobson via cfe-commits]

Author: Roy Jacobson
Date: 2022-09-30T11:13:18+03:00
New Revision: bd1bb8cd42548d5e50e84f52a5098eec1ee92c98

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

LOG: [Clang] define __cpp_named_character_escapes

Define the feature test macro for named character escapes.
I assume this was not done because it was implemented before formally accepted, 
right? cxx_status says the paper is implemented.

Reviewed By: cor3ntin

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

Added: 


Modified: 
clang/lib/Frontend/InitPreprocessor.cpp
clang/test/Lexer/cxx-features.cpp

Removed: 




diff  --git a/clang/lib/Frontend/InitPreprocessor.cpp 
b/clang/lib/Frontend/InitPreprocessor.cpp
index 04ebeea522d65..e9bfab9e695be 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -697,10 +697,11 @@ static void InitializeCPlusPlusFeatureTestMacros(const 
LangOptions &LangOpts,
 Builder.defineMacro("__cpp_multidimensional_subscript", "202110L");
   }
 
-  // We provide this as an extension in earlier language modes, so we
-  // also define the macro.
+  // We provide those C++2b features as extensions in earlier language modes, 
so
+  // we also define their feature test macros.
   if (LangOpts.CPlusPlus11)
 Builder.defineMacro("__cpp_static_call_operator", "202207L");
+  Builder.defineMacro("__cpp_named_character_escapes", "202207L");
 
   if (LangOpts.Char8)
 Builder.defineMacro("__cpp_char8_t", "201811L");

diff  --git a/clang/test/Lexer/cxx-features.cpp 
b/clang/test/Lexer/cxx-features.cpp
index d1c9f0d2b2aee..ee52017a2201e 100644
--- a/clang/test/Lexer/cxx-features.cpp
+++ b/clang/test/Lexer/cxx-features.cpp
@@ -51,7 +51,7 @@
 #error "wrong value for __cpp_static_call_operator"
 #endif
 
-#if check(named_character_escapes, 0, 0, 0, 0, 0, 0)
+#if check(named_character_escapes, 202207, 202207, 202207, 202207, 202207, 
202207)
 #error "wrong value for __cpp_named_character_escapes"
 #endif
 



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

[PATCH] D134898: [Clang] define __cpp_named_character_escapes

[Roy Jacobson 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 rGbd1bb8cd4254: [Clang] define __cpp_named_character_escapes 
(authored by royjacobson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134898

Files:
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/Lexer/cxx-features.cpp


Index: clang/test/Lexer/cxx-features.cpp
===
--- clang/test/Lexer/cxx-features.cpp
+++ clang/test/Lexer/cxx-features.cpp
@@ -51,7 +51,7 @@
 #error "wrong value for __cpp_static_call_operator"
 #endif
 
-#if check(named_character_escapes, 0, 0, 0, 0, 0, 0)
+#if check(named_character_escapes, 202207, 202207, 202207, 202207, 202207, 
202207)
 #error "wrong value for __cpp_named_character_escapes"
 #endif
 
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -697,10 +697,11 @@
 Builder.defineMacro("__cpp_multidimensional_subscript", "202110L");
   }
 
-  // We provide this as an extension in earlier language modes, so we
-  // also define the macro.
+  // We provide those C++2b features as extensions in earlier language modes, 
so
+  // we also define their feature test macros.
   if (LangOpts.CPlusPlus11)
 Builder.defineMacro("__cpp_static_call_operator", "202207L");
+  Builder.defineMacro("__cpp_named_character_escapes", "202207L");
 
   if (LangOpts.Char8)
 Builder.defineMacro("__cpp_char8_t", "201811L");


Index: clang/test/Lexer/cxx-features.cpp
===
--- clang/test/Lexer/cxx-features.cpp
+++ clang/test/Lexer/cxx-features.cpp
@@ -51,7 +51,7 @@
 #error "wrong value for __cpp_static_call_operator"
 #endif
 
-#if check(named_character_escapes, 0, 0, 0, 0, 0, 0)
+#if check(named_character_escapes, 202207, 202207, 202207, 202207, 202207, 202207)
 #error "wrong value for __cpp_named_character_escapes"
 #endif
 
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -697,10 +697,11 @@
 Builder.defineMacro("__cpp_multidimensional_subscript", "202110L");
   }
 
-  // We provide this as an extension in earlier language modes, so we
-  // also define the macro.
+  // We provide those C++2b features as extensions in earlier language modes, so
+  // we also define their feature test macros.
   if (LangOpts.CPlusPlus11)
 Builder.defineMacro("__cpp_static_call_operator", "202207L");
+  Builder.defineMacro("__cpp_named_character_escapes", "202207L");
 
   if (LangOpts.Char8)
 Builder.defineMacro("__cpp_char8_t", "201811L");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133413: [clang-tidy] Fix crashes on `if consteval` in readability checks

[Emilia Dreamer via Phabricator via cfe-commits]

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

Thank you for pointing those out, I was indeed able to cause crashes from those 
locations


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133413

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

[PATCH] D133413: [clang-tidy] Fix crashes on `if consteval` in readability checks

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel updated this revision to Diff 464176.
rymiel added a comment.

Extra consteval checks in SimplifyBooleanExprCheck


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133413

Files:
  clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-consteval-if.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-cxx23.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-cxx23.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/simplify-bool-expr-cxx23.cpp
@@ -0,0 +1,37 @@
+// RUN: clang-tidy %s -checks='-*,readability-simplify-boolean-expr' -- -std=c++2b | count 0
+template 
+constexpr int testIf() {
+  if consteval {
+if constexpr (Cond) {
+  return 0;
+} else {
+  return 1;
+}
+  } else {
+return 2;
+  }
+}
+
+constexpr bool testCompound() {
+  if consteval {
+return true;
+  }
+  return false;
+}
+
+constexpr bool testCase(int I) {
+  switch (I) {
+case 0: {
+  if consteval {
+return true;
+  }
+  return false;
+}
+default: {
+  if consteval {
+return false;
+  }
+  return true;
+}
+  }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-consteval-if.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/braces-around-statements-consteval-if.cpp
@@ -0,0 +1,31 @@
+// RUN: clang-tidy %s -checks='-*,readability-braces-around-statements' -- -std=c++2b | count 0
+
+constexpr void handle(bool) {}
+
+constexpr void shouldPass() {
+  if consteval {
+handle(true);
+  } else {
+handle(false);
+  }
+}
+
+constexpr void shouldPassNegated() {
+  if !consteval {
+handle(false);
+  } else {
+handle(true);
+  }
+}
+
+constexpr void shouldPassSimple() {
+  if consteval {
+handle(true);
+  }
+}
+
+void run() {
+shouldPass();
+shouldPassNegated();
+shouldPassSimple();
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -149,6 +149,11 @@
   copy assignment operators with nonstandard return types. The check is restricted to
   c++11-or-later.
 
+- Fixed crashes in :doc:`readability-braces-around-statements
+  ` and
+  :doc:`readability-simplify-boolean-expr `
+  when using a C++23 ``if consteval`` statement.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -354,8 +354,9 @@
   }
 
   bool VisitIfStmt(IfStmt *If) {
-// Skip any if's that have a condition var or an init statement.
-if (If->hasInitStorage() || If->hasVarStorage())
+// Skip any if's that have a condition var or an init statement, or are
+// "if consteval" statements.
+if (If->hasInitStorage() || If->hasVarStorage() || If->isConsteval())
   return true;
 /*
  * if (true) ThenStmt(); -> ThenStmt();
@@ -467,7 +468,8 @@
  * if (Cond) return false; return true; -> return !Cond;
  */
 auto *If = cast(*First);
-if (!If->hasInitStorage() && !If->hasVarStorage()) {
+if (!If->hasInitStorage() && !If->hasVarStorage() &&
+!If->isConsteval()) {
   ExprAndBool ThenReturnBool =
   checkSingleStatement(If->getThen(), parseReturnLiteralBool);
   if (ThenReturnBool &&
@@ -491,7 +493,7 @@
 : cast(*First)->getSubStmt();
 auto *SubIf = dyn_cast(SubStmt);
 if (SubIf && !SubIf->getElse() && !SubIf->hasInitStorage() &&
-!SubIf->hasVarStorage()) {
+!SubIf->hasVarStorage() && !SubIf->isConsteval()) {
   ExprAndBool ThenReturnBool =
   checkSingleStatement(SubIf->getThen(), parseReturnLiteralBool);
   if (ThenReturnBool &&
Index: clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/BracesAroundStatementsCheck.cpp
@@ -131,6 +131,10 @@
   return;
 checkStmt(Result, S->getBody(), StartLoc);
   } else if (const auto *S = Result.Nodes.getNodeAs("if")) {
+   

[clang] 894c0e9 - Revert "[Clang] Fix variant crashes from GH58028, GH57370"

[Roy Jacobson via cfe-commits]

Author: Roy Jacobson
Date: 2022-09-30T11:24:46+03:00
New Revision: 894c0e94f9c62413feef88fd577c430839abaea7

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

LOG: Revert "[Clang] Fix variant crashes from GH58028, GH57370"

This reverts commit 9706bb3165f5e508d5e2247ad8a3f45077df546d, some CI workers 
complain about the test.

Added: 


Modified: 
clang/lib/Sema/SemaInit.cpp

Removed: 
clang/test/SemaCXX/specialization-diagnose-crash.cpp



diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index ee6fee0ac7324..7097b9deb8ed6 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -695,10 +695,10 @@ void InitListChecker::FillInEmptyInitForField(unsigned 
Init, FieldDecl *Field,
 //   member of reference type uninitialized, the program is
 //   ill-formed.
 SemaRef.Diag(Loc, diag::err_init_reference_member_uninitialized)
-<< Field->getType()
-<< (ILE->isSyntacticForm() ? ILE : ILE->getSyntacticForm())
-   ->getSourceRange();
-SemaRef.Diag(Field->getLocation(), diag::note_uninit_reference_member);
+  << Field->getType()
+  << ILE->getSyntacticForm()->getSourceRange();
+SemaRef.Diag(Field->getLocation(),
+ diag::note_uninit_reference_member);
   }
   hadError = true;
   return;

diff  --git a/clang/test/SemaCXX/specialization-diagnose-crash.cpp 
b/clang/test/SemaCXX/specialization-diagnose-crash.cpp
deleted file mode 100644
index ceb3c4655f745..0
--- a/clang/test/SemaCXX/specialization-diagnose-crash.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-// RUN: %clang_cc1 -fsyntax-only %s -verify
-// This is a reduction of GH57370 and GH58028, originally appearing
-// in libstdc++'s variant code.
-
-struct V1 {};
-struct V2 : V1 {
-  int &a;
-};
-
-template  using void_t = void;
-
-template  struct X { T x; };
-
-template  struct Variant {
-  Variant() = delete; // expected-note {{deleted here}}
-};
-
-template 
-struct Variant{T1()})>> {};
-
-void f() {
-  Variant();
-  Variant(); // expected-error {{call to deleted constructor}}
-}



___
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 updated this revision to Diff 464179.

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

https://reviews.llvm.org/D134699

Files:
  clang/lib/AST/Interp/ByteCodeEmitter.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Disasm.cpp
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/Function.cpp
  clang/lib/AST/Interp/Function.h
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/InterpFrame.cpp
  clang/lib/AST/Interp/InterpFrame.h
  clang/lib/AST/Interp/Pointer.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -102,3 +102,24 @@
   return &c;
 }
 static_assert(getPointer()->a == 100, "");
+
+constexpr C RVOAndParams(const C *c) {
+  return C();
+}
+constexpr C RVOAndParamsResult = RVOAndParams(&c);
+
+constexpr int locals() {
+  C c;
+  c.a = 10;
+
+  // Assignment, not an initializer.
+  // c = C(); FIXME
+  c.a = 10;
+
+
+  // Assignment, not an initializer.
+  //c = RVOAndParams(&c); FIXME
+
+  return c.a;
+}
+static_assert(locals() == 10, "");
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -317,7 +317,7 @@
 
   /// Prints the pointer.
   void print(llvm::raw_ostream &OS) const {
-OS << "{" << Base << ", " << Offset << ", ";
+OS << (void *)Pointee << "{" << Base << ", " << Offset << ", ";
 if (Pointee)
   OS << Pointee->getSize();
 else
Index: clang/lib/AST/Interp/InterpFrame.h
===
--- clang/lib/AST/Interp/InterpFrame.h
+++ clang/lib/AST/Interp/InterpFrame.h
@@ -35,6 +35,11 @@
   InterpFrame(InterpState &S, Function *Func, InterpFrame *Caller,
   CodePtr RetPC, Pointer &&This);
 
+  /// Creates a new frame with the values that make sense.
+  /// I.e., the caller is the current frame of S,
+  /// and the This() pointer is the current Pointer on the top of S's stack,
+  InterpFrame(InterpState &S, Function *Func, CodePtr RetPC);
+
   /// Destroys the frame, killing all live pointers to stack slots.
   ~InterpFrame();
 
Index: clang/lib/AST/Interp/InterpFrame.cpp
===
--- clang/lib/AST/Interp/InterpFrame.cpp
+++ clang/lib/AST/Interp/InterpFrame.cpp
@@ -36,6 +36,36 @@
   }
 }
 
+InterpFrame::InterpFrame(InterpState &S, Function *Func, CodePtr RetPC)
+: Caller(S.Current), S(S), Func(Func), RetPC(RetPC),
+  ArgSize(Func ? Func->getArgSize() : 0),
+  Args(static_cast(S.Stk.top())), FrameOffset(S.Stk.size()) {
+  assert(Func);
+
+  // As per our calling convention, the this pointer is
+  // part of the ArgSize.
+  // If the function has RVO, the RVO pointer is first.
+  // If the fuction has a This pointer, that one is next.
+  // Then follow the actual arguments (but those are handled
+  // in getParamPointer()).
+  if (Func->hasThisPointer()) {
+if (Func->hasRVO())
+  This = stackRef(sizeof(Pointer));
+else
+  This = stackRef(0);
+  }
+
+  if (unsigned FrameSize = Func->getFrameSize()) {
+Locals = std::make_unique(FrameSize);
+for (auto &Scope : Func->scopes()) {
+  for (auto &Local : Scope.locals()) {
+Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
+B->invokeCtor();
+  }
+}
+  }
+}
+
 InterpFrame::~InterpFrame() {
   if (Func && Func->isConstructor() && This.isBaseClass())
 This.initialize();
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -55,8 +55,7 @@
 
 template ::T>
 static bool Call(InterpState &S, CodePtr &PC, const Function *Func) {
-  S.Current =
-  new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+  S.Current = new InterpFrame(S, const_cast(Func), PC);
   APValue CallResult;
   // Note that we cannot assert(CallResult.hasValue()) here since
   // Ret() above only sets the APValue if the curent frame doesn't
@@ -66,8 +65,7 @@
 
 static bool CallVoid(InterpState &S, CodePtr &PC, const Function *Func) {
   APValue VoidResult;
-  S.Current =
-  new InterpFrame(S, const_cast(Func), S.Current, PC, {});
+  S.Current = new InterpFrame(S, const_cast(Func), PC);
   bool Success = Interpret(S, VoidResult);
   assert(VoidResult.isAbsent());
 
Index: clang/lib/AST/Interp/Function.h
===
--- clang/lib/AST/Interp/Function.h
+++ clang/lib/AST/Interp/Function.h
@@ -56,6 +56,21 @@
 ///
 /// Contains links to the bytecode of the function, as well as metadata
 /// describing all arguments and stack-local variables.
+///
+/// # Cal

[PATCH] D134941: [analyzer][NFC] Add tests for D132236

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

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

D132236  would have introduced regressions in 
the symbol lifetime
handling. However, the testsuite did not catch this, so here we have
some tests, which would have break if D132236 
 had landed.

This patch addresses the comment https://reviews.llvm.org/D132236#3753238

Co-authored-by: Balazs Benics 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134941

Files:
  clang/test/Analysis/NewDeleteLeaks.cpp
  clang/test/Analysis/symbol-reaper-lambda.cpp


Index: clang/test/Analysis/symbol-reaper-lambda.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-reaper-lambda.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+template 
+void escape(Ts&...);
+struct Dummy {};
+
+int strange(Dummy param) {
+  Dummy local_pre_lambda;
+  int ref_captured = 0;
+
+  auto fn = [&] {
+escape(param, local_pre_lambda);
+return ref_captured; // no-warning: The value is not garbage.
+  };
+
+  int local_defined_after_lambda; // Unused, but necessary! Important that 
it's before the call.
+  return fn();
+}
\ No newline at end of file
Index: clang/test/Analysis/NewDeleteLeaks.cpp
===
--- clang/test/Analysis/NewDeleteLeaks.cpp
+++ clang/test/Analysis/NewDeleteLeaks.cpp
@@ -192,3 +192,26 @@
 // expected-note@-1 {{Potential leak of memory pointed to by 'v'}}
 
 } // namespace refkind_from_unoallocated_to_allocated
+
+namespace symbol_reaper_lifetime {
+struct Nested {
+  int buf[2];
+};
+struct Wrapping {
+  Nested data;
+};
+
+Nested allocateWrappingAndReturnNested() {
+  // expected-note@+1 {{Memory is allocated}}
+  Wrapping const* p = new Wrapping();
+  // expected-warning@+2 {{Potential leak of memory pointed to by 'p'}}
+  // expected-note@+1{{Potential leak of memory pointed to by 'p'}}
+  return p->data;
+}
+
+void caller() {
+  // expected-note@+1 {{Calling 'allocateWrappingAndReturnNested'}}
+  Nested n = allocateWrappingAndReturnNested();
+  (void)n;
+} // no-warning: No potential memory leak here, because that's been already 
reported.
+} // namespace symbol_reaper_lifetime


Index: clang/test/Analysis/symbol-reaper-lambda.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-reaper-lambda.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+template 
+void escape(Ts&...);
+struct Dummy {};
+
+int strange(Dummy param) {
+  Dummy local_pre_lambda;
+  int ref_captured = 0;
+
+  auto fn = [&] {
+escape(param, local_pre_lambda);
+return ref_captured; // no-warning: The value is not garbage.
+  };
+
+  int local_defined_after_lambda; // Unused, but necessary! Important that it's before the call.
+  return fn();
+}
\ No newline at end of file
Index: clang/test/Analysis/NewDeleteLeaks.cpp
===
--- clang/test/Analysis/NewDeleteLeaks.cpp
+++ clang/test/Analysis/NewDeleteLeaks.cpp
@@ -192,3 +192,26 @@
 // expected-note@-1 {{Potential leak of memory pointed to by 'v'}}
 
 } // namespace refkind_from_unoallocated_to_allocated
+
+namespace symbol_reaper_lifetime {
+struct Nested {
+  int buf[2];
+};
+struct Wrapping {
+  Nested data;
+};
+
+Nested allocateWrappingAndReturnNested() {
+  // expected-note@+1 {{Memory is allocated}}
+  Wrapping const* p = new Wrapping();
+  // expected-warning@+2 {{Potential leak of memory pointed to by 'p'}}
+  // expected-note@+1{{Potential leak of memory pointed to by 'p'}}
+  return p->data;
+}
+
+void caller() {
+  // expected-note@+1 {{Calling 'allocateWrappingAndReturnNested'}}
+  Nested n = allocateWrappingAndReturnNested();
+  (void)n;
+} // no-warning: No potential memory leak here, because that's been already reported.
+} // namespace symbol_reaper_lifetime
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134885: [Clang] Fix variant crashes from GH58028, GH57370

[Roy Jacobson via Phabricator via cfe-commits]

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

Apparently some of the workers crashed with the test - 
https://lab.llvm.org/buildbot/#/builders/216/builds/10556, but I couldn't 
reproduce this locally. @shafik any idea why the diagnostics might change? :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134885

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

[David Spickett via Phabricator via cfe-commits]

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

Please mark comments as done if you feel that they have been addressed. (I know 
this can be a bit weird, do you mark them or do I, I go with the former, I can 
disagree if needed)

Lack of docs right now is fine, we have enough to know this exists and it's 
quite simple.

With my one final comment addressed, this LGTM.




Comment at: clang/lib/Driver/Driver.cpp:1384
+  TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec) {
+if (UArgs->hasArg(options::OPT__SLASH_arm64EC)) {
+  getDiags().Report(clang::diag::warn_target_override_arm64ec)

DavidSpickett wrote:
> This would read better to me if you put them all in one and put the most 
> important check first like:
> ```
> if ( UArgs->hasArg(options::OPT__SLASH_arm64EC) &&
> ( (TC.getTriple().getArch() != llvm::Triple::aarch64 ||
>   TC.getTriple().getSubArch() != llvm::Triple::AArch64SubArch_arm64ec)) {
> ```
This is good but I didn't emphasise one bit. Putting the arm64ec option check 
first saves reading the rest if the reader knows it's not relevant.


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] D134941: [analyzer][NFC] Add tests for D132236

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

tomasz-kaminski-sonarsource updated this revision to Diff 464184.
tomasz-kaminski-sonarsource added a comment.

Added new line


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134941

Files:
  clang/test/Analysis/NewDeleteLeaks.cpp
  clang/test/Analysis/symbol-reaper-lambda.cpp


Index: clang/test/Analysis/symbol-reaper-lambda.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-reaper-lambda.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+template 
+void escape(Ts&...);
+struct Dummy {};
+
+int strange(Dummy param) {
+  Dummy local_pre_lambda;
+  int ref_captured = 0;
+
+  auto fn = [&] {
+escape(param, local_pre_lambda);
+return ref_captured; // no-warning: The value is not garbage.
+  };
+
+  int local_defined_after_lambda; // Unused, but necessary! Important that 
it's before the call.
+  return fn();
+}
+
Index: clang/test/Analysis/NewDeleteLeaks.cpp
===
--- clang/test/Analysis/NewDeleteLeaks.cpp
+++ clang/test/Analysis/NewDeleteLeaks.cpp
@@ -192,3 +192,26 @@
 // expected-note@-1 {{Potential leak of memory pointed to by 'v'}}
 
 } // namespace refkind_from_unoallocated_to_allocated
+
+namespace symbol_reaper_lifetime {
+struct Nested {
+  int buf[2];
+};
+struct Wrapping {
+  Nested data;
+};
+
+Nested allocateWrappingAndReturnNested() {
+  // expected-note@+1 {{Memory is allocated}}
+  Wrapping const* p = new Wrapping();
+  // expected-warning@+2 {{Potential leak of memory pointed to by 'p'}}
+  // expected-note@+1{{Potential leak of memory pointed to by 'p'}}
+  return p->data;
+}
+
+void caller() {
+  // expected-note@+1 {{Calling 'allocateWrappingAndReturnNested'}}
+  Nested n = allocateWrappingAndReturnNested();
+  (void)n;
+} // no-warning: No potential memory leak here, because that's been already 
reported.
+} // namespace symbol_reaper_lifetime


Index: clang/test/Analysis/symbol-reaper-lambda.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-reaper-lambda.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+template 
+void escape(Ts&...);
+struct Dummy {};
+
+int strange(Dummy param) {
+  Dummy local_pre_lambda;
+  int ref_captured = 0;
+
+  auto fn = [&] {
+escape(param, local_pre_lambda);
+return ref_captured; // no-warning: The value is not garbage.
+  };
+
+  int local_defined_after_lambda; // Unused, but necessary! Important that it's before the call.
+  return fn();
+}
+
Index: clang/test/Analysis/NewDeleteLeaks.cpp
===
--- clang/test/Analysis/NewDeleteLeaks.cpp
+++ clang/test/Analysis/NewDeleteLeaks.cpp
@@ -192,3 +192,26 @@
 // expected-note@-1 {{Potential leak of memory pointed to by 'v'}}
 
 } // namespace refkind_from_unoallocated_to_allocated
+
+namespace symbol_reaper_lifetime {
+struct Nested {
+  int buf[2];
+};
+struct Wrapping {
+  Nested data;
+};
+
+Nested allocateWrappingAndReturnNested() {
+  // expected-note@+1 {{Memory is allocated}}
+  Wrapping const* p = new Wrapping();
+  // expected-warning@+2 {{Potential leak of memory pointed to by 'p'}}
+  // expected-note@+1{{Potential leak of memory pointed to by 'p'}}
+  return p->data;
+}
+
+void caller() {
+  // expected-note@+1 {{Calling 'allocateWrappingAndReturnNested'}}
+  Nested n = allocateWrappingAndReturnNested();
+  (void)n;
+} // no-warning: No potential memory leak here, because that's been already reported.
+} // namespace symbol_reaper_lifetime
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134942: [Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens implementation.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: clang.

The code falls back to the pre-2011 partition-file-id solution (see for
details ).

This patch simplifies/rewrites the code based on the partition-based-on-file-id
idea. The new implementation is optimized by reducing the number of
calling getFileID (~40% drop).

Despite the huge drop of getFileID, this is a marignal improvment on
speed (becase the number of calling non-cached getFileID is roughly
the same). It removes the evaluation-order performance gap between 
gcc-built-clang
and clang-built-clang.

SemaExpr.cpp:

- before: 315063 SLocEntries, FileID scans: 388230 linear, 1393437 binary. 
458893 cache hits, 672299 getFileID calls
- after:  313494 SLocEntries, FileID scans: 397525 linear, 1451890 binary, 
176714 cache hits, 397144 getFileID calls

FindTarget.cpp:

- before: 279984 SLocEntries, FileID scans: 361926 linear, 1275930 binary, 
436072 cache hits, 632150 getFileID calls
- after:  278426 SLocEntries, FileID scans: 371279 linear, 1333963 binary, 
153705 cache hits, 356814 getFileID calls


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134942

Files:
  clang/lib/Lex/TokenLexer.cpp

Index: clang/lib/Lex/TokenLexer.cpp
===
--- clang/lib/Lex/TokenLexer.cpp
+++ clang/lib/Lex/TokenLexer.cpp
@@ -25,6 +25,7 @@
 #include "clang/Lex/Token.h"
 #include "clang/Lex/VariadicMacroSupport.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/iterator_range.h"
@@ -989,60 +990,55 @@
 Token * end_tokens) {
   assert(begin_tokens < end_tokens);
 
-  SourceLocation FirstLoc = begin_tokens->getLocation();
-  SourceLocation CurLoc = FirstLoc;
-
-  // Compare the source location offset of tokens and group together tokens that
-  // are close, even if their locations point to different FileIDs. e.g.
-  //
-  //  |bar|  foo | cake   |  (3 tokens from 3 consecutive FileIDs)
-  //  ^^
-  //  |bar   foo   cake| (one SLocEntry chunk for all tokens)
-  //
-  // we can perform this "merge" since the token's spelling location depends
-  // on the relative offset.
-
-  Token *NextTok = begin_tokens + 1;
-  for (; NextTok < end_tokens; ++NextTok) {
-SourceLocation NextLoc = NextTok->getLocation();
-if (CurLoc.isFileID() != NextLoc.isFileID())
-  break; // Token from different kind of FileID.
-
-SourceLocation::IntTy RelOffs;
-if (!SM.isInSameSLocAddrSpace(CurLoc, NextLoc, &RelOffs))
-  break; // Token from different local/loaded location.
-// Check that token is not before the previous token or more than 50
-// "characters" away.
-if (RelOffs < 0 || RelOffs > 50)
-  break;
-
-if (CurLoc.isMacroID() && !SM.isWrittenInSameFile(CurLoc, NextLoc))
-  break; // Token from a different macro.
-
-CurLoc = NextLoc;
+  SourceLocation BeginLoc = begin_tokens->getLocation();
+  llvm::MutableArrayRef All(begin_tokens, end_tokens);
+  llvm::MutableArrayRef Partition;
+
+  // Partition the tokens by their FileID.
+  // This is a hot function, and calling getFileID can be expensive, the
+  // implementation is optimized by reducing the number of getFileID.
+  if (BeginLoc.isFileID()) {
+// We can avoid calling getFileID at all for *file* consecutive tokens --
+// for this case it is impossible to switch between files in marco arguments
+// (e.g. we can't use #include in marco arguments, and clang rejects it),
+// therefore these token must point to the same file.
+Partition = All.take_while([&](const Token &T) {
+  return T.getLocation().isFileID();
+});
+assert(!Partition.empty() &&
+   llvm::all_of(Partition.drop_front(),
+[&SM](const Token &T) {
+  return SM.getFileID((&T - 1)->getLocation()) ==
+ SM.getFileID(T.getLocation());
+}) &&
+   "Must have the same FIleID!");
+  } else {
+// Call getFileID once to calculate the bounds, and use the cheaper
+// sourcelocation-against-bounds comparison.
+FileID BeginFID = SM.getFileID(BeginLoc);
+SourceLocation Limit =
+SM.getComposedLoc(BeginFID, SM.getFileIDSize(BeginFID));
+Partition = All.take_while([&](const Token &T) {
+  return T.getLocation() >= BeginLoc && T.getLocation() < Limit;
+});
   }
-
   // For the consecutive tokens, find the length of the SLocEntry to contain
   // all of them.
-  Token &LastConsecutiveTok = *(NextTok-1);
-  SourceLocation::IntTy LastRelOffs = 0;
-  SM.isInSameSLocAddrSpace(FirstLoc, LastConsecutiveTok.getLocation(),
-   &LastRelOffs);
   Source

[PATCH] D134115: [clang] Store in exprs the deduced arguments for function calls.

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 464190.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134115

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/Sema/Initialization.h
  clang/include/clang/Sema/Overload.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/Analysis/BodyFarm.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaCUDA.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprMember.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp

Index: clang/lib/Serialization/ASTWriterStmt.cpp
===
--- clang/lib/Serialization/ASTWriterStmt.cpp
+++ clang/lib/Serialization/ASTWriterStmt.cpp
@@ -621,7 +621,8 @@
   if ((!E->hasTemplateKWAndArgsInfo()) && (!E->hasQualifier()) &&
   (E->getDecl() == E->getFoundDecl()) &&
   nk == DeclarationName::Identifier &&
-  !E->refersToEnclosingVariableOrCapture() && !E->isNonOdrUse()) {
+  !E->refersToEnclosingVariableOrCapture() && !E->isNonOdrUse() &&
+  !E->getConvertedArgs()) {
 AbbrevToUse = Writer.getDeclRefExprAbbrev();
   }
 
@@ -636,6 +637,10 @@
  E->getTrailingObjects());
 
   Record.AddDeclRef(E->getDecl());
+  if (E->ConvertedArgs)
+Record.AddTemplateArgumentList(E->ConvertedArgs);
+  else
+Record.push_back(0);
   Record.AddSourceLocation(E->getLocation());
   Record.AddDeclarationNameLoc(E->DNLoc, E->getDecl()->getDeclName());
   Code = serialization::EXPR_DECL_REF;
@@ -911,6 +916,10 @@
   Record.push_back(E->hadMultipleCandidates());
   Record.push_back(E->isNonOdrUse());
   Record.AddSourceLocation(E->getOperatorLoc());
+  if (E->Deduced)
+Record.AddTemplateArgumentList(E->Deduced);
+  else
+Record.push_back(0);
 
   if (HasFoundDecl) {
 DeclAccessPair FoundDecl = E->getFoundDecl();
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -2311,6 +2311,7 @@
   Abv->Add(BitCodeAbbrevOp(0)); // RefersToEnclosingVariableOrCapture
   Abv->Add(BitCodeAbbrevOp(0)); // NonOdrUseReason
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // DeclRef
+  Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // ConvertedArgs
   Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Location
   DeclRefExprAbbrev = Stream.EmitAbbrev(std::move(Abv));
 
Index: clang/lib/Serialization/ASTReaderStmt.cpp
===
--- clang/lib/Serialization/ASTReaderStmt.cpp
+++ clang/lib/Serialization/ASTReaderStmt.cpp
@@ -595,6 +595,7 @@
   E->DeclRefExprBits.HadMultipleCandidates = Record.readInt();
   E->DeclRefExprBits.RefersToEnclosingVariableOrCapture = Record.readInt();
   E->DeclRefExprBits.NonOdrUseReason = Record.readInt();
+
   unsigned NumTemplateArgs = 0;
   if (E->hasTemplateKWAndArgsInfo())
 NumTemplateArgs = Record.readInt();
@@ -612,6 +613,7 @@
 E->getTrailingObjects(), NumTemplateArgs);
 
   E->D = readDeclAs();
+  E->ConvertedArgs = Record.readTemplateArgumentList();
   E->setLocation(readSourceLocation());
   E->DNLoc = Record.readDeclarationNameLoc(E->getDecl()->getDeclName());
 }
@@ -1027,6 +1029,7 @@
   E->MemberExprBits.HadMultipleCandidates = Record.readInt();
   E->MemberExprBits.NonOdrUseReason = Record.readInt();
   E->MemberExprBits.OperatorLoc = Record.readSourceLocation();
+  E->Deduced = Record.readTemplateArgumentList();
 
   if (HasQualifier || HasFoundDecl) {
 DeclAccessPair FoundDecl;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -8844,6 +8844,15 @@
 TemplArgs.push_back(readTemplateArgument(Canonicalize));
 }
 
+const TemplateArgumentList *
+ASTRecordReader::readTemplateArgumentList(bool Canonicalize) {
+  SmallVector Args;
+  readTemplateArgumentList(Args, Canonicalize);
+  if (Args.size() == 0)
+return nullptr;
+  return TemplateArgumentList::CreateCopy(getContext(), Args);
+}
+
 /// Read a UnresolvedSet structure.
 void ASTRecordReader::readUnresolvedSet(LazyASTUnresolvedSet &Set) {
   unsigned NumDecls = readInt();
Index: clang/lib/Sema/TreeTransform.h

[PATCH] D134143: [clang] Misc type sugar preservation improvements

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov updated this revision to Diff 464191.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134143

Files:
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaType.cpp

Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -9157,6 +9157,56 @@
   return Context.getTypeOfExprType(E);
 }
 
+static QualType getSameReferencedType(ASTContext &Context, QualType VT,
+  QualType ET) {
+  assert(!ET->isReferenceType());
+  if (const auto *VTL = VT->getAs())
+ET = Context.getLValueReferenceType(ET, VTL->isSpelledAsLValue());
+  else if (VT->isRValueReferenceType())
+ET = Context.getRValueReferenceType(ET);
+
+  if (!Context.hasSameUnqualifiedType(ET, VT)) {
+ET.dump();
+VT.dump();
+assert(false && "!hasSameUnqualifiedType");
+  }
+
+  Qualifiers ToAdd = VT.getQualifiers(), ToRemove = ET.getQualifiers();
+  (void)Qualifiers::removeCommonQualifiers(ToAdd, ToRemove);
+
+  SplitQualType Split = ET.split();
+  while (!ToRemove.empty()) {
+(void)Qualifiers::removeCommonQualifiers(Split.Quals, ToRemove);
+if (ToRemove.empty())
+  break;
+QualType Next;
+switch (ET->getTypeClass()) {
+#define ABSTRACT_TYPE(Class, Parent)
+#define TYPE(Class, Parent)\
+  case Type::Class: {  \
+const auto *ty = cast(ET);\
+if (!ty->isSugared())  \
+  goto done;   \
+Next = ty->desugar();  \
+break; \
+  }
+#include "clang/AST/TypeNodes.inc"
+}
+Split = Next.split();
+  }
+done:
+  assert(ToRemove.empty());
+  Split.Quals += ToAdd;
+  ET = Context.getQualifiedType(Split);
+
+  if (!Context.hasSameType(ET, VT)) {
+ET.dump();
+VT.dump();
+assert(false && "!hasSameType");
+  }
+  return ET;
+}
+
 /// getDecltypeForExpr - Given an expr, will return the decltype for
 /// that expression, according to the rules in C++11
 /// [dcl.type.simple]p4 and C++11 [expr.lambda.prim]p18.
@@ -9189,18 +9239,20 @@
   // We apply the same rules for Objective-C ivar and property references.
   if (const auto *DRE = dyn_cast(IDExpr)) {
 const ValueDecl *VD = DRE->getDecl();
-QualType T = VD->getType();
+QualType T = getSameReferencedType(Context, VD->getType(), DRE->getType());
 return isa(VD) ? T.getUnqualifiedType() : T;
   }
   if (const auto *ME = dyn_cast(IDExpr)) {
 if (const auto *VD = ME->getMemberDecl())
   if (isa(VD) || isa(VD))
-return VD->getType();
+return getSameReferencedType(Context, VD->getType(), ME->getType());
   } else if (const auto *IR = dyn_cast(IDExpr)) {
+// FIXME: Sugar these. Breaks Modules/odr_hash.mm.
 return IR->getDecl()->getType();
   } else if (const auto *PR = dyn_cast(IDExpr)) {
 if (PR->isExplicitProperty())
-  return PR->getExplicitProperty()->getType();
+  return getSameReferencedType(
+  Context, PR->getExplicitProperty()->getType(), PR->getType());
   } else if (const auto *PE = dyn_cast(IDExpr)) {
 return PE->getType();
   }
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14381,8 +14381,24 @@
 if (isa(MD))
   Diag(OpLoc, diag::err_typecheck_addrof_dtor) << op->getSourceRange();
 
-QualType MPTy = Context.getMemberPointerType(
-op->getType(), Context.getTypeDeclType(MD->getParent()).getTypePtr());
+const CXXRecordDecl *Cls = MD->getParent();
+const Type *ClsType = nullptr;
+if (const NestedNameSpecifier *NNS = DRE->getQualifier()) {
+  const Type *Type = NNS->getAsType();
+  const CXXRecordDecl *ClsAsWritten =
+  Type ? Type->getAsCXXRecordDecl() : NNS->getAsRecordDecl();
+  assert(ClsAsWritten != nullptr);
+  if (declaresSameEntity(Cls, ClsAsWritten))
+ClsType =
+Type ? Type : Context.getTypeDeclType(ClsAsWritten).getTypePtr();
+  else
+// FIXME: Can we do better here?
+assert(ClsAsWritten->isDerivedFrom(Cls));
+}
+if (!ClsType)
+  ClsType = Context.getTypeDeclType(Cls).getTypePtr();
+
+QualType MPTy = Context.getMemberPointerType(op->getType(), ClsType);
 // Under the MS ABI, lock down the inheritance model now.
 if (Context.getTargetInfo().getCXXABI().isMicrosoft())
   (void)isCompleteType(OpLoc, MPTy);
Index: clang/lib/Sema/SemaCXXScopeSpec.cpp
=

[PATCH] D132941: [DO NOT SUBMIT] [Sema] Delay evaluation of std::source_location::current() in default arguments

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

I think this is superseded by https://cplusplus.github.io/CWG/issues/2631.html 
and its resolution.
Which I'm looking into implementing - I'll let you know if I manage to get it 
working (I still have a number of failing tests). 
The idea is the same though, we keep a rewritten Expr in `CXXDefaultArgExpr`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132941

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

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

>>> @hokein or I will try to find time to take a stab at this.
>>
>> Awesome, please keep me in the loop.
>
> Will do!

https://reviews.llvm.org/D134942 is my attempt.

In D20401#2770059 , @nickdesaulniers 
wrote:

>> I discussed this bug with Argyrios off-list, who lgtm'd on the condition 
>> that it doesn't introduce a performance regression.
>
> Well, I'd say it's a performance regression, though perhaps reported 5 years 
> too late.

This patch does increase the number of SLocEntries. At least for SemaExpr.cpp, 
prior this patch it is ~298K, after this patch it is ~315K (~5% increase).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D20401

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

[PATCH] D134733: [clang-format][chore] transparent #include name regex

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk updated this revision to Diff 464196.
kwk added a comment.

- Make include regex a static member and not a function
- Run clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134733

Files:
  clang/include/clang/Tooling/Inclusions/HeaderIncludes.h
  clang/lib/Format/Format.cpp
  clang/lib/Tooling/Inclusions/HeaderIncludes.cpp

Index: clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
===
--- clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
+++ clang/lib/Tooling/Inclusions/HeaderIncludes.cpp
@@ -169,13 +169,6 @@
   });
 }
 
-inline StringRef trimInclude(StringRef IncludeName) {
-  return IncludeName.trim("\"<>");
-}
-
-const char IncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
-
 // The filename of Path excluding extension.
 // Used to match implementation with headers, this differs from sys::path::stem:
 //  - in names with multiple dots (foo.cu.cc) it terminates at the *first*
@@ -266,6 +259,16 @@
   return false;
 }
 
+static const char IncludeRegexPattern[] =
+R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
+/// IncludeRegex ia regex that can match various styles of C++ includes.
+/// For example:
+/// \code
+/// #include 
+/// #include "bar.h"
+/// \endcode
+const llvm::Regex HeaderIncludes::IncludeRegex(IncludeRegexPattern);
+
 HeaderIncludes::HeaderIncludes(StringRef FileName, StringRef Code,
const IncludeStyle &Style)
 : FileName(FileName), Code(Code), FirstIncludeOffset(-1),
@@ -274,8 +277,7 @@
   MaxInsertOffset(MinInsertOffset +
   getMaxHeaderInsertionOffset(
   FileName, Code.drop_front(MinInsertOffset), Style)),
-  Categories(Style, FileName),
-  IncludeRegex(llvm::Regex(IncludeRegexPattern)) {
+  Categories(Style, FileName) {
   // Add 0 for main header and INT_MAX for headers that are not in any
   // category.
   Priorities = {0, INT_MAX};
@@ -289,11 +291,12 @@
   SmallVector Matches;
   for (auto Line : Lines) {
 NextLineOffset = std::min(Code.size(), Offset + Line.size() + 1);
-if (IncludeRegex.match(Line, &Matches)) {
+if (tooling::HeaderIncludes::IncludeRegex.match(Line, &Matches)) {
+  StringRef IncludeName = tooling::getIncludeNameFromMatches(Matches);
   // If this is the last line without trailing newline, we need to make
   // sure we don't delete across the file boundary.
   addExistingInclude(
-  Include(Matches[2],
+  Include(IncludeName,
   tooling::Range(
   Offset, std::min(Line.size() + 1, Code.size() - Offset))),
   NextLineOffset);
@@ -403,5 +406,18 @@
   return Result;
 }
 
+llvm::StringRef getIncludeNameFromMatches(
+const llvm::SmallVectorImpl &Matches) {
+  if (Matches.size() >= 3) {
+return Matches[2];
+  }
+  llvm_unreachable("Matches is too small");
+  return llvm::StringRef();
+}
+
+llvm::StringRef trimInclude(llvm::StringRef IncludeName) {
+  return IncludeName.trim("\"<>;");
+}
+
 } // namespace tooling
 } // namespace clang
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -44,6 +44,7 @@
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/YAMLTraits.h"
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2769,13 +2770,6 @@
   }
 }
 
-namespace {
-
-const char CppIncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
-
-} // anonymous namespace
-
 tooling::Replacements sortCppIncludes(const FormatStyle &Style, StringRef Code,
   ArrayRef Ranges,
   StringRef FileName,
@@ -2785,7 +2779,6 @@
   .StartsWith("\xEF\xBB\xBF", 3) // UTF-8 BOM
   .Default(0);
   unsigned SearchFrom = 0;
-  llvm::Regex IncludeRegex(CppIncludeRegexPattern);
   SmallVector Matches;
   SmallVector IncludesInBlock;
 
@@ -2842,8 +2835,9 @@
 
 bool MergeWithNextLine = Trimmed.endswith("\\");
 if (!FormattingOff && !MergeWithNextLine) {
-  if (IncludeRegex.match(Line, &Matches)) {
-StringRef IncludeName = Matches[2];
+  if (tooling::HeaderIncludes::IncludeRegex.match(Line, &Matches)) {
+StringRef IncludeName = tooling::getIncludeNameFromMatches(Matches);
+
 if (Line.contains("/*") && !Line.contains("*/")) {
   // #include with a start of a block comment, but without the end.
   // Need to keep all the lines until the end of the comment together.
@@ -3120,8 +3114,8 @@
 
 inline bool isHeaderInsertion(const tooling::Replacement &Replace) {
   return Replace.getOffset() == UINT_MAX && Replace.getLength() == 0 &&
- llvm::Regex(CppIncludeRegexPatte

[PATCH] D134852: [clang-format][NFC] Clean up class HeaderIncludes and Format.cpp

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk added inline comments.



Comment at: clang/lib/Format/Format.cpp:2774
-
-const char CppIncludeRegexPattern[] =
-R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";

owenpan wrote:
> kwk wrote:
> > MyDeveloperDay wrote:
> > > Did I miss where this comes from now?
> > @MyDeveloperDay `clang/lib/Tooling/Inclusions/HeaderIncludes.cpp` still has 
> > this:
> > 
> > ```lang=c++
> > const char IncludeRegexPattern[] =
> > R"(^[\t\ ]*#[\t\ ]*(import|include)[^"<]*(["<][^">]*[">]))";
> > ```
> > 
> > And in `clang/lib/Tooling/Inclusions/HeaderIncludes.cpp` @owenpan uses it 
> > to initialize the final regex 
> > 
> > ```lang=c++
> > const llvm::Regex HeaderIncludes::IncludeRegex(IncludeRegexPattern); 
> > ```
> > 
> > The fact that we have two regex that are identical is an issue on its own 
> > that I tried to address with [my patch](https://reviews.llvm.org/D134733) 
> > as well. I didn't initialize the regex like @owenpan does here but I had a 
> > function to return it. Eventually a function makes it easier to apply the 
> > injection from a config file as you've suggested 
> > [here](https://reviews.llvm.org/D134733#3821957). So I favor my solution.
> > 
> > 
> > 
> > 
> > The fact that we have two regex that are identical is an issue on its own 
> > that I tried to address with [my patch](https://reviews.llvm.org/D134733) 
> > as well.
> 
> It should be addressed in a separate NFC patch such as this one.
> 
> > I didn't initialize the regex like @owenpan does here but I had a function 
> > to return it. Eventually a function makes it easier to apply the injection 
> > from a config file as you've suggested 
> > [here](https://reviews.llvm.org/D134733#3821957). So I favor my solution.
> 
> Making `IncludeRegex` a public static const member is one of the better 
> solutions when `IncludeRegexPattern` is fixed as it has been. If and when we 
> decide to support user specified patterns, we will make any necessary changes 
> then.
> > The fact that we have two regex that are identical is an issue on its own 
> > that I tried to address with [my patch](https://reviews.llvm.org/D134733) 
> > as well.
> 
> It should be addressed in a separate NFC patch such as this one.

Ehm, I thought I *did* create a separate NFC patch with D134733. I prefixed it 
with `[chore]` but that is as good as NFC. I can rename it if you want.  

> 
> > I didn't initialize the regex like @owenpan does here but I had a function 
> > to return it. Eventually a function makes it easier to apply the injection 
> > from a config file as you've suggested 
> > [here](https://reviews.llvm.org/D134733#3821957). So I favor my solution.
> 
> Making `IncludeRegex` a public static const member is one of the better 
> solutions when `IncludeRegexPattern` is fixed as it has been. If and when we 
> decide to support user specified patterns, we will make any necessary changes 
> then.

Okay, but you could have suggested that in D134733, no? I've made the change in 
D134733 here: https://reviews.llvm.org/D134733?vs=463205&id=464196#toc, so the 
regex is static const. But I've also outsourced the function for accessing the 
include name so the logic is at one place not scattered over and over and the 
trimming is also in its own function. Having everything going through that 
functions is easier for maintenance IMHO. Before I wondered why we had two 
include regex patterns (both the same btw.) and why an include name wasn't 
found when I had changed the `Matches[2]` to `Matches[3]` for example. That 
won't happen when its going through the function. You just change it in one 
place and not "plenty".

I hope we can agree that my change is now complete with additions from yours 
here. I most certainly don't want to disrupt your workflow and I apologize if I 
had. Unfortunately text can be misinterpreted way too much.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134852

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

[PATCH] D134652: [clang-format] Add Basic Carbon Support/Infrastructure to clang-format

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

HazardyKnusperkeks added inline comments.



Comment at: clang/lib/Format/UnwrappedLineParser.cpp:3671
   parseRecord();
-  // This does not apply to Java, JavaScript and C#.
+  // This does not apply to Java, JavaScript and C# or Carbon
   if (Style.Language == FormatStyle::LK_Java || Style.isJavaScript() ||




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

https://reviews.llvm.org/D134652

___
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 created this revision.
Herald added subscribers: steakhal, manas, ASDenysPetrov, martong, dkrupp, 
donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, szepet, baloghadamsoftware, 
xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
tomasz-kaminski-sonarsource requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

To illustrate our current understanding, let's start with the following
program:
https://godbolt.org/z/33f6vheh1

  void clang_analyzer_printState();
  
  struct C {
 int x;
 int y;
 int more_padding;
  };
  
  struct D {
 C c;
 int z;
  };
  
  C foo(D d, int new_x, int new_y) {
 d.c.x = new_x;   // B1
 assert(d.c.x < 13);  // C1
  
 C c = d.c;   // L
  
 assert(d.c.y < 10);  // C2
 assert(d.z < 5); // C3
  
 d.c.y = new_y;   // B2
  
 assert(d.c.y < 10);  // C4
  
 return c;  // R
  }

In the code, we create a few bindings to subregions of root region `d`
(`B1`, `B2`), a constrain on the values  (`C1`, `C2`, ….), and create a
`lazyCompoundVal` for the part of the region `d` at point `L`, which is
returned at point `R`.

Now, the question is which of these should remain live as long the
return value of the `foo` call is live. In perfect a word we should
preserve:

1. only the bindings of the subregions of `d.c`, which were created

before the copy at `L`. In our example, this includes `B1`, and not
`B2`. In other words, `new_x` should be live but `new_y` shouldn’t.

2. constraints on the values of `d.c`, that are reachable through `c`.

This can be created both before the point of making the copy (`L`) or
after. In our case, that would be `C1` and `C2`. But not `C3` (`d.z`
value is not reachable through `c`) and `C4` (the original value of
`d.c.y` was overridden at `B2` after creation of `c`).

The current code in the `RegionStore` covers the use case (1), by using
the `getInterestingValues()` to extract bindings to parts of the
referred region present in the store at the point of copy. This also
partially covers point (2), in case when constraints are applied to a
location that has binding at the point of the copy (in our case `d.c.x`
in `C1` that has value `new_x`), but it fails to preserve the
constraints that require creating a new symbol for location (`d.c.y` in
`C2`).

We introduce the concept of _lazily copied_ locations (regions) to the
`SymbolReaper`, i.e. for which a program can access the value stored at
that location, but not its address. These locations are constructed as a
set of regions referred to by `lazyCompoundVal`. A _readable_ location
(region) is a location that _live_ or _lazily copied_ . And symbols that
refer to values in regions are alive if the region is _readable_.

For simplicity, we follow the current approach to live regions and mark
the base region as _lazily copied_, and consider any subregions as
_readable_. This makes some symbols falsy live (`d.z` in our example)
and keeps the corresponding constraints alive.

The rename `Regions` to `LiveRegions` inside  `RegionStore` is NFC
change, that was done to make it clear, what is difference between
regions stored in this two sets.

Co-authored-by: Balazs Benics 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134947

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
  clang/test/Analysis/trivial-copy-struct.cpp

Index: clang/test/Analysis/trivial-copy-struct.cpp
===
--- clang/test/Analysis/trivial-copy-struct.cpp
+++ clang/test/Analysis/trivial-copy-struct.cpp
@@ -34,3 +34,28 @@
   (void)(n1->ptr);
   (void)(n2->ptr);
 }
+
+struct List {
+  List* next;
+};
+
+void ptr1(List* n) {
+  List* n2 = new List(*n); // cctor
+  if (!n->next) {
+if (n2->next) {
+  clang_analyzer_warnIfReached(); // unreachable
+}
+  }
+  delete n2;
+}
+
+void ptr2(List* n) {
+  List* n2 = new List(); // ctor
+  *n2 = *n; // assignment
+  if (!n->next) {
+if (n2->next) {
+  clang_analyzer_warnIfReached(); // unreachable
+}
+  }
+  delete n2;
+}
Index: clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
+++ clang/lib/StaticAnalyzer/Core/SymbolManager.cpp
@@ -411,10 +411,14 @@
 }
 
 void SymbolReaper::markLive(const MemRegion *region) {
-  RegionRoots.insert(region->getBaseRegion());
+  LiveRegionRoots.insert(region->getBaseRegion());
   markElementIndicesLive(region);
 }
 
+void SymbolReaper::markLazilyCopied(const clang::ento::MemRegion *region) {
+  LazilyCopiedRegionRoots.insert(region->getBaseRegion());
+}
+
 void SymbolReaper::markElementIndicesLive(const MemRegion *region) {
   for (auto SR = dyn_cast(region); SR;
SR = dyn_

[PATCH] D111000: [clang-format] allow clang-format to be passed a file of filenames so we can add a regression suite of "clean clang-formatted files" from LLVM

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

HazardyKnusperkeks added a comment.

One does not need to remove this, when adding response file support.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111000

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

[PATCH] D132236: [analyzer] Fix liveness of LazyCompoundVals

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

steakhal abandoned this revision.
steakhal added a comment.

I'm abandoning this change in favor of D134947 
.
I'll leave the patch summary and the discussion here for the history.

---

In D132236#3753238 , @NoQ wrote:

>> For FPs I dont know how to automate this process. :/
>
> Given that our initial hypothesis was that there should be zero new false 
> positives, it could probably work with a creduce predicate //"this code has a 
> warning after the patch but not before the patch"//. The initial hypothesis 
> is probably not entirely correct, given that even a slight change in coverage 
> could result in false positives, but I doubt that if you pick 4-5 examples, 
> all of them would reduce to a situation where it's just a change of coverage. 
> Especially given how common you say the problem appears to be. So I think 
> it's worth a try.

D134941  addresses this comment. I'm also 
about to investigate other cases with differences - but no promises about that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132236

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

[PATCH] D134453: Introduce the `AlwaysIncludeTypeForNonTypeTemplateArgument` into printing policy

[Nenad Mikša via Phabricator via cfe-commits]

DoDoENT added a comment.

> Right - I'm trying to understand what your overall goals are and what clang 
> needs to facilitate them, if it's suitable to do so. And it seems to me that 
> the right way to support them, is for your approach to change - not to rely 
> on the type printing to communicate certain information, but to use the AST 
> to get the information you need (& if you want to encode that in a string, 
> that's up to you - I'd suggest encoding it in some more semantic data 
> structure (a struct, with variables, lists/vectors, etc, describing the 
> properties you want - so you don't have to rely on what clang does/doesn't 
> include in the text and have to parse that information out of the text later 
> on with your own home-grown C++ type parsing code, I guess)).

I've got a machine-generated c++ function in the form:

  c++
  template< typename Pixel >
  auto processImage( Image< Pixel > const & input )
  {
  auto result0{ processor.step1( input ) };
  auto result1{ processor.step2( result0 ) };
  ...
  auto resultN{ processor.stepNplus1( resultNminus1, resultK, resultM, ... 
) }; // for K, M, ... < N - 1
  return resultN;
  }

The `processor` is a graph processor that has steps that perform node 
computations (imagine it as neural network layers in machine learning 
inference).

Now, this works fine, but produces unnecessarily large binary, as types of most 
results don't depend on the `Pixel` template, but get instantiated anyway, thus 
making both binary larger and source slower to compile.

The idea of my tool is to use a clang to obtain concrete types of every 
`result0, result1, ..., resultN` and then decide whether its type depends on 
`Pixel` or not. Then use that information to cut the function into two 
functions - one that contains part of processing that depends on `Pixel` and 
the one that does not. The one that does not will be a normal function, while 
the one that does will remain in the function template and will call to the 
other after performing part of processing that depends on `Pixel`.

So, my idea is to obtain the types of each result as strings and then simply 
search for the keyword "Pixel"` in those strings to classify each result type. 
This approach works pretty well with my patch. Also, I find that much easier 
than navigating the AST.

But in general, you both are right - this tool could possibly be implemented 
differently and probably will in the future.

Now, let's keep our focus on verbosity in the diagnostic outputs, as this was 
the main reason I contributed this patch back to the upstream LLVM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134453

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

[PATCH] D132941: [DO NOT SUBMIT] [Sema] Delay evaluation of std::source_location::current() in default arguments

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a subscriber: rsmith.
ilya-biryukov added a comment.

In D132941#3826398 , @cor3ntin wrote:

> I think this is superseded by 
> https://cplusplus.github.io/CWG/issues/2631.html and its resolution.
> Which I'm looking into implementing - I'll let you know if I manage to get it 
> working (I still have a number of failing tests). 
> The idea is the same though, we keep a rewritten Expr in `CXXDefaultArgExpr`.

LG, thanks for taking this over.
We chatted with @rsmith about this and one of the seemingly important 
optimizations that Richard suggested is to try avoiding the extra rewriting of 
expressions that don't have any immediate function calls.
Not sure how important this ends being for performance of compiling real code, 
but may be worth exploring.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132941

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

[PATCH] D132236: [analyzer] Fix liveness of LazyCompoundVals

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

tomasz-kaminski-sonarsource added a comment.

> What looks fishy about getInterestingValues() is that it assumes that the 
> amount of interesting values is finite. This sounds incredibly wrong to me. 
> If a lazy compound value contains any pointer symbol `$p`, then all values in 
> the following infinite series are interesting:
>
>   $p,  *$p,  **$p,  ***$p,  ...

We have also looked into this, and indeed the `getInterestingValues()` produces 
the first level of indirection for storage. However, the code in 
`RemoveDeadBindingsWorker` is recursively visiting each of the bindings found, 
so, at least per our understanding, we should visit all regions that are 
reachable through indirection:

  const RegionStoreManager::SValListTy &Vals = RM.getInterestingValues(*LCS);
  for (RegionStoreManager::SValListTy::const_iterator I = Vals.begin(),
  E = Vals.end();
   I != E; ++I)
VisitBinding(*I);

Also, from the temporal perspective, visiting these regions in a current 
snapshot of the storage seems correct from the temporal perspective - we can 
reach current state in region, via the pointer to it, that was present at the 
time of copy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132236

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

[PATCH] D133413: [clang-tidy] Fix crashes on `if consteval` in readability checks

[Nathan James via Phabricator via cfe-commits]

njames93 accepted this revision.
njames93 added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133413

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

[PATCH] D131963: [libc++] Add custom clang-tidy checks

[Jonathan Wakely via Phabricator via cfe-commits]

jwakely added inline comments.



Comment at: libcxx/test/tools/clang_tidy_checks/robust_against_adl.cpp:22
+AST_MATCHER(clang::UnresolvedLookupExpr, isCustomizationPoint) {
+  // TODO: Are make_error_code and make_error_condition actually customization 
points?
+  return std::ranges::any_of(

jwakely wrote:
> Mordante wrote:
> > ldionne wrote:
> > > This is funny, I actually saw a LWG issue about that go by a few weeks 
> > > ago. I'll try to get more details.
> > @ldionne @jwakely posted this bug report about it 
> > https://github.com/llvm/llvm-project/issues/57614
> They're definitely customization points. If they weren't, specializing 
> `is_error_code_enum` and `is_error_condition_enum` would be completely 
> useless and serve no purpose.
> 
> LWG confirmed that and [LWG 3629](https://wg21.link/lwg3629) is Tentatively 
> Ready now.
I created https://reviews.llvm.org/D134943 to fix those customization points.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131963

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

[PATCH] D134652: [clang-format] Add Basic Carbon Support/Infrastructure to clang-format

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

@HazardyKnusperkeks thank you, any thoughts from you or others on if you feel 
its ok for me to continue?


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

https://reviews.llvm.org/D134652

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

[PATCH] D134650: [runtimes] Remove all traces of the legacy testing configuration system

[Louis Dionne via Phabricator via cfe-commits]

ldionne updated this revision to Diff 464234.
ldionne added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134650

Files:
  clang/cmake/caches/CrossWinToARMLinux.cmake
  libcxx/cmake/caches/MinGW.cmake
  libcxx/docs/BuildingLibcxx.rst
  libcxx/test/CMakeLists.txt
  libcxx/test/configs/legacy.cfg.in
  libcxx/utils/ci/buildkite-pipeline.yml
  libcxx/utils/ci/run-buildbot
  libcxx/utils/libcxx/compiler.py
  libcxx/utils/libcxx/test/config.py
  libcxx/utils/libcxx/test/target_info.py
  libcxx/utils/libcxx/util.py
  libcxxabi/test/CMakeLists.txt
  libcxxabi/test/libcxxabi/__init__.py
  libcxxabi/test/libcxxabi/test/__init__.py
  libcxxabi/test/libcxxabi/test/config.py
  libcxxabi/test/lit.site.cfg.in
  libunwind/CMakeLists.txt
  libunwind/test/CMakeLists.txt
  libunwind/test/configs/cmake-bridge.cfg.in
  libunwind/test/libunwind/__init__.py
  libunwind/test/libunwind/test/__init__.py
  libunwind/test/libunwind/test/config.py
  libunwind/test/lit.site.cfg.in

Index: libunwind/test/lit.site.cfg.in
===
--- libunwind/test/lit.site.cfg.in
+++ /dev/null
@@ -1,61 +0,0 @@
-@AUTO_GEN_COMMENT@
-
-@SERIALIZED_LIT_PARAMS@
-
-import os
-import site
-
-config.cxx_under_test   = "@CMAKE_CXX_COMPILER@"
-config.project_obj_root = "@CMAKE_BINARY_DIR@"
-config.install_root = "@CMAKE_BINARY_DIR@"
-config.libunwind_src_root   = "@LIBUNWIND_SOURCE_DIR@"
-config.libunwind_obj_root   = "@LIBUNWIND_BINARY_DIR@"
-config.abi_library_root = "@LIBUNWIND_LIBRARY_DIR@"
-config.libcxx_src_root  = "@LIBUNWIND_LIBCXX_PATH@"
-config.libunwind_headers= "@LIBUNWIND_SOURCE_DIR@/include"
-config.cxx_library_root = "@LIBUNWIND_LIBCXX_LIBRARY_PATH@"
-config.llvm_unwinder= True
-config.builtins_library = "@LIBUNWIND_BUILTINS_LIBRARY@"
-config.enable_threads   = @LIBUNWIND_ENABLE_THREADS@
-config.target_info  = "@LIBUNWIND_TARGET_INFO@"
-config.test_linker_flags= "@LIBUNWIND_TEST_LINKER_FLAGS@"
-config.test_compiler_flags  = "@LIBUNWIND_TEST_COMPILER_FLAGS@"
-config.executor = "@LIBUNWIND_EXECUTOR@"
-config.libunwind_shared = @LIBUNWIND_ENABLE_SHARED@
-config.enable_shared= @LIBCXX_ENABLE_SHARED@
-config.arm_ehabi= @LIBUNWIND_USES_ARM_EHABI@
-config.host_triple  = "@LLVM_HOST_TRIPLE@"
-config.sysroot  = "@CMAKE_SYSROOT@"
-config.gcc_toolchain= "@CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN@"
-config.cxx_ext_threads  = @LIBUNWIND_BUILD_EXTERNAL_THREAD_LIBRARY@
-config.x86_cet  = @LIBUNWIND_ENABLE_CET@
-
-site.addsitedir(os.path.join(config.libunwind_src_root, 'test'))
-site.addsitedir(os.path.join(config.libcxx_src_root, 'utils'))
-
-# name: The name of this test suite.
-config.name = 'libunwind'
-
-# suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.cpp', '.s']
-
-# test_source_root: The root path where tests are located.
-config.test_source_root = os.path.join(config.libunwind_src_root, 'test')
-
-# Allow expanding substitutions that are based on other substitutions
-config.recursiveExpansionLimit = 10
-
-# Infer the test_exec_root from the build directory.
-config.test_exec_root = os.path.join(config.libunwind_obj_root, 'test')
-
-import libcxx.test.format
-config.test_format = libcxx.test.format.CxxStandardLibraryTest()
-
-lit_config.note('Using configuration variant: libunwind')
-import libunwind.test.config
-configuration = libunwind.test.config.Configuration(lit_config, config)
-configuration.configure()
-configuration.print_config_info()
-
-lit_config.warning("This is a legacy testing configuration which will be removed in LLVM 16. "
-   "Please use one of the configurations in libunwind/test/configs or define your own.")
Index: libunwind/test/libunwind/test/config.py
===
--- libunwind/test/libunwind/test/config.py
+++ /dev/null
@@ -1,71 +0,0 @@
-#===--===##
-#
-# 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
-#
-#===--===##
-import os
-import sys
-
-from libcxx.test.config import Configuration as LibcxxConfiguration
-
-
-class Configuration(LibcxxConfiguration):
-# pylint: disable=redefined-outer-name
-def __init__(self, lit_config, config):
-super(Configuration, self).__init__(lit_config, config)
-self.libunwind_src_root = None
-self.libunwind_obj_root = None
-self.abi_library_root = None
-self.libcxx_s

[PATCH] D134650: [runtimes] Remove all traces of the legacy testing configuration system

[Louis Dionne via Phabricator via cfe-commits]

ldionne added inline comments.



Comment at: clang/cmake/caches/CrossWinToARMLinux.cmake:163
   set(RUNTIMES_${TOOLCHAIN_TARGET_TRIPLE}_COMPILER_RT_EMULATOR
 "\\\"${Python3_EXECUTABLE}\\\" 
\\\"${LLVM_PROJECT_DIR}/llvm/utils/remote-exec.py\\\" --execdir %%T 
--exec-pattern='.*\\.c.*\\.tmp.*' 
--host=${REMOTE_TEST_USER}@${REMOTE_TEST_HOST}"
 CACHE STRING "")

vvereschaka wrote:
> @ldionne `%%T` must be replaced with `%T` for some reason; otherwise we get 
> the errors during test execution with these differential changes. Everything 
> else works fine. Thank you.
That sounds extremely surprising to me, but I'll do that. Thanks a lot for 
checking!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134650

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

[PATCH] D134453: Introduce the `AlwaysIncludeTypeForNonTypeTemplateArgument` into printing policy

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134453#3825902 , @dblaikie wrote:

> Again, I'm not advocating for the printing as-is, I think adding the top 
> level name that disambiguates would be a good thing - and I think the GCC 
> and MSVC examples somewhat show why adding all the other layers would be 
> harmful to readability - there's a lot of text in those messages that 
> doesn't directly help the user and gets in the way of identifying the 
> important differences between the type names.

 I think this is a matter of taste. In the example that you've shown, I 
 personally prefer the verbosity of GCC and don't see it as "less 
 readable", but as "more informative". However, I do understand that some 
 people may prefer the output you suggested. What about making this 
 configurable, i.e. behind some clang flag, so that developers that prefer 
 verbosity can enable that?
>>>
>>> I don't think that's the right choice for clang - though I'm not the only 
>>> decider here. Be great to hear from @aaron.ballman at some point.
>>>
>>> My perspective on this issue at the moment is that we should fix any case 
>>> where we print an ambiguous type (so `template struct t1;` should 
>>> never produce `t1<{}>` and should instnead produce `t1`) - but I 
>>> don't think extra {} should be added where the language doesn't require it 
>>> and where it isn't necessary to disambiguate.
>>
>> My thinking is along a somewhat different line of approach. Clang has 
>> `-ast-print` which is intended to pretty print the AST back to the original 
>> source code without losing information. It is a `-cc1` option and not a 
>> driver option (though we do document it a little bit here 
>> https://releases.llvm.org/15.0.0/tools/clang/docs/HowToSetupToolingForLLVM.html),
>>  so it's not something we expose trivially to users. This option has always 
>> been aspirational -- we'd love it to pretty print back to the original 
>> source, but we know there are plenty of cases where we don't manage it and 
>> we typically don't require people to maintain it except superficially. Now 
>> that we have clang-format, I think its utility is largely theoretical in 
>> some ways. Despite its known issues, I am aware of at least some users who 
>> make use of the functionality (for various reasons) and I am not 100% 
>> convinced we'll get community agreement to drop support for it as a failed 
>> experiment despite my personal feelings that the functionality is more 
>> trouble than it's worth, especially because I'm reasonably certain some 
>> other functionality relies on it (ARC rewriting maybe?) and we'd have to 
>> figure out what to do with that.
>>
>> So my thinking is: if we're going to have `-ast-print`, we should maintain 
>> it, and part of maintaining it means having the ability for it to print this 
>> code back to compilable source with the same semantics as the original 
>> source. Our current behavior with the example is... not particularly 
>> fantastic: https://godbolt.org/z/88eK3q869. Alternatively, we could remove 
>> `-ast-print` as an option and narrow its focus to only the internal uses we 
>> need it for.
>
> Hmm - that seems to be to be somewhat of a tangent, though? I think the claim 
> that `template struct t1; struct t2 { }; t1 v1;` and then 
> rendering `t1<{}>` in a diagnostic is wrong is solid, and we should fix that 
> for auto parameters to include the type info that'd be necessary in the 
> source code/to disambiguate. (I guess C++20 actually requires the name even 
> in non-ambiguous, non-auto cases - so I'd be OK leaning towards the syntax 
> requirement, even if it's not necessary to disambiguate (maybe there are 
> cases where it is?).
>
> You're suggesting that -ast-print should respect the type as-written by the 
> user, even? Not just "enough to be valid C++ code/disambiguate" - and that 
> mode might be enough to support @DoDoENT's use case using strings for type 
> information? (more addressing this feature request, less about the incidental 
> diagnostic quality bug report that's a side issue in this discussion)

I'm mostly looking at it from the perspective of "is it reasonable to add a new 
printing policy here and if so, what do we need from it" first and then "when 
is it reasonable to enable that printing policy". I think the presence of 
`-ast-print` answers the first question that we probably do need *some* sort of 
printing policy here because we believe we have at least two scenarios to 
support: print for a diagnostic and print for the pretty printer. We might even 
find we want a verbosity level instead of a boolean switch: print minimal type 
info, print "disambiguating" type info, print all type info, though it sort of 
sounds to me like we only need two modes (disambiguating mode for diagnostics 
and all type info for pretty printing/@DoDoENT).

(I think the original goal of `-ast-print` was to respec

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:108-111
+- The :doc:`readability-avoid-const-params-in-decls
+  ` check does not
+  warn about const value parameters in declarations inside macros anymore by
+  default.




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

https://reviews.llvm.org/D130130

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

[PATCH] D134671: [Driver] Prevent Mips specific code from claiming -mabi argument on other targets.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134671#3824868 , @MaskRay wrote:

> In D134671#3824672 , @aaron.ballman 
> wrote:
>
>> In D134671#3824644 , 
>> @nickdesaulniers wrote:
>>
>>> I don't think it's an issue for us to work around downstream, but this did 
>>> regress support for `-mabi=ms` used in UEFI related build scripts.
>>> https://github.com/ClangBuiltLinux/linux/issues/1725
>>> Noting it in case others find their way here via bisection. Thanks to 
>>> @nathanchance for the report.
>>
>> If that's intentional, should we call this out as a potentially breaking 
>> change in the release notes and post an announcement?
>
> I feel that this is still a minor issue. If we consider this as potentially 
> breaking change, then we'd add many many driver changes as potentially 
> breaking.
> I think we should wait for more evidence.

Perfect, thank you for verifying! (That's my intuition as well, but I was on 
the fence and wondering if my intuition was wrong.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134671

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

[PATCH] D134744: [clang][Interp] Implement rem opcode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/Interp.h:170
+
+  if (LHS.isSigned() && LHS.isMin() && RHS.isNegative() && RHS.isMinusOne()) {
+APSInt LHSInt = LHS.toAPSInt();

I really like how clear and generalized this predicate is!



Comment at: clang/lib/AST/Interp/Interp.h:164
+
+  if (RHS.isZero()) {
+const SourceInfo &Loc = S.Current->getSource(OpPC);

tbaeder wrote:
> shafik wrote:
> > You also need to catch when the result is not representable e.g `INT_MIN % 
> > -1`
> > 
> > see `CheckICE(...)`
> I added the check here some lines below; Is that alright? If so I'd add the 
> same code to the `div()` implementation.
Looks correct to me.


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

https://reviews.llvm.org/D134744

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

[PATCH] D134804: [clang][Interp] Implement bitwise Not operations

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D134804

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

[PATCH] D134801: [clang][Interp] Implement ConditionalOperators

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D134801

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

[PATCH] D134454: [Driver][Distro] Fix ArchLinux sysroot detection

[Adrian Ratiu via Phabricator via cfe-commits]

10ne1 added a comment.

In D134454#3826179 , @MaskRay wrote:

> In D134454#3826143 , @10ne1 wrote:
>
>> In D134454#3824571 , @MaskRay 
>> wrote:
>>
>>> I'll grab an Arch Linux machine for testing, but I don't think this is 
>>> currently in a form for submitting.
>>> This adds new functionality for non-MIPS and we need some fake file 
>>> hierarchies (like those used in `linux-cross.cpp`).
>>> I'll add the test, though, and submit this for you.
>>>
>>> Request changes for now.
>>
>> Ok. Thanks, please ping if you need any action on my side.
>
> My latest thought is that this patch is going toward a wrong direction: 
> https://reviews.llvm.org/D134454#3824630

To verify I understand correctly what you are suggesting: should we close this 
review and wait for distros like Arch to specify the sysroots via the new 
config files mechanism? (Arch is currently at LLVM 14.0.6, not sure when they 
will upgrade).

This will also block the kernel cleanups, but if @nickdesaulniers also agrees 
to postpone that kernel work, then ok, let's wait.


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

https://reviews.llvm.org/D134454

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

[PATCH] D134874: [Concepts] Fix Concepts on generic lambda in a VarTemplateSpecDecl

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 464238.
erichkeane marked 10 inline comments as done.
erichkeane added a comment.

Did @shafik s suggestions.  And yes, most of the work is just copy/pasted with 
slight changes to work in the new refactor, which does make it tougher to read, 
sorry about that :(


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

https://reviews.llvm.org/D134874

Files:
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/SemaTemplate/concepts-lambda.cpp

Index: clang/test/SemaTemplate/concepts-lambda.cpp
===
--- clang/test/SemaTemplate/concepts-lambda.cpp
+++ clang/test/SemaTemplate/concepts-lambda.cpp
@@ -13,3 +13,45 @@
   f();
   };
 }
+
+namespace GH57945_2 {
+  template
+concept c = true;
+
+  template
+auto f = [](auto... args) requires c  {
+};
+
+  template 
+  auto f2 = [](auto... args)
+requires (sizeof...(args) > 0)
+  {};
+
+  void g() {
+  f();
+  f2(5.0);
+  }
+}
+
+namespace GH57958 {
+  template concept C = true;
+  template constexpr bool v = [](C auto) { return true; }(0);
+  int _ = v<0>;
+}
+namespace GH57958_2 {
+  template concept C = true;
+  template constexpr bool v = [](C auto...) { return true; }(0);
+  int _ = v<0>;
+}
+
+namespace GH57971 {
+  template
+concept any = true;
+
+  template
+auto f = [](any auto) {
+};
+
+  using function_ptr = void(*)(int);
+  function_ptr ptr = f;
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -40,6 +40,191 @@
 // Template Instantiation Support
 //===--===/
 
+namespace {
+namespace TemplateInstArgsHelpers {
+struct Response {
+  const Decl *NextDecl = nullptr;
+  bool IsDone = false;
+  bool RelativeToPrimaryOff = true;
+  static Response Done() {
+Response R;
+R.IsDone = true;
+return R;
+  }
+  static Response ChangeDecl(const Decl *ND) {
+Response R;
+R.NextDecl = ND;
+return R;
+  }
+  static Response ChangeDecl(const DeclContext *Ctx) {
+Response R;
+R.NextDecl = Decl::castFromDeclContext(Ctx);
+return R;
+  }
+  static Response DontRelativeToPrimaryOff() {
+Response R;
+R.RelativeToPrimaryOff = false;
+return R;
+  }
+};
+// Add template arguments from a variable template instantiation. For a
+// class-scope explicit specialization, there are no template arguments
+// at this level, but there may be enclosing template arguments.
+Response
+HandleVarTemplateSpec(const VarTemplateSpecializationDecl *VarTemplSpec,
+  MultiLevelTemplateArgumentList &Result) {
+  if (VarTemplSpec->isClassScopeExplicitSpecialization())
+return Response::DontRelativeToPrimaryOff();
+
+  // We're done when we hit an explicit specialization.
+  if (VarTemplSpec->getSpecializationKind() == TSK_ExplicitSpecialization &&
+  !isa(VarTemplSpec))
+return Response::Done();
+
+  Result.addOuterTemplateArguments(
+  &VarTemplSpec->getTemplateInstantiationArgs());
+
+  // If this variable template specialization was instantiated from a
+  // specialized member that is a variable template, we're done.
+  assert(VarTemplSpec->getSpecializedTemplate() && "No variable template?");
+  llvm::PointerUnion
+  Specialized = VarTemplSpec->getSpecializedTemplateOrPartial();
+  if (VarTemplatePartialSpecializationDecl *Partial =
+  Specialized.dyn_cast()) {
+if (Partial->isMemberSpecialization())
+  return Response::Done();
+  } else {
+VarTemplateDecl *Tmpl = Specialized.get();
+if (Tmpl->isMemberSpecialization())
+  return Response::Done();
+  }
+  return Response::DontRelativeToPrimaryOff();
+}
+
+// If we have a template template parameter with translation unit context,
+// then we're performing substitution into a default template argument of
+// this template template parameter before we've constructed the template
+// that will own this template template parameter. In this case, we
+// use empty template parameter lists for all of the outer templates
+// to avoid performing any substitutions.
+Response
+HandleDefaultTempArgIntoTempTempParam(const TemplateTemplateParmDecl *TTP,
+  MultiLevelTemplateArgumentList &Result) {
+  for (unsigned I = 0, N = TTP->getDepth() + 1; I != N; ++I)
+Result.addOuterTemplateArguments(None);
+  return Response::Done();
+}
+
+// Add template arguments from a class template instantiation.
+Response
+HandleClassTemplateSpec(const ClassTemplateSpecializationDecl *ClassTemplSpec,
+MultiLev

[PATCH] D134928: [Sema] Don't treat a non-null template argument as if it were null.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Can you also add a release note for the fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134928

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

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Could you please rebase from `main` and upload patch here before commit?


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

https://reviews.llvm.org/D130130

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

[PATCH] D134958: [clang][Interp] Support __builtin_clz calls

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

I was wondering how to best implement builtin function calls, but I don't think 
actually generating bytecode for them makes much sense.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134958

Files:
  clang/lib/AST/CMakeLists.txt
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/lib/AST/Interp/Opcodes.td
  clang/test/AST/Interp/builtins.cpp

Index: clang/test/AST/Interp/builtins.cpp
===
--- /dev/null
+++ clang/test/AST/Interp/builtins.cpp
@@ -0,0 +1,8 @@
+// RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
+// RUN: %clang_cc1 -verify=ref %s
+
+// expected-no-diagnostics
+// ref-no-diagnostics
+
+
+static_assert(__builtin_clz(10) == 28, "");
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -169,6 +169,13 @@
   let HasCustomEval = 1;
 }
 
+/// Call a builtin function.
+def CallBI : Opcode {
+  let Args = [ArgUint32];
+  let Types = [];
+  let ChangesPC = 1;
+}
+
 //===--===//
 // Frame management
 //===--===//
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- /dev/null
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -0,0 +1,23 @@
+//===- InterpBuiltin.cpp - Interpret builtin functions --*- 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 "Integral.h"
+#include "InterpState.h"
+
+namespace clang {
+namespace interp {
+
+bool InterpretBuiltinClz(InterpState &S) {
+  auto Arg = S.Stk.pop>();
+  auto Result = Arg.countLeadingZeros();
+  S.Stk.push>(Integral<32, true>::from(Result));
+  return true;
+}
+
+} // namespace interp
+} // namespace clang
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -25,6 +25,7 @@
 #include "clang/AST/ASTDiagnostic.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/Expr.h"
+#include "clang/Basic/TargetBuiltins.h"
 #include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/Support/Endian.h"
@@ -91,6 +92,10 @@
 /// Checks if a method is pure virtual.
 bool CheckPure(InterpState &S, CodePtr OpPC, const CXXMethodDecl *MD);
 
+/// Prototypes for evaluation of builtin functions.
+/// Implemented in InterpBuiltin.cpp
+bool InterpretBuiltinClz(InterpState &S);
+
 template  inline bool IsTrue(const T &V) { return !V.isZero(); }
 
 //===--===//
@@ -1090,6 +1095,16 @@
   return true;
 }
 
+inline bool CallBI(InterpState &S, CodePtr OpPC, uint32_t ID) {
+  switch (ID) {
+  case Builtin::BI__builtin_clz:
+return InterpretBuiltinClz(S);
+
+  default:
+return false;
+  }
+}
+
 //===--===//
 // Read opcode arguments
 //===--===//
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -943,42 +943,56 @@
 
 template 
 bool ByteCodeExprGen::VisitCallExpr(const CallExpr *E) {
-  assert(!E->getBuiltinCallee() && "Builtin functions aren't supported yet");
-
   const Decl *Callee = E->getCalleeDecl();
   if (const auto *FuncDecl = dyn_cast_or_null(Callee)) {
-const Function *Func = getFunction(FuncDecl);
-if (!Func)
-  return false;
-// If the function is being compiled right now, this is a recursive call.
-// In that case, the function can't be valid yet, even though it will be
-// later.
-// If the function is already fully compiled but not constexpr, it was
-// found to be faulty earlier on, so bail out.
-if (Func->isFullyCompiled() && !Func->isConstexpr())
-  return false;
-
+unsigned BuiltinID = E->getBuiltinCallee();
 QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
 Optional T = classify(ReturnType);
-// Put arguments on the stack.
-for (const auto *Arg : E->arguments()) {
-  if (!this->visi

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: sammccall.
aaron.ballman added a comment.

The clangd test failure found by precommit CI is stumping me as to how to 
resolve it. @sammccall -- can you help me out? The issue is that the test is 
expecting no name but now we're printing more information, but that information 
includes a file path which may differ between machines.


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] D134958: [clang][Interp] Support __builtin_clz calls

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:16-19
+  auto Arg = S.Stk.pop>();
+  auto Result = Arg.countLeadingZeros();
+  S.Stk.push>(Integral<32, true>::from(Result));
+  return true;

This would also work as:

```
  auto Arg = S.Stk.pop();
  auto Result = __builtin_clz(Arg);
  S.Stk.push(Result);
  return true;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134958

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

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

[Thomas Etter via Phabricator via cfe-commits]

thomasetter updated this revision to Diff 464250.
thomasetter added a comment.

rebased


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

https://reviews.llvm.org/D130130

Files:
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
@@ -170,12 +170,17 @@
 void NF(const int* const*);
 void NF(alias_const_type);
 
-// Regression test for when the 'const' token is not in the code.
+// Regression tests involving macros, which are ignored by default.
 #define CONCAT(a, b) a##b
 void ConstNotVisible(CONCAT(cons, t) int i);
-// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
-// We warn, but we can't give a fix
-// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+void member_function(const int i);
+);
 
 // Regression test. We should not warn (or crash) on lambda expressions
 auto lambda_with_name = [](const int n) {};
Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s readability-avoid-const-params-in-decls %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-avoid-const-params-in-decls.IgnoreMacros, value: false}]}"
+
+// Regression tests involving macros
+#define CONCAT(a, b) a##b
+void ConstNotVisible(CONCAT(cons, t) int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+void member_function(const int i);
+);
+// CHECK-MESSAGES: :[[@LINE-2]]:26: warning: parameter 'i'
+// CHECK-FIXES: void member_function(int i);
Index: clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
@@ -15,3 +15,11 @@
 
   void f(const string);   // Bad: const is top level.
   void f(const string&);  // Good: const is not top level.
+
+Options
+---
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will not give warnings inside macros. Default
+   is `true`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -149,6 +149,11 @@
   copy assignment operators with nonstandard return types. The check is restricted to
   c++11-or-later.
 
+- The :doc:`readability-avoid-const-params-in-decls
+  ` check does not
+  warn about const value parameters in declarations inside macros anymore by
+  default.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
===
--- clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
+++ clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
@@ -20,13 +20,18 @@
 class AvoidConstParamsInDecls : public ClangTidyCheck {
 public:
   AvoidConstParamsInDecls(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   llvm::Optional getCheckTrav

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

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.

Thank you! Just wanted to ensure alphabetical order in `Changes in existing 
checks` :-)  Fine to merge.


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

https://reviews.llvm.org/D130130

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

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

[Thomas Etter via Phabricator via cfe-commits]

thomasetter updated this revision to Diff 464251.
thomasetter added a comment.

Applied review changes


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

https://reviews.llvm.org/D130130

Files:
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
@@ -170,12 +170,17 @@
 void NF(const int* const*);
 void NF(alias_const_type);
 
-// Regression test for when the 'const' token is not in the code.
+// Regression tests involving macros, which are ignored by default.
 #define CONCAT(a, b) a##b
 void ConstNotVisible(CONCAT(cons, t) int i);
-// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
-// We warn, but we can't give a fix
-// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+void member_function(const int i);
+);
 
 // Regression test. We should not warn (or crash) on lambda expressions
 auto lambda_with_name = [](const int n) {};
Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s readability-avoid-const-params-in-decls %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-avoid-const-params-in-decls.IgnoreMacros, value: false}]}"
+
+// Regression tests involving macros
+#define CONCAT(a, b) a##b
+void ConstNotVisible(CONCAT(cons, t) int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+void member_function(const int i);
+);
+// CHECK-MESSAGES: :[[@LINE-2]]:26: warning: parameter 'i'
+// CHECK-FIXES: void member_function(int i);
Index: clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
@@ -15,3 +15,11 @@
 
   void f(const string);   // Bad: const is top level.
   void f(const string&);  // Good: const is not top level.
+
+Options
+---
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will not give warnings inside macros. Default
+   is `true`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -149,6 +149,10 @@
   copy assignment operators with nonstandard return types. The check is restricted to
   c++11-or-later.
 
+- Change the default behavior of :doc:`readability-avoid-const-params-in-decls
+  ` to not
+  warn about `const` value parameters of declarations inside macros.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
===
--- clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
+++ clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
@@ -20,13 +20,18 @@
 class AvoidConstParamsInDecls : public ClangTidyCheck {
 public:
   AvoidConstParamsInDecls(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   llvm::Optiona

[PATCH] D134958: [clang][Interp] Support __builtin_clz calls

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

This seems right enough to me, though you  might consider CTZ as well since it 
is equally as easy.  A better/more useful attempt is going to be 
builtin_strlen.  Note that with builtins they are going to be particularly 
difficult because you can't execute them on the local machine, since we have to 
give the same result as the target machine.  Integer sizes can be a problem for 
that, but a lot of the builtins work differently based on which target you're 
running on.




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:954
+  assert(*T);
+  FuncDecl->dump();
+

Whats happening here?  Is this leftover debugging code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134958

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

[PATCH] D134853: [clang-format] Correctly annotate UDLs as OverloadedOperator

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

HazardyKnusperkeks added a comment.

Looks good. You could add a FIXME before the commented out tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134853

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

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

[Thomas Etter via Phabricator via cfe-commits]

thomasetter marked an inline comment as done.
thomasetter added a comment.

Thanks for the review!


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

https://reviews.llvm.org/D130130

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

[PATCH] D134958: [clang][Interp] Support __builtin_clz calls

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

tbaeder added a comment.

In D134958#3827024 , @erichkeane 
wrote:

> This seems right enough to me, though you  might consider CTZ as well since 
> it is equally as easy.  A better/more useful attempt is going to be 
> builtin_strlen.  Note that with builtins they are going to be particularly 
> difficult because you can't execute them on the local machine, since we have 
> to give the same result as the target machine.  Integer sizes can be a 
> problem for that, but a lot of the builtins work differently based on which 
> target you're running on.

Hmm, I see. Do you know of any existing tests in the clang code base that 
exercise those differences?




Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:954
+  assert(*T);
+  FuncDecl->dump();
+

erichkeane wrote:
> Whats happening here?  Is this leftover debugging code?
Oh, yep!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134958

___
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 reviewers: dexonsmith, dnovillo.
aaron.ballman added subscribers: dexonsmith, dnovillo.
aaron.ballman added a comment.

In D134456#3819185 , @rnk wrote:

> In D134456#3819042 , @aaron.ballman 
> wrote:
>
>> Alternatively, we could document that we don't follow the intent of the 
>> standard feature and that's just the way things are, and add a command line 
>> option to honor that intent. However, given that GCC honors the attribute 
>> over PGO, I think we should change our default behavior.
>
> That makes sense to me. I think it's reasonable to request folks to add a 
> flag in the context of a bug fix, but it would be too much to ask a drive by 
> contributor to go through the multi-step process to change the default flag 
> behavior.

I guess I view it differently -- I think users should get the correct behavior 
by default rather than have to opt into it, and I'm not certain that "it's a 
drive-by contribution" is a good driver of user experience decisions. Either 
way, I think we need an explicit decision as to which approach we think is 
*right* and go from there in terms of how to achieve that goal. (It'd be a bit 
different if I thought this patch was incremental progress, but from my current 
views on the topic, I actually think the patch is a further regression away 
from where we want to be.)

I'm adding in @dexonsmith as "branch weights" code owner and @dnovillo as 
"SampleProfile and related parts of ProfileData" code owner on the LLVM side to 
see if they have opinions on default behaviors (if there are other PGO experts 
to bring in, please sign them up).


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] D134958: [clang][Interp] Support __builtin_clz calls

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D134958#3827038 , @tbaeder wrote:

> In D134958#3827024 , @erichkeane 
> wrote:
>
>> This seems right enough to me, though you  might consider CTZ as well since 
>> it is equally as easy.  A better/more useful attempt is going to be 
>> builtin_strlen.  Note that with builtins they are going to be particularly 
>> difficult because you can't execute them on the local machine, since we have 
>> to give the same result as the target machine.  Integer sizes can be a 
>> problem for that, but a lot of the builtins work differently based on which 
>> target you're running on.
>
> Hmm, I see. Do you know of any existing tests in the clang code base that 
> exercise those differences?

I don't really know all that well... I don't spend much time in target-specific 
builtins. I think there are a couple of the floating-point and AVX type 
builtins that have differences between ARM and x86 though?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134958

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

[PATCH] D134958: [clang][Interp] Support __builtin_clz calls

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134958#3827038 , @tbaeder wrote:

> In D134958#3827024 , @erichkeane 
> wrote:
>
>> This seems right enough to me, though you  might consider CTZ as well since 
>> it is equally as easy.  A better/more useful attempt is going to be 
>> builtin_strlen.  Note that with builtins they are going to be particularly 
>> difficult because you can't execute them on the local machine, since we have 
>> to give the same result as the target machine.  Integer sizes can be a 
>> problem for that, but a lot of the builtins work differently based on which 
>> target you're running on.
>
> Hmm, I see. Do you know of any existing tests in the clang code base that 
> exercise those differences?

I'm not certain on existing tests, but the kinds of things I'm thinking about 
are builtins around floating-point (where the host and the target may be using 
different floating-point semantics entirely), builtin classification functions 
may be sensitive to compiler options like whether we honor nans or not, 
security builtins like `__builtin_add_overflow` which may behave differently on 
host and target depending on the size of variable-width types like `long` or 
`__builtin_strlen` whose return type is `size_t`, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134958

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

[PATCH] D130513: [Flang] Add -fconvert option to swap endianness for unformatted files

[Jean Perier via Phabricator via cfe-commits]

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

The lowering part looks good to me (I only have a minor comment inlined about a 
header used in lowering).




Comment at: flang/include/flang/Runtime/environment-defaults.h:19
+  std::string defaultValue;
+};
+

I think this header may better belong to Semantics (or even Lower since it is 
only used there) in the sense that it does not define a data structure that is 
used at runtime, but it defines a data structure so that we can keep track of 
some default environment value during compilation (It is not a huge deal, but I 
am a little bit wary of seeing std::string in Fortran::runtime while the 
runtime is meant to be free of the C++ runtime).


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

https://reviews.llvm.org/D130513

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

[PATCH] D132416: [Driver][Fuchsia] Add default linker flags

[Alex Brachet 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 rG5dfc8ebee5d6: [Driver][Fuchsia] Add default linker flags 
(authored by abrachet).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D132416?vs=454924&id=464266#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132416

Files:
  clang/lib/Driver/ToolChains/Fuchsia.cpp
  clang/test/Driver/fuchsia.c
  clang/test/Driver/fuchsia.cpp

Index: clang/test/Driver/fuchsia.cpp
===
--- clang/test/Driver/fuchsia.cpp
+++ clang/test/Driver/fuchsia.cpp
@@ -2,32 +2,42 @@
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-X86_64 %s
+// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-X86_64 %s
+// RUN: %clangxx -### %s --target=x86_64-unknown-fuchsia \
+// RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform -fuse-ld=gold 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LD,CHECK-X86_64 %s
 // RUN: %clangxx -### %s --target=aarch64-unknown-fuchsia \
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-AARCH64 %s
 // RUN: %clangxx -### %s --target=riscv64-unknown-fuchsia \
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-RISCV64 %s
 // RUN: %clangxx -### %s --target=x86_64-fuchsia \
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-X86_64 %s
 // RUN: %clangxx -### %s --target=aarch64-fuchsia \
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-AARCH64 %s
 // RUN: %clangxx -### %s --target=riscv64-fuchsia \
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-RISCV64 %s
 // CHECK: "-cc1"
 // CHECK-X86_64: "-triple" "x86_64-unknown-fuchsia"
 // CHECK-AARCH64: "-triple" "aarch64-unknown-fuchsia"
@@ -40,7 +50,8 @@
 // CHECK-RISCV64: "-internal-isystem" "{{.*[/\\]}}include{{/|}}riscv64-unknown-fuchsia{{/|}}c++{{/|}}v1"
 // CHECK: "-internal-isystem" "{{.*[/\\]}}include{{/|}}c++{{/|}}v1"
 // CHECK: "-internal-externc-isystem" "[[SYSROOT]]{{/|}}include"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments"
+// CHECK-LLD: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "rodynamic" "-z" "separate-loadable-segments" "-z" "rel" "--pack-dyn-relocs=relr"
+// CHECK-LD: {{.*}}ld.gold{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" "combreloc" "-z" "text"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"
Index: clang/test/Driver/fuchsia.c
===
--- clang/test/Driver/fuchsia.c
+++ clang/test/Driver/fuchsia.c
@@ -1,27 +1,35 @@
 // RUN: %clang -### %s --target=x86_64-unknown-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixe

[clang] 5dfc8eb - [Driver][Fuchsia] Add default linker flags

[Alex Brachet via cfe-commits]

Author: Alex Brachet
Date: 2022-09-30T14:07:41Z
New Revision: 5dfc8ebee5d688ee94607ccce655672c1a198b82

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

LOG: [Driver][Fuchsia] Add default linker flags

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/test/Driver/fuchsia.c
clang/test/Driver/fuchsia.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp 
b/clang/lib/Driver/ToolChains/Fuchsia.cpp
index 6f4fa2ce7c40a..2e47a96bcfe3f 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -65,6 +65,12 @@ void fuchsia::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
 CmdArgs.push_back("-z");
 CmdArgs.push_back("rel");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
+  } else {
+// The following are already the default in lld
+CmdArgs.push_back("-z");
+CmdArgs.push_back("combreloc");
+CmdArgs.push_back("-z");
+CmdArgs.push_back("text");
   }
 
   if (!D.SysRoot.empty())

diff  --git a/clang/test/Driver/fuchsia.c b/clang/test/Driver/fuchsia.c
index 099a88c2e4e36..00c763a140c30 100644
--- a/clang/test/Driver/fuchsia.c
+++ b/clang/test/Driver/fuchsia.c
@@ -1,27 +1,35 @@
 // RUN: %clang -### %s --target=x86_64-unknown-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-X86_64 %s
+// RUN: %clang -### %s --target=x86_64-unknown-fuchsia \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-X86_64 %s
+// RUN: %clang -### %s --target=x86_64-unknown-fuchsia \
+// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
+// RUN: --sysroot=%S/platform -fuse-ld=gold 2>&1 \
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LD,CHECK-X86_64 %s
 // RUN: %clang -### %s --target=aarch64-unknown-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-AARCH64 %s
 // RUN: %clang -### %s --target=riscv64-unknown-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-RISCV64 %s
 // RUN: %clang -### %s --target=x86_64-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-X86_64 %s
 // RUN: %clang -### %s --target=aarch64-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-AARCH64 %s
 // RUN: %clang -### %s --target=riscv64-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-RISCV64 %s
 // CHECK: "-cc1"
 // CHECK-X86_64: "-triple" "x86_64-unknown-fuchsia"
 // CHECK-AARCH64: "-triple" "aarch64-unknown-fuchsia"
@@ -36,7 +44,8 @@
 // CHECK: "-stack-protector" "2"
 // CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
-// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
+// CHECK-LLD: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
+// CHECK-LD: {{.*}}ld.gold{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"combreloc" "-z" "text"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"

diff  --git a/clang/test/Driver/fuchsia.cpp b/clang/test/Driver/fuchsia.cpp
index e5640f5826271..e247c76dabdd7 100644
--- a/clang/test/Driver/fuchsia.cpp
+++ b/clang/test/Driver/fuchsia.cpp
@@ -2,32 +2,42 @@
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resource-dir=%S/

[PATCH] D134831: [Clang][Sema] Add -Wcast-function-type-strict

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:229-231
+- Introduced ``-Wcast-function-type-strict`` to warn about function type 
mismatches
+  in casts that may result in runtime indirect call `Control-Flow Integrity 
(CFI)
+  `_ failures.





Comment at: clang/test/Sema/warn-cast-function-type-strict.c:30
+  g = (f7 *)x; /* expected-warning {{cast from 'int (*)(long)' to 'f7 *' (aka 
'int (*)(long, ...)') converts to incompatible function type}} */
+}

samitolvanen wrote:
> aaron.ballman wrote:
> > Some other test cases I think we should try out:
> > ```
> > typedef int (f8)(int *);
> > typedef int (f9)(const int);
> > typedef int (f10)(int);
> > 
> > int foo(int array[static 12]);
> > int bar(int i);
> > const int baz(int i);
> > 
> > f8 *h = (f8 *)foo; // Should be okay, types are the same after adjustment
> > f9 *i = (f9 *)bar; // Should be okay, types are the same after adjustment
> > f10 *j = (f10 *)baz; // Should be okay, types are the same after adjustment
> > ```
> The first two seem to be OK, the last one does produce a warning here:
> ```
> cast from 'const int (*)(int)' to 'f10 *' (aka 'int (*)(int)') converts to 
> incompatible function type
> ```
Oh yeah, that's right, the C standard is pretty weird here. The return type is 
required to be compatible (aka same type in this case) per C2x 6.7.6.3p14, and 
`int` and `const int` are not compatible types (C2x 6.7.3p11). However, the 
qualifier on the return type is useless because it's stripped when the function 
is called (C2x 6.5.2.2p5, 6.8.6.4p3, 6.5.16p3, 6.3.2.1p2).

Compilers are wildly inconsistent about this: https://godbolt.org/z/hc6ordGeM



Comment at: clang/test/SemaCXX/warn-cast-function-type-strict.cpp:1
+// RUN: %clang_cc1 -x c++ %s -fblocks -fsyntax-only 
-Wcast-function-type-strict -triple x86_64-- -verify
+

aaron.ballman wrote:
> Same question about triples here.
You should remove the `-x c++` from the RUN line still.



Comment at: clang/test/SemaCXX/warn-cast-function-type.cpp:1
-// RUN: %clang_cc1 -x c++ %s -fblocks -fsyntax-only -Wcast-function-type 
-triple x86_64-- -verify
+// RUN: %clang_cc1 -x c++ %s -fblocks -fsyntax-only -Wcast-function-type 
-Wno-cast-function-type-strict -verify
 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134831

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

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

dexonsmith added a comment.

In D134456#3827040 , @aaron.ballman 
wrote:

> In D134456#3819185 , @rnk wrote:
>
>> In D134456#3819042 , 
>> @aaron.ballman wrote:
>>
>>> Alternatively, we could document that we don't follow the intent of the 
>>> standard feature and that's just the way things are, and add a command line 
>>> option to honor that intent. However, given that GCC honors the attribute 
>>> over PGO, I think we should change our default behavior.
>>
>> That makes sense to me. I think it's reasonable to request folks to add a 
>> flag in the context of a bug fix, but it would be too much to ask a drive by 
>> contributor to go through the multi-step process to change the default flag 
>> behavior.
>
> I guess I view it differently -- I think users should get the correct 
> behavior by default rather than have to opt into it, and I'm not certain that 
> "it's a drive-by contribution" is a good driver of user experience decisions. 
> Either way, I think we need an explicit decision as to which approach we 
> think is *right* and go from there in terms of how to achieve that goal. 
> (It'd be a bit different if I thought this patch was incremental progress, 
> but from my current views on the topic, I actually think the patch is a 
> further regression away from where we want to be.)
>
> I'm adding in @dexonsmith as "branch weights" code owner and @dnovillo as 
> "SampleProfile and related parts of ProfileData" code owner on the LLVM side 
> to see if they have opinions on default behaviors (if there are other PGO 
> experts to bring in, please sign them up).

The design of PGO was to use user hints when there’s no coverage, but basically 
ignore them if theres enough data to say otherwise.

This safety scenario sounds like it could differ within a file. Is a flag 
really the right control? Maybe what you want is a `[[noprofile]]`, similar to 
`[[noopt]]`, which selectively disables the profile where the user wants 
fine-grained control to ignore the profile.


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] D134942: [Lex] Simplify and cleanup the updateConsecutiveMacroArgTokens implementation.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Some more perf data on building linux kernel (x86_64)

before: getFileID 2.4% (1.10% on `getFileIDSlow`)
after: getFileID 2.35% (1.05% on `getFileIDSlow`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134942

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

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9a4e52ebeb6d: [clang-tidy] Add an IgnoreMacros option to 
readability-avoid-const-params-in… (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130130

Files:
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
  clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp
@@ -170,12 +170,17 @@
 void NF(const int* const*);
 void NF(alias_const_type);
 
-// Regression test for when the 'const' token is not in the code.
+// Regression tests involving macros, which are ignored by default.
 #define CONCAT(a, b) a##b
 void ConstNotVisible(CONCAT(cons, t) int i);
-// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
-// We warn, but we can't give a fix
-// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+void member_function(const int i);
+);
 
 // Regression test. We should not warn (or crash) on lambda expressions
 auto lambda_with_name = [](const int n) {};
Index: clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
@@ -0,0 +1,22 @@
+// RUN: %check_clang_tidy %s readability-avoid-const-params-in-decls %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-avoid-const-params-in-decls.IgnoreMacros, value: false}]}"
+
+// Regression tests involving macros
+#define CONCAT(a, b) a##b
+void ConstNotVisible(CONCAT(cons, t) int i);
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstNotVisible(CONCAT(cons, t) int i);
+
+#define CONST_INT_PARAM const int i
+void ConstInMacro(CONST_INT_PARAM);
+// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: parameter 'i'
+// We warn, but we can't give a fix
+// CHECK-FIXES: void ConstInMacro(CONST_INT_PARAM);
+
+#define DECLARE_FUNCTION_WITH_ARG(x) struct InsideMacro{ x }
+DECLARE_FUNCTION_WITH_ARG(
+void member_function(const int i);
+);
+// CHECK-MESSAGES: :[[@LINE-2]]:26: warning: parameter 'i'
+// CHECK-FIXES: void member_function(int i);
Index: clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
@@ -15,3 +15,11 @@
 
   void f(const string);   // Bad: const is top level.
   void f(const string&);  // Good: const is not top level.
+
+Options
+---
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will not give warnings inside macros. Default
+   is `true`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -149,6 +149,10 @@
   copy assignment operators with nonstandard return types. The check is restricted to
   c++11-or-later.
 
+- Change the default behavior of :doc:`readability-avoid-const-params-in-decls
+  ` to not
+  warn about `const` value parameters of declarations inside macros.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
===
--- clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
+++ clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
@@ -20,13 +20,18 @@
 class AvoidConstParamsInDecls : public ClangTidyCheck {
 public:
   AvoidConstParamsInDecls(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatc

[clang-tools-extra] 9a4e52e - [clang-tidy] Add an IgnoreMacros option to readability-avoid-const-params-in-decls

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-09-30T14:27:02Z
New Revision: 9a4e52ebeb6dd8527bc1ee944a9466d68a95b6f1

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

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

readability-avoid-const-params-in-decls.IgnoreMacros is enabled by default to 
be consistent with most other checks.

Reviewed By: ymandel

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp

Modified: 
clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst

clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp 
b/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
index e655f013a254..b4ff4ad805d9 100644
--- a/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
+++ b/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.cpp
@@ -27,6 +27,10 @@ SourceRange getTypeRange(const ParmVarDecl &Param) {
 
 } // namespace
 
+void AvoidConstParamsInDecls::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void AvoidConstParamsInDecls::registerMatchers(MatchFinder *Finder) {
   const auto ConstParamDecl =
   parmVarDecl(hasType(qualType(isConstQualified(.bind("param");
@@ -44,6 +48,12 @@ void AvoidConstParamsInDecls::check(const 
MatchFinder::MatchResult &Result) {
   if (!Param->getType().isLocalConstQualified())
 return;
 
+  if (IgnoreMacros &&
+  (Param->getBeginLoc().isMacroID() || Param->getEndLoc().isMacroID())) {
+// Suppress the check if macros are involved.
+return;
+  }
+
   auto Diag = diag(Param->getBeginLoc(),
"parameter %0 is const-qualified in the function "
"declaration; const-qualification of parameters only has an 
"

diff  --git 
a/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h 
b/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
index d366949e4a16..a7637a9c3050 100644
--- a/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
+++ b/clang-tools-extra/clang-tidy/readability/AvoidConstParamsInDecls.h
@@ -20,13 +20,18 @@ namespace readability {
 class AvoidConstParamsInDecls : public ClangTidyCheck {
 public:
   AvoidConstParamsInDecls(StringRef Name, ClangTidyContext *Context)
-  : ClangTidyCheck(Name, Context) {}
+  : ClangTidyCheck(Name, Context),
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   llvm::Optional getCheckTraversalKind() const override {
 return TK_IgnoreUnlessSpelledInSource;
   }
+
+private:
+  const bool IgnoreMacros;
 };
 
 } // namespace readability

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 89dd7e746e58..c47f986e4021 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -149,6 +149,10 @@ Changes in existing checks
   copy assignment operators with nonstandard return types. The check is 
restricted to
   c++11-or-later.
 
+- Change the default behavior of :doc:`readability-avoid-const-params-in-decls
+  ` to not
+  warn about `const` value parameters of declarations inside macros.
+
 Removed checks
 ^^
 

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
 
b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
index 3aea5d0c0753..b1146e9da79b 100644
--- 
a/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
+++ 
b/clang-tools-extra/docs/clang-tidy/checks/readability/avoid-const-params-in-decls.rst
@@ -15,3 +15,11 @@ Examples:
 
   void f(const string);   // Bad: const is top level.
   void f(const string&);  // Good: const is not top level.
+
+Options
+---
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will not give warnings inside macros. Default
+   is `true`.

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-const-params-in-decls-macros.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/avoid-cons

[PATCH] D132416: [Driver][Fuchsia] Add default linker flags

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks tests on non-linux:

http://45.33.8.238/macm1/45619/step_7.txt
http://45.33.8.238/win/67193/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/D132416/new/

https://reviews.llvm.org/D132416

___
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#3827185 , @dexonsmith 
wrote:

> In D134456#3827040 , @aaron.ballman 
> wrote:
>
>> In D134456#3819185 , @rnk wrote:
>>
>>> In D134456#3819042 , 
>>> @aaron.ballman wrote:
>>>
 Alternatively, we could document that we don't follow the intent of the 
 standard feature and that's just the way things are, and add a command 
 line option to honor that intent. However, given that GCC honors the 
 attribute over PGO, I think we should change our default behavior.
>>>
>>> That makes sense to me. I think it's reasonable to request folks to add a 
>>> flag in the context of a bug fix, but it would be too much to ask a drive 
>>> by contributor to go through the multi-step process to change the default 
>>> flag behavior.
>>
>> I guess I view it differently -- I think users should get the correct 
>> behavior by default rather than have to opt into it, and I'm not certain 
>> that "it's a drive-by contribution" is a good driver of user experience 
>> decisions. Either way, I think we need an explicit decision as to which 
>> approach we think is *right* and go from there in terms of how to achieve 
>> that goal. (It'd be a bit different if I thought this patch was incremental 
>> progress, but from my current views on the topic, I actually think the patch 
>> is a further regression away from where we want to be.)
>>
>> I'm adding in @dexonsmith as "branch weights" code owner and @dnovillo as 
>> "SampleProfile and related parts of ProfileData" code owner on the LLVM side 
>> to see if they have opinions on default behaviors (if there are other PGO 
>> experts to bring in, please sign them up).
>
> The design of PGO was to use user hints when there’s no coverage, but 
> basically ignore them if theres enough data to say otherwise.

Whelp, if that's the design, then I guess we should stick with it. It's rather 
unfortunate to me that our default behavior is "ignore what the user told us 
because we're certain we know better" when we know that to be false and isn't 
how GCC behaves in this case, but it also is what it is. At least it's 
consistent with other highly successful standardized 
optimization hints like `register`, `inline`, and 
`[[carries_dependency]]`. :-P

> This safety scenario sounds like it could differ within a file. Is a flag 
> really the right control? Maybe what you want is a `[[noprofile]]`, similar 
> to `[[noopt]]`, which selectively disables the profile where the user wants 
> fine-grained control to ignore the profile.

My understanding is that it's pretty rare for safety critical code to mix with 
non-safety critical code, so a flag sounds like the correct granularity to me 
in terms of the use cases I'm aware of. However, parallel attributes do give 
the most control to the user. My worry is that parallel attributes will be used 
as:

  #if __has_cpp_attribute(clang::likely_but_honor_this_one)
  #define LIKELY [[clang::likely_but_honor_this_one]]
  #elif __has_cpp_attribute(likely)
  #define LIKELY [[likely]]
  #else
  #define LIKELY
  #endif

either due to need (like safety critical situations) or due to user confusion 
(like we used to see happen with `register` or `inline` in the Olden Days), but 
I suppose a command line flag runs that same danger of misuse and is about as 
portable, so I could go with either approach.


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

[clang-tools-extra] 40f5c63 - [NFC] Fix for doc typo in commit 9a4e52ebeb6d

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-09-30T14:53:18Z
New Revision: 40f5c634bc991510427e274ba49d4c56f29ef593

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

LOG: [NFC] Fix for doc typo in commit 9a4e52ebeb6d

Added: 


Modified: 
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index c47f986e4021..9447cbfba73e 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -150,7 +150,7 @@ Changes in existing checks
   c++11-or-later.
 
 - Change the default behavior of :doc:`readability-avoid-const-params-in-decls
-  ` to not
+  ` to not
   warn about `const` value parameters of declarations inside macros.
 
 Removed checks



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

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

dexonsmith added a comment.

In D134456#3827263 , @aaron.ballman 
wrote:

> In D134456#3827185 , @dexonsmith 
> wrote:
>
>> This safety scenario sounds like it could differ within a file. Is a flag 
>> really the right control? Maybe what you want is a `[[noprofile]]`, similar 
>> to `[[noopt]]`, which selectively disables the profile where the user wants 
>> fine-grained control to ignore the profile.
>
> My understanding is that it's pretty rare for safety critical code to mix 
> with non-safety critical code, so a flag sounds like the correct granularity 
> to me in terms of the use cases I'm aware of.

In that case, maybe those files should just turn off PGO entirely. There's 
already a command-line for that:

- `-fprofile-use=...`: compiler should use the profile to improve performance 
(user hints lose when there's disagreement)
- default: no access to a profile; user hints by default

Maybe (if it doesn't exist yet) `-fno-profile-use` would be useful for easy 
cancellation.

In safety-critical code where you don't entirely trust the 
profile/coverage/compiler to do the right thing, why risk having the feature 
enabled at all?


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

[clang] 9ec7272 - Revert "[Driver][Fuchsia] Add default linker flags"

[Alex Brachet via cfe-commits]

Author: Alex Brachet
Date: 2022-09-30T14:54:48Z
New Revision: 9ec7272fc5eab5295372d46f2f1d59b10907c093

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

LOG: Revert "[Driver][Fuchsia] Add default linker flags"

This reverts commit 5dfc8ebee5d688ee94607ccce655672c1a198b82.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Fuchsia.cpp
clang/test/Driver/fuchsia.c
clang/test/Driver/fuchsia.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Fuchsia.cpp 
b/clang/lib/Driver/ToolChains/Fuchsia.cpp
index 2e47a96bcfe3..6f4fa2ce7c40 100644
--- a/clang/lib/Driver/ToolChains/Fuchsia.cpp
+++ b/clang/lib/Driver/ToolChains/Fuchsia.cpp
@@ -65,12 +65,6 @@ void fuchsia::Linker::ConstructJob(Compilation &C, const 
JobAction &JA,
 CmdArgs.push_back("-z");
 CmdArgs.push_back("rel");
 CmdArgs.push_back("--pack-dyn-relocs=relr");
-  } else {
-// The following are already the default in lld
-CmdArgs.push_back("-z");
-CmdArgs.push_back("combreloc");
-CmdArgs.push_back("-z");
-CmdArgs.push_back("text");
   }
 
   if (!D.SysRoot.empty())

diff  --git a/clang/test/Driver/fuchsia.c b/clang/test/Driver/fuchsia.c
index 00c763a140c3..099a88c2e4e3 100644
--- a/clang/test/Driver/fuchsia.c
+++ b/clang/test/Driver/fuchsia.c
@@ -1,35 +1,27 @@
 // RUN: %clang -### %s --target=x86_64-unknown-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-X86_64 %s
-// RUN: %clang -### %s --target=x86_64-unknown-fuchsia \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: --sysroot=%S/platform 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-X86_64 %s
-// RUN: %clang -### %s --target=x86_64-unknown-fuchsia \
-// RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
-// RUN: --sysroot=%S/platform -fuse-ld=gold 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LD,CHECK-X86_64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
 // RUN: %clang -### %s --target=aarch64-unknown-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-AARCH64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
 // RUN: %clang -### %s --target=riscv64-unknown-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-RISCV64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
 // RUN: %clang -### %s --target=x86_64-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-X86_64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-X86_64 %s
 // RUN: %clang -### %s --target=aarch64-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-AARCH64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-AARCH64 %s
 // RUN: %clang -### %s --target=riscv64-fuchsia \
 // RUN: -resource-dir=%S/Inputs/resource_dir_with_per_target_subdir \
 // RUN: --sysroot=%S/platform -fuse-ld=lld 2>&1 \
-// RUN: | FileCheck -check-prefixes=CHECK,CHECK-LLD,CHECK-RISCV64 %s
+// RUN: | FileCheck -check-prefixes=CHECK,CHECK-RISCV64 %s
 // CHECK: "-cc1"
 // CHECK-X86_64: "-triple" "x86_64-unknown-fuchsia"
 // CHECK-AARCH64: "-triple" "aarch64-unknown-fuchsia"
@@ -44,8 +36,7 @@
 // CHECK: "-stack-protector" "2"
 // CHECK-AARCH64: "-target-feature" "+outline-atomics"
 // CHECK-NOT: "-fcommon"
-// CHECK-LLD: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
-// CHECK-LD: {{.*}}ld.gold{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"combreloc" "-z" "text"
+// CHECK: {{.*}}ld.lld{{.*}}" "-z" "max-page-size=4096" "-z" "now" "-z" 
"rodynamic" "-z" "separate-loadable-segments" "-z" "rel" 
"--pack-dyn-relocs=relr"
 // CHECK: "--sysroot=[[SYSROOT]]"
 // CHECK: "-pie"
 // CHECK: "--build-id"

diff  --git a/clang/test/Driver/fuchsia.cpp b/clang/test/Driver/fuchsia.cpp
index e247c76dabdd..e5640f582627 100644
--- a/clang/test/Driver/fuchsia.cpp
+++ b/clang/test/Driver/fuchsia.cpp
@@ -2,42 +2,32 @@
 // RUN: -ccc-install-dir %S/Inputs/basic_fuchsia_tree/bin \
 // RUN: -resourc

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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Note: fixed typo in doc in followup commit 
40f5c634bc991510427e274ba49d4c56f29ef593 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130130

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

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134456#3827281 , @dexonsmith 
wrote:

> In D134456#3827263 , @aaron.ballman 
> wrote:
>
>> In D134456#3827185 , @dexonsmith 
>> wrote:
>>
>>> This safety scenario sounds like it could differ within a file. Is a flag 
>>> really the right control? Maybe what you want is a `[[noprofile]]`, similar 
>>> to `[[noopt]]`, which selectively disables the profile where the user wants 
>>> fine-grained control to ignore the profile.
>>
>> My understanding is that it's pretty rare for safety critical code to mix 
>> with non-safety critical code, so a flag sounds like the correct granularity 
>> to me in terms of the use cases I'm aware of.
>
> In that case, maybe those files should just turn off PGO entirely. There's 
> already a command-line for that:
>
> - `-fprofile-use=...`: compiler should use the profile to improve performance 
> (user hints lose when there's disagreement)
> - default: no access to a profile; user hints by default
>
> Maybe (if it doesn't exist yet) `-fno-profile-use` would be useful for easy 
> cancellation.
>
> In safety-critical code where you don't entirely trust the 
> profile/coverage/compiler to do the right thing, why risk having the feature 
> enabled at all?

My point is that the annotation is a way for the user to be able to entirely 
trust the implementation so that they can enable things like PGO, and because 
it's a standard attribute and GCC honors it during PGO, the user gets some 
theoretical portability out of it. But, 1) attributes can be ignored, even 
standard ones, and so there really isn't a super strong portability argument in 
terms of being able to *rely* on anything, 2) optimization hints are 
traditionally things the optimizer ignores when it believes it knows better, 3) 
it matches the design intent of our PGO implementation. So I think I'm sold on 
the approach in this patch, and going with a separate set of attributes to give 
users more control -- that at least leaves users in a situation where they can 
benefit from PGO but still have more control over its behavior. (I don't think 
this patch should be blocked on those attributes being implemented, either.)


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] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

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

dexonsmith added a comment.

In D134456#3827263 , @aaron.ballman 
wrote:

> My worry is that parallel attributes will be used as:
>
>   #if __has_cpp_attribute(clang::likely_but_honor_this_one)
>   #define LIKELY [[clang::likely_but_honor_this_one]]
>   #elif __has_cpp_attribute(likely)
>   #define LIKELY [[likely]]
>   #else
>   #define LIKELY
>   #endif

To be clear, I was imagining:

  if (always_false()) [[likely]] [[clang::nopgo]] {
// ...
  }

where ``nopgo`` might be independently useful for telling clang to ignore any 
collected PGO data when estimating branch weights in a particular control flow 
block.

Some users might combine the two into a macro ("always ignore the profile when 
I say something is likely!"), but I don't think there'd be a cascade.


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] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

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

dexonsmith added a comment.

In D134456#3827332 , @dexonsmith 
wrote:

> In D134456#3827263 , @aaron.ballman 
> wrote:
>
>> My worry is that parallel attributes will be used as:
>>
>>   #if __has_cpp_attribute(clang::likely_but_honor_this_one)
>>   #define LIKELY [[clang::likely_but_honor_this_one]]
>>   #elif __has_cpp_attribute(likely)
>>   #define LIKELY [[likely]]
>>   #else
>>   #define LIKELY
>>   #endif
>
> To be clear, I was imagining:
>
>   if (always_false()) [[likely]] [[clang::nopgo]] {
> // ...
>   }
>
> where ``nopgo`` might be independently useful for telling clang to ignore any 
> collected PGO data when estimating branch weights in a particular control 
> flow block.
>
> Some users might combine the two into a macro ("always ignore the profile 
> when I say something is likely!"), but I don't think there'd be a cascade.

(In practice a use of `nopgo` probably needs to drop the profile for the whole 
function, but maybe that's fine...)


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] D134456: [PGO] Consider parent context when weighing branches with likelyhood.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D134456#3827332 , @dexonsmith 
wrote:

> In D134456#3827263 , @aaron.ballman 
> wrote:
>
>> My worry is that parallel attributes will be used as:
>>
>>   #if __has_cpp_attribute(clang::likely_but_honor_this_one)
>>   #define LIKELY [[clang::likely_but_honor_this_one]]
>>   #elif __has_cpp_attribute(likely)
>>   #define LIKELY [[likely]]
>>   #else
>>   #define LIKELY
>>   #endif
>
> To be clear, I was imagining:
>
>   if (always_false()) [[likely]] [[clang::nopgo]] {
> // ...
>   }
>
> where ``nopgo`` might be independently useful for telling clang to ignore any 
> collected PGO data when estimating branch weights in a particular control 
> flow block.
>
> Some users might combine the two into a macro ("always ignore the profile 
> when I say something is likely!"), but I don't think there'd be a cascade.

Ah, I see, thank you for clarifying! Does that seem like a generally useful 
attribute to you? (It seems like it could be, but this isn't my area of 
expertise.)

Another thought I had is: when PGO is enabled, can we diagnose when PGO 
countermands an explicit annotation? Something to at least alert the user "hey, 
look over here, this suggestion was wrong" so they have more of a chance of 
knowing something is up?


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] D134885: [Clang] Fix variant crashes from GH58028, GH57370

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D134885#3826335 , @royjacobson 
wrote:

> Apparently some of the workers crashed with the test - 
> https://lab.llvm.org/buildbot/#/builders/216/builds/10556, but I couldn't 
> reproduce this locally. @shafik any idea why the diagnostics might change? :/

I have no idea where that warning comes from.  That shouldn't happen on ANY 
machine, since it is a public member (a is), not a private one (which should 
cause the diagnostic).  Is that the only bot with this problem?

I don't know what the 'sie' build is... but it gets me wondering if that 
diagnostic is getting an uninitialized version of "VerifyOnly" somewhere?  It 
seems to me that it should be suppressing that diagnostic (if that is what 
VerifyOnly means here?)?

It IS bizarre to me that this crash would be noticed by something that DIDN'T 
try to evaluate that diagnostic though, since the ICE is on that code path.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134885

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

[PATCH] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

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

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

Added simple check for create of copy of pointer before the `realloc` call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133119

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousReallocUsageCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-realloc-usage.cpp
@@ -0,0 +1,102 @@
+// RUN: %check_clang_tidy %s bugprone-suspicious-realloc-usage %t
+
+void *realloc(void *, __SIZE_TYPE__);
+
+namespace std {
+  using ::realloc;
+}
+
+namespace n1 {
+  void *p;
+}
+
+namespace n2 {
+  void *p;
+}
+
+struct P {
+  void *p;
+  void *q;
+  P *pp;
+  void *&f();
+};
+
+struct P1 {
+  static void *p;
+  static void *q;
+};
+
+template
+struct P2 {
+  static void *p;
+  static void *q;
+};
+
+template
+void templ(void *p) {
+  A::p = realloc(A::p, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: 'A::p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+  p = realloc(A::p, 10);
+  A::q = realloc(A::p, 10);
+  A::p = realloc(B::p, 10);
+  P2::p = realloc(P2::p, 1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'P2::p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+  P2::p = realloc(P2::p, 1);
+}
+
+void *&getPtr();
+P &getP();
+
+void warn(void *p, P *p1, int *pi) {
+  p = realloc(p, 111);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  p = std::realloc(p, sizeof(int));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  p1->p = realloc(p1->p, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: 'p1->p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  p1->pp->p = realloc(p1->pp->p, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: 'p1->pp->p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  pi = (int*)realloc(pi, 10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: 'pi' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+
+  templ>(p);
+}
+
+void no_warn(void *p, P *p1, P *p2) {
+  void *q = realloc(p, 10);
+  q = realloc(p, 10);
+  p1->q = realloc(p1->p, 10);
+  p2->p = realloc(p1->p, 20);
+  n1::p = realloc(n2::p, 30);
+  p1->pp->p = realloc(p1->p, 10);
+  getPtr() = realloc(getPtr(), 30);
+  getP().p = realloc(getP().p, 20);
+  p1->f() = realloc(p1->f(), 30);
+}
+
+void no_warn_if_copy_exists_before1(void *p) {
+  void *q = p;
+  p = realloc(p, 111);
+}
+
+void no_warn_if_copy_exists_before2(void *p, void *q) {
+  q = p;
+  p = realloc(p, 111);
+}
+
+void *g_p;
+
+void no_warn_if_copy_exists_before3() {
+  void *q = g_p;
+  g_p = realloc(g_p, 111);
+}
+
+void warn_if_copy_exists_after(void *p) {
+  p = realloc(p, 111);
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'p' may be set to null if 'realloc' fails, which may result in a leak of the original buffer [bugprone-suspicious-realloc-usage]
+  void *q = p;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -124,6 +124,7 @@
`bugprone-suspicious-memory-comparison `_,
`bugprone-suspicious-memset-usage `_, "Yes"
`bugprone-suspicious-missing-comma `_,
+   `bugprone-suspicious-realloc-usage `_,
`bugprone-suspicious-semicolon `_, "Yes"
`bugprone-suspicious-string-compare `_, "Yes"
`bugprone-swapped-arguments `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/suspicious-realloc-usage.rst
@@ -0,0 +1,44 @@
+.. title:: clang-tidy - bugprone-suspic

[PATCH] D134885: [Clang] Fix variant crashes from GH58028, GH57370

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D134885#3827383 , @erichkeane 
wrote:

> In D134885#3826335 , @royjacobson 
> wrote:
>
>> Apparently some of the workers crashed with the test - 
>> https://lab.llvm.org/buildbot/#/builders/216/builds/10556, but I couldn't 
>> reproduce this locally. @shafik any idea why the diagnostics might change? :/
>
> I have no idea where that warning comes from.  That shouldn't happen on ANY 
> machine, since it is a public member (a is), not a private one (which should 
> cause the diagnostic).  Is that the only bot with this problem?
>
> I don't know what the 'sie' build is... but it gets me wondering if that 
> diagnostic is getting an uninitialized version of "VerifyOnly" somewhere?  It 
> seems to me that it should be suppressing that diagnostic (if that is what 
> VerifyOnly means here?)?
>
> It IS bizarre to me that this crash would be noticed by something that DIDN'T 
> try to evaluate that diagnostic though, since the ICE is on that code path.

Hmmm, uninitialized access might explain why it's runner dependent. I'll try 
and see if I can run it through USan.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134885

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

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

dexonsmith added a comment.

In D134456#3827377 , @aaron.ballman 
wrote:

> In D134456#3827332 , @dexonsmith 
> wrote:
>
>> In D134456#3827263 , 
>> @aaron.ballman wrote:
>>
>>> My worry is that parallel attributes will be used as:
>>>
>>>   #if __has_cpp_attribute(clang::likely_but_honor_this_one)
>>>   #define LIKELY [[clang::likely_but_honor_this_one]]
>>>   #elif __has_cpp_attribute(likely)
>>>   #define LIKELY [[likely]]
>>>   #else
>>>   #define LIKELY
>>>   #endif
>>
>> To be clear, I was imagining:
>>
>>   if (always_false()) [[likely]] [[clang::nopgo]] {
>> // ...
>>   }
>>
>> where ``nopgo`` might be independently useful for telling clang to ignore 
>> any collected PGO data when estimating branch weights in a particular 
>> control flow block.
>>
>> Some users might combine the two into a macro ("always ignore the profile 
>> when I say something is likely!"), but I don't think there'd be a cascade.
>
> Ah, I see, thank you for clarifying! Does that seem like a generally useful 
> attribute to you? (It seems like it could be, but this isn't my area of 
> expertise.)

Well, "generally useful" might be a stretch. Probably most users wouldn't 
want/need this. But if a user wants fine-grained control of the optimizer 
(probably not achievable, really), then turning off PGO seems like a knob they 
might want.

On the other hand, maybe `[[nopgo]]` is all that the safety-critical code 
should be using. IIRC, `[[likely]]` is really saying "hint: the other path is 
cold", and the optimizer pessimists the other path. Perhaps the safety-critical 
crowd just wants to turn off all pessimization; if so, they'll find that adding 
`[[likely]]` to the must-fail-quickly error path will do bad things to the 
non-error path.

> Another thought I had is: when PGO is enabled, can we diagnose when PGO 
> countermands an explicit annotation? Something to at least alert the user 
> "hey, look over here, this suggestion was wrong" so they have more of a 
> chance of knowing something is up?

Seems pretty noisy, much like when the inliner countermands an `inline` hint. 
Maybe an optimization remark would be appropriate, allowing advanced users to 
selectively enable them in a critical section when debugging a performance 
issue?


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] D133119: [clang-tidy] Add checker 'bugprone-suspicious-realloc-usage'.

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

balazske marked an inline comment as done.
balazske added a comment.

I added a simple detection of create a copy of `p` before `p = realloc(p, 
...)`. This can remove the warning at very obvious cases when a copy of `p` is 
created (but even if the copy is made inside an `if` branch for example).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133119

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

[PATCH] D134885: [Clang] Fix variant crashes from GH58028, GH57370

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Only thing I can think of is the 'Aggregate' calculation for this diagnostic is 
going wrong somewhere.  See SemaDeclcXX.cpp ~6759 for where this all happens.

Aggregate IS initialized correctly to 'true' in CXXRecordDecl's DefinitionData 
as far as I can tell.  BUT I can't imagine that some sort of optimization or 
mis-initialization of aggregate wouldn't cause some significant problems 
elsewhere.  I'm thoroughly confused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134885

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

[PATCH] D134176: Add MC support of RISCV Zcf Extension

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper 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/D134176/new/

https://reviews.llvm.org/D134176

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

[PATCH] D134177: Add MC support of RISCV Zcd Extension

[Craig Topper via Phabricator via cfe-commits]

craig.topper accepted this revision.
craig.topper 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/D134177/new/

https://reviews.llvm.org/D134177

___
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#3827410 , @dexonsmith 
wrote:

> In D134456#3827377 , @aaron.ballman 
> wrote:
>
>> Ah, I see, thank you for clarifying! Does that seem like a generally useful 
>> attribute to you? (It seems like it could be, but this isn't my area of 
>> expertise.)
>
> Well, "generally useful" might be a stretch. Probably most users wouldn't 
> want/need this. But if a user wants fine-grained control of the optimizer 
> (probably not achievable, really), then turning off PGO seems like a knob 
> they might want.

Heh, you interpreted me properly. :-D I mostly meant "does this attribute seem 
like it'd be usable in other circumstances or is it likely to be really 
specific to just this situation?

> On the other hand, maybe `[[nopgo]]` is all that the safety-critical code 
> should be using. IIRC, `[[likely]]` is really saying "hint: the other path is 
> cold", and the optimizer pessimists the other path. Perhaps the 
> safety-critical crowd just wants to turn off all pessimization; if so, 
> they'll find that adding `[[likely]]` to the must-fail-quickly error path 
> will do bad things to the non-error path.

If "bad things" is "make the non-error path slow", my understanding is that's 
exactly their expectations.

>> Another thought I had is: when PGO is enabled, can we diagnose when PGO 
>> countermands an explicit annotation? Something to at least alert the user 
>> "hey, look over here, this suggestion was wrong" so they have more of a 
>> chance of knowing something is up?
>
> Seems pretty noisy, much like when the inliner countermands an `inline` hint. 
> Maybe an optimization remark would be appropriate, allowing advanced users to 
> selectively enable them in a critical section when debugging a performance 
> issue?

Hmmm, on the one hand, any optimization hint is really an advanced user feature 
and so the opt remark might make sense. On the other hand, when I posted about 
`[[likely]]` and PGO on Twitter (not exactly what I'd call a scientific poll, 
take with an ocean of salt!), multiple people were surprised and expected PGO 
to honor the attribute, which suggests users may want this information. I don't 
have a good feel for what the user expectations are here, so I think an opt 
remark would be the most conservative way to start, and if we find that it's 
too hidden and non-expert users really need warnings, we can add a warning at 
that point. WDYT?


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] D134885: [Clang] Fix variant crashes from GH58028, GH57370

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D134885#3826335 , @royjacobson 
wrote:

> Apparently some of the workers crashed with the test - 
> https://lab.llvm.org/buildbot/#/builders/216/builds/10556, but I couldn't 
> reproduce this locally. @shafik any idea why the diagnostics might change? :/

I can't see what is going on here either. It looks like this is a windows PS5 
bot. If you have access to a windows machine it might be worth trying a windows 
release w/ asserts build to see if it reproduces there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134885

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

[PATCH] D20401: [Lexer] Don't merge macro args from different macro files

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D20401#3824713 , @sammccall wrote:

> Thanks Nick for the info! No kernel experience here, so if you have any 
> particular suggestions about how to measure the workload you care about it'd 
> be much appreciated (e.g. are particular files that are slow enough to 
> measure in isolation, or is it better to do a full build)

@sammccall I wrote up instructions for how to profile a Linux kernel build with 
LLVM.
https://github.com/ClangBuiltLinux/profiling/tree/main/perf
A build on a 72+ threaded workstation should only take ~1 minute. Can you 
please give it a shot and let me know off-thread if you encounter any issues?
One thing I found about that workflow: just this week I upgraded to a 
zen2-based threadripper workstation. It appears that zen2 has issues using 
per-thread LBR.
https://www.spinics.net/lists/linux-perf-users/msg23103.html
(There's follow up responses that I don't see yet in the archive, but it looks 
like there's pending Linux kernel patches to get that working.)
https://lore.kernel.org/lkml/166155216401.401.5809694678609694438.tip-bot2@tip-bot2/
https://lore.kernel.org/lkml/20220829113347.295-1-ravi.bango...@amd.com/
So you might want to ensure you're running those instructions on an intel box, 
for now. I'm also happy to hop on a virtual call with you and @hokein anytime.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D20401

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

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

In D134337#3824862 , @MaskRay wrote:

> It's unclear Clang wants to support GCC style specs file and whether GCC 
> wants to adopt another system beside its specs.
> I lean toward there isn't much cooperation as we might think.

Perhaps there ought to be?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134337

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

[PATCH] D134885: [Clang] Fix variant crashes from GH58028, GH57370

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D134885#3827479 , @shafik wrote:

> In D134885#3826335 , @royjacobson 
> wrote:
>
>> Apparently some of the workers crashed with the test - 
>> https://lab.llvm.org/buildbot/#/builders/216/builds/10556, but I couldn't 
>> reproduce this locally. @shafik any idea why the diagnostics might change? :/
>
> I can't see what is going on here either. It looks like this is a windows PS5 
> bot. If you have access to a windows machine it might be worth trying a 
> windows release w/ asserts build to see if it reproduces there.

Ahhh, that exaplains it. The PlayStation targets didn't switch their default 
standard version to C++17, and using --std=c++14 will indeed reproduce the 
warnings. Some C++17 aggregate initialization changes, I guess. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134885

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