[PATCH] D120111: [AArch64] Default HBC/MOPS features in clang

[Son Tuan Vu via Phabricator via cfe-commits]

tyb0807 created this revision.
Herald added a subscriber: kristof.beyls.
tyb0807 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This implements minimum support in clang for default HBC/MOPS features
on v8.8-a/v9.3-a or later architectures.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120111

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-hbc.c
  clang/test/Driver/aarch64-mops.c
  clang/test/Preprocessor/aarch64-target-features.c

Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -521,13 +521,17 @@
 // CHECK-LSE: __ARM_FEATURE_ATOMICS 1
 
 // == Check Armv8.8-A/Armv9.3-A memcpy and memset acceleration instructions (MOPS)
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.7-a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.7-a+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.2-a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.2-a+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
-// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.7-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.7-a+mops-x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+nomops  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+nomops+mops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+mops-x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv8.8-a+mops+nomops -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.2-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.2-a+mops-x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+nomops  -x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-NOMOPS %s
+// RUN: %clang -target aarch64-arm-none-eabi -march=armv9.3-a+mops-x c -E -dM %s -o - | FileCheck --check-prefix=CHECK-MOPS   %s
 // CHECK-MOPS: __ARM_FEATURE_MOPS 1
 // CHECK-NOMOPS-NOT: __ARM_FEATURE_MOPS 1
Index: clang/test/Driver/aarch64-mops.c
===
--- clang/test/Driver/aarch64-mops.c
+++ clang/test/Driver/aarch64-mops.c
@@ -1,6 +1,12 @@
 // Test that target feature mops is implemented and available correctly
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+mops %s 2>&1 | FileCheck %s
-// CHECK: "-target-feature" "+mops"
-
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.7-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a%s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+mops   %s 2>&1 | FileCheck %s
 // RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+nomops %s 2>&1 | FileCheck %s --check-prefix=NO_MOPS
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.2-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a%s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+mops   %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+nomops %s 2>&1 | FileCheck %s --check

[PATCH] D119144: [tests][Driver] Pass an empty sysroot for `DEFAULT_SYSROOT` builds

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D119144/new/

https://reviews.llvm.org/D119144

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

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 409872.
ChuanqiXu retitled this revision from "[C++20] [Modules] Remain 
internal-linkage variables in module interface unit" to "[C++20] [Modules] 
Remain dynamic initializing internal-linkage variables in module interface unit 
".
ChuanqiXu edited the summary of this revision.
ChuanqiXu added a comment.

Address comments.


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

https://reviews.llvm.org/D119409

Files:
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CodeGenCXX/static-variable-in-module.cpp


Index: clang/test/CodeGenCXX/static-variable-in-module.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/static-variable-in-module.cpp
@@ -0,0 +1,34 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: echo "struct S { S(); };" >> %t/foo.h
+// RUN: echo "static S s = S();" >> %t/foo.h
+// RUN: echo "static int static_init_gmf = 43;" >> %t/foo.h
+// RUN: %clang_cc1 -std=c++20 -I%t %s -emit-module-interface -o %t/m.pcm
+// RUN: %clang_cc1 -std=c++20 %t/m.pcm -triple %itanium_abi_triple -S 
-emit-llvm -o - | FileCheck %s
+module;
+#include "foo.h"
+export module m;
+class A {
+public:
+  A();
+};
+static A a = A();
+static int static_init = 43;
+
+// We shouldn't emit unused internal linkage variable.
+// CHECK-NOT: static_init_gmf
+// CHECK: @_ZW1mEL1s = internal global %struct.S zeroinitializer
+// CHECK: @_ZW1mE1a = {{(dso_local )?}}global %class.A zeroinitializer
+// This is surprising at the first sight. But given [basic.link]4.8:
+//  - otherwise, if the declaration of the name is attached to a named module 
([module.unit]) and is not exported ([module.interface]), the name has module 
linkage;
+//
+// So static_init has module linkage instead of internal linkage. So we should 
emit it even if it is not used.
+// CHECK: @_ZW1mE11static_init = global i32 43
+// CHECK: @llvm.global_ctors = appending global{{.*}}@_GLOBAL__sub_I_m.pcm
+// CHECK: define {{.*}}__cxx_global_var_init[[SUFFIX:[^)]*]]
+// CHECK: call void @_ZN1SC1Ev
+// CHECK: define {{.*}}__cxx_global_var_init[[SUFFIX2:[^)]*]]
+// CHECK: call void @_ZW1mEN1AC1Ev
+// CHECK: define {{.*}}@_GLOBAL__sub_I_m.pcm
+// CHECK: call void @__cxx_global_var_init[[SUFFIX]]
+// CHECK: call void @__cxx_global_var_init[[SUFFIX2]]
Index: clang/lib/Serialization/ASTWriterDecl.cpp
===
--- clang/lib/Serialization/ASTWriterDecl.cpp
+++ clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1022,15 +1022,27 @@
 if (Writer.WritingModule &&
 !D->getDescribedVarTemplate() && !D->getMemberSpecializationInfo() &&
 !isa(D)) {
-  // When building a C++ Modules TS module interface unit, a strong
-  // definition in the module interface is provided by the compilation of
-  // that module interface unit, not by its users. (Inline variables are
-  // still emitted in module users.)
-  ModulesCodegen =
-  (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit ||
-   (D->hasAttr() &&
-Writer.Context->getLangOpts().BuildingPCHWithObjectFile)) &&
-  Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal;
+  // When building a C++20 Modules module interface unit, a strong
+  // definition in the module interface is provided by the module interface
+  // unit, not by its users. (Inline variables are still emitted in module
+  // users.)
+  auto Linkage = Writer.Context->GetGVALinkageForVariable(D);
+  if (Writer.WritingModule->Kind == Module::ModuleInterfaceUnit) {
+ModulesCodegen = Linkage == GVA_StrongExternal;
+if (Linkage == GVA_Internal) {
+  // FIXME: We emit internal variable with dynamic initializer here
+  // otherwise we would fail to compile a hello-world example. (See
+  // https://github.com/llvm/llvm-project/issues/51873). Look back here
+  // once we get rules in CXXABI for variables in named module.
+  const VarDecl *InitDecl;
+  const Expr *InitExpr = D->getAnyInitializer(InitDecl);
+  ModulesCodegen = InitExpr && InitDecl;
+}
+  } else
+ModulesCodegen =
+D->hasAttr() &&
+Writer.Context->getLangOpts().BuildingPCHWithObjectFile &&
+Writer.Context->GetGVALinkageForVariable(D) == GVA_StrongExternal;
 }
 Record.push_back(ModulesCodegen);
 if (ModulesCodegen)


Index: clang/test/CodeGenCXX/static-variable-in-module.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/static-variable-in-module.cpp
@@ -0,0 +1,34 @@
+// RUN: rm -fr %t
+// RUN: mkdir %t
+// RUN: echo "struct S { S(); };" >> %t/foo.h
+// RUN: echo "static S s = S();" >> %t/foo.h
+// RUN: echo "static int static_init_gmf = 43;" >> %t/foo.h
+// RUN: %clang_cc1 -std=c++20 -I%t %s -emit-module-interface -o %t/m.pcm
+// RUN: %clang

[clang] 5333447 - [NFC] Fix a buildbot failure after b529744

[via cfe-commits]

Author: hyeongyukim
Date: 2022-02-18T17:38:50+09:00
New Revision: 5333447a00ff568e5029483e1b46041ad1c9788f

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

LOG: [NFC] Fix a buildbot failure after b529744

Added: 


Modified: 
clang/test/CodeGen/noundef-analysis.cpp

Removed: 




diff  --git a/clang/test/CodeGen/noundef-analysis.cpp 
b/clang/test/CodeGen/noundef-analysis.cpp
index 778810758de4..8a439cde3899 100644
--- a/clang/test/CodeGen/noundef-analysis.cpp
+++ b/clang/test/CodeGen/noundef-analysis.cpp
@@ -19,12 +19,12 @@ static int sink;
 static void examineValue(int x) { sink = x; }
 
 // ENABLED-LABEL: @main(
-// ENABLED:[[CALL:%.*]] = call noundef i32 @_Z19indirect_callee_inti(i32 
noundef 0)
+// ENABLED:[[CALL:%.*]] = call noundef {{.*}}i32 
@_Z19indirect_callee_inti(i32 noundef {{.*}}0)
 // ENABLED:[[CALL1:%.*]] = call i32 @_Z21indirect_callee_union2u1(i32 
{{.*}})
 // ENABLED:[[CALL2:%.*]] = call noalias noundef nonnull i8* @_Znwm(i64 
noundef 4) #[[ATTR4:[0-9]+]]
 // ENABLED:call void @_ZL12examineValuei(i32 noundef {{.*}})
 // DISABLED-LABEL: @main(
-// DISABLED:[[CALL:%.*]] = call i32 @_Z19indirect_callee_inti(i32 0)
+// DISABLED:[[CALL:%.*]] = call {{.*}}i32 @_Z19indirect_callee_inti(i32 
{{.*}}0)
 // DISABLED:[[CALL1:%.*]] = call i32 @_Z21indirect_callee_union2u1(i32 
{{.*}})
 // DISABLED:[[CALL2:%.*]] = call noalias nonnull i8* @_Znwm(i64 4) 
#[[ATTR4:[0-9]+]]
 // DISABLED:call void @_ZL12examineValuei(i32 {{.*}})



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

[clang] c85a264 - [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via cfe-commits]

Author: Alexander Potapenko
Date: 2022-02-18T09:51:54+01:00
New Revision: c85a26454d4b3dab383555c3864568b7aff9c225

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

LOG: [asan] Add support for disable_sanitizer_instrumentation attribute

For ASan this will effectively serve as a synonym for
__attribute__((no_sanitize("address"))).

Adding the disable_sanitizer_instrumentation to functions will drop the
sanitize_XXX attributes on the IR level.

This is the third reland of https://reviews.llvm.org/D114421.
Now that TSan test is fixed (https://reviews.llvm.org/D120050) there
should be no deadlocks.

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

Added: 

llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll

Modified: 
clang/docs/AddressSanitizer.rst
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/SanitizerMetadata.cpp
clang/test/CodeGen/address-safety-attr-flavors.cpp
clang/test/CodeGen/asan-globals.cpp
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

Removed: 




diff  --git a/clang/docs/AddressSanitizer.rst b/clang/docs/AddressSanitizer.rst
index 06b53e2e5da0b..fe5f683580a46 100644
--- a/clang/docs/AddressSanitizer.rst
+++ b/clang/docs/AddressSanitizer.rst
@@ -229,6 +229,12 @@ compilers, so we suggest to use it together with
 The same attribute used on a global variable prevents AddressSanitizer
 from adding redzones around it and detecting out of bounds accesses.
 
+
+AddressSanitizer also supports
+``__attribute__((disable_sanitizer_instrumentation))``. This attribute
+works similar to ``__attribute__((no_sanitize("address")))``, but it also
+prevents instrumentation performed by other sanitizers.
+
 Suppressing Errors in Recompiled Code (Ignorelist)
 --
 

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index 95091edd9ecb7..c4ccc8e1b0424 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -383,9 +383,6 @@ void CodeGenFunction::FinishFunction(SourceLocation EndLoc) 
{
"__cyg_profile_func_exit");
   }
 
-  if (ShouldSkipSanitizerInstrumentation())
-CurFn->addFnAttr(llvm::Attribute::DisableSanitizerInstrumentation);
-
   // Emit debug descriptor for function end.
   if (CGDebugInfo *DI = getDebugInfo())
 DI->EmitFunctionEnd(Builder, CurFn);
@@ -767,18 +764,22 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 
QualType RetTy,
   Fn->addFnAttr(llvm::Attribute::NoSanitizeCoverage);
   }
 
-  // Apply sanitizer attributes to the function.
-  if (SanOpts.hasOneOf(SanitizerKind::Address | SanitizerKind::KernelAddress))
-Fn->addFnAttr(llvm::Attribute::SanitizeAddress);
-  if (SanOpts.hasOneOf(SanitizerKind::HWAddress |
-   SanitizerKind::KernelHWAddress))
-Fn->addFnAttr(llvm::Attribute::SanitizeHWAddress);
-  if (SanOpts.has(SanitizerKind::MemTag))
-Fn->addFnAttr(llvm::Attribute::SanitizeMemTag);
-  if (SanOpts.has(SanitizerKind::Thread))
-Fn->addFnAttr(llvm::Attribute::SanitizeThread);
-  if (SanOpts.hasOneOf(SanitizerKind::Memory | SanitizerKind::KernelMemory))
-Fn->addFnAttr(llvm::Attribute::SanitizeMemory);
+  if (ShouldSkipSanitizerInstrumentation()) {
+CurFn->addFnAttr(llvm::Attribute::DisableSanitizerInstrumentation);
+  } else {
+// Apply sanitizer attributes to the function.
+if (SanOpts.hasOneOf(SanitizerKind::Address | 
SanitizerKind::KernelAddress))
+  Fn->addFnAttr(llvm::Attribute::SanitizeAddress);
+if (SanOpts.hasOneOf(SanitizerKind::HWAddress |
+ SanitizerKind::KernelHWAddress))
+  Fn->addFnAttr(llvm::Attribute::SanitizeHWAddress);
+if (SanOpts.has(SanitizerKind::MemTag))
+  Fn->addFnAttr(llvm::Attribute::SanitizeMemTag);
+if (SanOpts.has(SanitizerKind::Thread))
+  Fn->addFnAttr(llvm::Attribute::SanitizeThread);
+if (SanOpts.hasOneOf(SanitizerKind::Memory | SanitizerKind::KernelMemory))
+  Fn->addFnAttr(llvm::Attribute::SanitizeMemory);
+  }
   if (SanOpts.has(SanitizerKind::SafeStack))
 Fn->addFnAttr(llvm::Attribute::SafeStack);
   if (SanOpts.has(SanitizerKind::ShadowCallStack))

diff  --git a/clang/lib/CodeGen/SanitizerMetadata.cpp 
b/clang/lib/CodeGen/SanitizerMetadata.cpp
index 009965a36c396..9e26d242d3a7e 100644
--- a/clang/lib/CodeGen/SanitizerMetadata.cpp
+++ b/clang/lib/CodeGen/SanitizerMetadata.cpp
@@ -73,6 +73,8 @@ void 
SanitizerMetadata::reportGlobalToASan(llvm::GlobalVariable *GV,
   for (auto Attr : D.specific_attrs())
 if (Attr->getMask() & SanitizerKind::Address)
   IsExcluded = true;
+  if (D.hasAttr())
+IsExcluded = true;
   reportGlobalToASan(GV, D.getLocation(), OS.str(

[PATCH] D120055: [asan] Add support for disable_sanitizer_instrumentation attribute

[Alexander Potapenko via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc85a26454d4b: [asan] Add support for 
disable_sanitizer_instrumentation attribute (authored by glider).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120055

Files:
  clang/docs/AddressSanitizer.rst
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/SanitizerMetadata.cpp
  clang/test/CodeGen/address-safety-attr-flavors.cpp
  clang/test/CodeGen/asan-globals.cpp
  llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
  
llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll

Index: llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
===
--- /dev/null
+++ llvm/test/Instrumentation/AddressSanitizer/asan-disable-sanitizer-instrumentation.ll
@@ -0,0 +1,47 @@
+; This test checks that we are not instrumenting sanitizer code.
+; RUN: opt < %s -passes='asan-pipeline' -S | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+; Function with sanitize_address is instrumented.
+; Function Attrs: nounwind uwtable
+define void @instr_sa(i32* %a) sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @instr_sa
+; CHECK: call void @__asan_report_load
+
+
+; Function with disable_sanitizer_instrumentation is not instrumented.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi(i32* %a) disable_sanitizer_instrumentation {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi
+; CHECK-NOT: call void @__asan_report_load
+
+
+; disable_sanitizer_instrumentation takes precedence over sanitize_address.
+; Function Attrs: nounwind uwtable
+define void @noinstr_dsi_sa(i32* %a) disable_sanitizer_instrumentation sanitize_address {
+entry:
+  %tmp1 = load i32, i32* %a, align 4
+  %tmp2 = add i32 %tmp1,  1
+  store i32 %tmp2, i32* %a, align 4
+  ret void
+}
+
+; CHECK-LABEL: @noinstr_dsi_sa
+; CHECK-NOT: call void @__asan_report_load
+
Index: llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -2888,6 +2888,9 @@
   // Leave if the function doesn't need instrumentation.
   if (!F.hasFnAttribute(Attribute::SanitizeAddress)) return FunctionModified;
 
+  if (F.hasFnAttribute(Attribute::DisableSanitizerInstrumentation))
+return FunctionModified;
+
   LLVM_DEBUG(dbgs() << "ASAN instrumenting:\n" << F << "\n");
 
   initializeCallbacks(*F.getParent());
Index: clang/test/CodeGen/asan-globals.cpp
===
--- clang/test/CodeGen/asan-globals.cpp
+++ clang/test/CodeGen/asan-globals.cpp
@@ -10,6 +10,7 @@
 int global;
 int dyn_init_global = global;
 int __attribute__((no_sanitize("address"))) attributed_global;
+int __attribute__((disable_sanitizer_instrumentation)) disable_instrumentation_global;
 int ignorelisted_global;
 
 int __attribute__((section("__DATA, __common"))) sectioned_global; // KASAN - ignore globals in a section
@@ -50,31 +51,33 @@
 // UWTABLE: attributes #[[#ATTR]] = { nounwind uwtable }
 // UWTABLE: ![[#]] = !{i32 7, !"uwtable", i32 2}
 
-// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
+// CHECK: !llvm.asan.globals = !{![[EXTRA_GLOBAL:[0-9]+]], ![[GLOBAL:[0-9]+]], ![[DYN_INIT_GLOBAL:[0-9]+]], ![[ATTR_GLOBAL:[0-9]+]], ![[DISABLE_INSTR_GLOBAL:[0-9]+]], ![[IGNORELISTED_GLOBAL:[0-9]+]], ![[SECTIONED_GLOBAL:[0-9]+]], ![[SPECIAL_GLOBAL:[0-9]+]], ![[STATIC_VAR:[0-9]+]], ![[LITERAL:[0-9]+]]}
 // CHECK: ![[EXTRA_GLOBAL]] = !{{{.*}} ![[EXTRA_GLOBAL_LOC:[0-9]+]], !"extra_global", i1 false, i1 false}
 // CHECK: ![[EXTRA_GLOBAL_LOC]] = !{!"{{.*}}extra-source.cpp", i32 1, i32 5}
 // CHECK: ![[GLOBAL]] = !{{{.*}} ![[GLOBAL_LOC:[0-9]+]], !"global", i1 false, i1 false}
 // CHECK: ![[GLOBAL_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 10, i32 5}
 // CHECK: ![[DYN_INIT_GLOBAL]] = !{{{.*}} ![[DYN_INIT_LOC:[0-9]+]], !"dyn_init_global", i1 true, i1 false}
 // CHECK: ![[DYN_INIT_LOC]] = !{!"{{.*}}asan-globals.cpp", i32 11, i32 5}
-// CHECK: ![[ATTR_GLOBAL]] = !{{{.*}}, null, null, i1 false, i1 true}
-// CHECK: ![[IGNORELISTED_GLOBAL]] = !{{{.*}},

[clang] 35baa26 - [NFC][Clang/test] add target triple to CodeGen/analyze_noundef.cpp

[via cfe-commits]

Author: hyeongyukim
Date: 2022-02-18T17:54:22+09:00
New Revision: 35baa26747b0033afac15d7989bc2100b251412c

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

LOG: [NFC][Clang/test] add target triple to CodeGen/analyze_noundef.cpp

Added: 


Modified: 
clang/test/CodeGen/noundef-analysis.cpp

Removed: 




diff  --git a/clang/test/CodeGen/noundef-analysis.cpp 
b/clang/test/CodeGen/noundef-analysis.cpp
index 8a439cde3899..1ba60cf0154b 100644
--- a/clang/test/CodeGen/noundef-analysis.cpp
+++ b/clang/test/CodeGen/noundef-analysis.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang -cc1 -enable-noundef-analysis -emit-llvm -o - %s | FileCheck %s 
-check-prefix ENABLED
-// RUN: %clang -cc1 -no-enable-noundef-analysis -emit-llvm -o - %s | FileCheck 
%s -check-prefix DISABLED
+// RUN: %clang_cc1 -triple arm64-darwin -enable-noundef-analysis -emit-llvm -o 
- %s | FileCheck %s -check-prefix ENABLED
+// RUN: %clang_cc1 -triple arm64-darwin -no-enable-noundef-analysis -emit-llvm 
-o - %s | FileCheck %s -check-prefix DISABLED
 
 union u1 {
   int val;
@@ -20,12 +20,12 @@ static void examineValue(int x) { sink = x; }
 
 // ENABLED-LABEL: @main(
 // ENABLED:[[CALL:%.*]] = call noundef {{.*}}i32 
@_Z19indirect_callee_inti(i32 noundef {{.*}}0)
-// ENABLED:[[CALL1:%.*]] = call i32 @_Z21indirect_callee_union2u1(i32 
{{.*}})
+// ENABLED:[[CALL1:%.*]] = call i32 @_Z21indirect_callee_union2u1(i64 
{{.*}})
 // ENABLED:[[CALL2:%.*]] = call noalias noundef nonnull i8* @_Znwm(i64 
noundef 4) #[[ATTR4:[0-9]+]]
 // ENABLED:call void @_ZL12examineValuei(i32 noundef {{.*}})
 // DISABLED-LABEL: @main(
 // DISABLED:[[CALL:%.*]] = call {{.*}}i32 @_Z19indirect_callee_inti(i32 
{{.*}}0)
-// DISABLED:[[CALL1:%.*]] = call i32 @_Z21indirect_callee_union2u1(i32 
{{.*}})
+// DISABLED:[[CALL1:%.*]] = call i32 @_Z21indirect_callee_union2u1(i64 
{{.*}})
 // DISABLED:[[CALL2:%.*]] = call noalias nonnull i8* @_Znwm(i64 4) 
#[[ATTR4:[0-9]+]]
 // DISABLED:call void @_ZL12examineValuei(i32 {{.*}})
 int main() {



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

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/test/CXX/modules-ts/basic/basic.def.odr/p4/module.cpp:5
 // CHECK-DAG: @extern_var_exported = external {{(dso_local )?}}global
-// CHECK-DAG: @inline_var_exported = linkonce_odr {{(dso_local )?}}global
+// CHECK-DAG: @inline_var_exported = available_externally {{(dso_local 
)?}}global
 // CHECK-DAG: @const_var_exported = available_externally {{(dso_local 
)?}}constant i32 3,

urnathan wrote:
> ChuanqiXu wrote:
> > urnathan wrote:
> > > ChuanqiXu wrote:
> > > > dblaikie wrote:
> > > > > ChuanqiXu wrote:
> > > > > > ChuanqiXu wrote:
> > > > > > > urnathan wrote:
> > > > > > > > ChuanqiXu wrote:
> > > > > > > > > urnathan wrote:
> > > > > > > > > > ChuanqiXu wrote:
> > > > > > > > > > > urnathan wrote:
> > > > > > > > > > > > I don;t think this is correct.  That should still be a 
> > > > > > > > > > > > linkonce odr, otherwise you'll get conflicts with other 
> > > > > > > > > > > > module implementation units.
> > > > > > > > > > > It is still linkonce_odr in the module it get defined. 
> > > > > > > > > > > See the new added test case: 
> > > > > > > > > > > inline-variable-in-module.cpp for example. The attribute 
> > > > > > > > > > > `available_externally` is equivalent to external from the 
> > > > > > > > > > > perspective of linker. See 
> > > > > > > > > > > https://llvm.org/docs/LangRef.html#linkage-types. 
> > > > > > > > > > > According to [dcl.inline]p7, inline variable attached to 
> > > > > > > > > > > named module should be defined in that domain. Note that 
> > > > > > > > > > > the variable attached to global module fragment and 
> > > > > > > > > > > private module fragment shouldn't be accessed outside the 
> > > > > > > > > > > module, so it implies that all the variable defined in 
> > > > > > > > > > > the module could only be defined in the module unit 
> > > > > > > > > > > itself.
> > > > > > > > > > There's a couple of issues with this.  module.cppm is 
> > > > > > > > > > emitting a (linkonce) definition of inlne_var_exported, but 
> > > > > > > > > > only because it itself is ODR-using that variable.  If you 
> > > > > > > > > > take out the ODR-use in noninline_exported, there is no 
> > > > > > > > > > longer a symbol emitted.
> > > > > > > > > > 
> > > > > > > > > > But, even if you forced inline vars to be emitted in their 
> > > > > > > > > > defining-module's interface unit, that would be an ABI 
> > > > > > > > > > change.  inline vars are emitted whereever ODR-used.  They 
> > > > > > > > > > have no fixed home TU.  Now, we could alter the ABI and 
> > > > > > > > > > allow interface units to define a home location for inline 
> > > > > > > > > > vars and similar entities (eg, vtables for keyless 
> > > > > > > > > > classes).  But we'd need buy-in from other compilers to do 
> > > > > > > > > > that.
> > > > > > > > > > 
> > > > > > > > > > FWIW such a discussion did come up early in implementing 
> > > > > > > > > > modules-ts, but we decided there was enough going on just 
> > > > > > > > > > getting the TS implemented.  I'm fine with revisiting that, 
> > > > > > > > > > but it is a more significant change.
> > > > > > > > > > 
> > > > > > > > > > And it wouldn't apply to (eg) templated variables, which of 
> > > > > > > > > > course could be instantiated anywhere.
> > > > > > > > > Oh, now the key point here is what the correct behavior is 
> > > > > > > > > instead of the patch. Let's discuss it first.
> > > > > > > > > 
> > > > > > > > > According to [[ http://eel.is/c++draft/dcl.inline#7 | 
> > > > > > > > > [dcl.inline]p7 ]], 
> > > > > > > > > > If an inline function or variable that is attached to a 
> > > > > > > > > > named module is declared in a definition domain, it shall 
> > > > > > > > > > be defined in that domain.
> > > > > > > > > 
> > > > > > > > > I think the intention of the sentence is to define inline 
> > > > > > > > > variable in the module interface. So if it is required by the 
> > > > > > > > > standard, I think other compiler need to follow up. As I 
> > > > > > > > > described in the summary, it might be a difference between 
> > > > > > > > > C++20 module and ModuleTS. Do you think it is necessary to 
> > > > > > > > > send the question to WG21? (I get the behavior from reading 
> > > > > > > > > the words. Maybe I misread or the word is not intentional).
> > > > > > > > > 
> > > > > > > > > Maybe the ABI standard group need to discuss what the linkage 
> > > > > > > > > should be. Now it may be weak_odr or linkonce_odr. It depends 
> > > > > > > > > on how we compile the file. If we compile the .cppm file 
> > > > > > > > > directly, it would be linkonce_odr. And if we compile it to 
> > > > > > > > > *.pcm file first, it would be weak_odr. I have registered an 
> > > > > > > > > issue for this: 
> > > > > > > > > https://github.com/llvm/llvm-project/issues/53838.
> > > > > > > > > Oh, now the key point here is what the correct behavior is 
> > > > > >

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682
+Expr::EvalResult EVRX, EVRY;
+if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) ||
+!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C))
+  return false;
+
+APValue VX = EVRX.Val, VY = EVRY.Val;
+if (VX.getKind() != VY.getKind())

urnathan wrote:
> urnathan wrote:
> > ChuanqiXu wrote:
> > > urnathan wrote:
> > > > ChuanqiXu wrote:
> > > > > urnathan wrote:
> > > > > > I'm kind of surprised how complex this check is.  Isn't there an 
> > > > > > AST comparator available somewhere?
> > > > > I found ODRHash. I think it is much better now.
> > > > hm that suggests there there must be a comparator too -- this isn't a 
> > > > cryptographically strong hash is it?  How would the compiler currently 
> > > > make use of 'definitely different' and 'probably the same' without such 
> > > > a comparator?
> > > Yeah, I am sure there is not an such comparator. Or it has some methods 
> > > like: `ASTContext::hasSameType` for type and `ASTContext::isSameEntity()` 
> > > for Decl. But it lacks such methods now for Stmt and Expr.
> > > 
> > > > How would the compiler currently make use of 'definitely different' and 
> > > > 'probably the same' without such a comparator?
> > > 
> > > Now it uses the two methods I listed above and ODRHash to compare. I 
> > > think the two methods works for  'definitely different' and ODRHash works 
> > > for 'probably the same'. So it's the reason why my previous 
> > > implementation looks lengthy. Since I want to handle it by hand. (The 
> > > previous method only works for simple Expr. I think it would be large 
> > > work to implement comparator for whole Expr or Stmt).
> > Hm, how do template instantations work -- there must be some way of 
> > determining 'is this instantation just here the same as one I've already 
> > seen'
> also, the same functionality (with more generality) is needed for comparing 
> regular function default arguments with multiple definitions in the GM.  How 
> is that done (or is it yet to be implemented?)
> also, the same functionality (with more generality) is needed for comparing 
> regular function default arguments with multiple definitions in the GM. How 
> is that done (or is it yet to be implemented?)

We implemented ODR check in ASTReader by ODRHash.

> Hm, how do template instantations work -- there must be some way of 
> determining 'is this instantation just here the same as one I've already seen'

First, previously we don't allow the redefinition of template default argument. 
And for the case without template default argument, previously we would try 
find the definition for the synthesized type, if we could find one, use it. And 
if we couldn't find one, initialize one.

---

Given we already checks ODR by ODRHash in ASTReader, I think what we do here 
might not be bad. I agree that it is odd at the first sight to use Hash to 
detect difference from the perspective of CS. But if this is what we used to 
do, I think it is acceptable.


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

https://reviews.llvm.org/D118034

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

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

It looks like this patch changed the value of `__FLT_EVAL_METHOD__` when 
building on macOS for some configurations. This is causing build failures when 
using macOS 10.15's `math.h` and that is breaking most builds on GreenDragon, 
e.g. : 
https://green.lab.llvm.org/green/job/clang-stage1-RA/28282/consoleFull#129538464349ba4694-19c4-4d7e-bec5-911270d8a58c

  FAILED: lib/sanitizer_common/tests/CMakeFiles/CompilerRTUnitTestCheckCxx 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/tools/clang/runtime/compiler-rt-bins/lib/sanitizer_common/tests/CMakeFiles/CompilerRTUnitTestCheckCxx
 
  cd 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/tools/clang/runtime/compiler-rt-bins/lib/sanitizer_common/tests
 && bash -c "echo '#include ' | 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/clang 
-isysroot/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk
 -E -x c++ - > /dev/null;if [ \$? != 0 ] ;  then echo;  echo 'Your just-built 
clang cannot find C++ headers, which are needed to build and run compiler-rt 
tests.';  echo 'You should copy or symlink your system C++ headers into 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/include/c++';  
if [ -d \$(dirname \$(dirname \$(xcrun -f clang)))/include/c++ ];then echo 
'e.g. with:';echo '  cp -r' \$(dirname \$(dirname \$(xcrun -f 
clang)))/include/c++ 
'/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/include/';  
elif [ -d \$(dirname \$(dirname \$(xcrun -f clang)))/lib/c++ ];then echo 
'e.g. with:';echo '  cp -r' \$(dirname \$(dirname \$(xcrun -f 
clang)))/lib/c++ 
'/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/include/';  
fi;  echo 'This can also be fixed by checking out the libcxx project from 
llvm.org and installing the headers';  echo 'into your build directory:';  echo 
'  cd 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/llvm-project/llvm/projects 
&& svn co http://llvm.org/svn/llvm-project/libcxx/trunk libcxx';  echo '  cd 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build && make -C 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/llvm-project/llvm/projects/libcxx
 installheaders 
HEADER_DIR=/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/include';
  echo;  false;fi"
  In file included from :1:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/iostream:37:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/ios:215:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/__locale:18:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/memory:813:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/__memory/compressed_pair.h:16:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/tuple:178:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/compare:144:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/__compare/compare_partial_order_fallback.h:13:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/__compare/partial_order.h:14:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/__compare/weak_order.h:14:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/__compare/strong_order.h:18:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/cmath:308:
  In file included from 
/Users/buildslave/jenkins/workspace/clang-stage1-RA/clang-build/./bin/../include/c++/v1/math.h:300:
  
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/usr/include/math.h:53:5:
 error: "Unsupported value of __FLT_EVAL_METHOD__."
  #   error "Unsupported value of __FLT_EVAL_METHOD__."
  ^
  1 error generated.
  
  Your just-built clang cannot find C++ headers, which are needed to build and 
run compiler-rt tests.

From looking at `math.h`, the supported values for `__FLT_EVAL_METHOD__` are 0, 
1, 2 and -1. It seems like there may be a code path that sets it to 
`FEM_UnsetOnCommandLine = 3`. Unless there's a quick fix, I am planning on 
reverting the change soon to bring the bots back to green.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109239

___
cfe-commits ma

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:177
+  if (IsPartition) {
+ModuleName += ":";
+ModuleName += stringFromPath(Partition);

ChuanqiXu wrote:
> iains wrote:
> > urnathan wrote:
> > > iains wrote:
> > > > ChuanqiXu wrote:
> > > > > I chose '-' in my implementation since here ModuleName refers to the 
> > > > > name in filesystem, right? And I remember '-' is the choice of GCC. 
> > > > > So let's keep consistency.
> > > > This is not my understanding.
> > > > 
> > > > ModuleName is the "user-facing" name of the module that is described in 
> > > > the source, and we use this in diagnostics etc.
> > > > 
> > > > The translation of that name to an on-disk name can be arbitrary and 
> > > > there are several mechanisms in clang already (e.g. 
> > > > -fmodule-file=A:B=foo.pcm) the module loader uses these to find the 
> > > > module on the basis of its user-facing name where required.
> > > > 
> > > > When we have P1184, it is even more important that the interface is 
> > > > supplied with the name that the user will put into the source code.
> > > > 
> > > > 
> > > I agree with Iain, we should use ':' is module names here.  When mapping 
> > > a named module to the file system some translation is needed because ':' 
> > > is not permitted in file names on some filesystems (you know the ones!)
> > (just to expand a little more)
> > 
> > the on-disk name needs to be chosen suitably for the platform and by the 
> > user and/or the build system.
> > 
> > When the FE chooses a default filename (which is done in building jobs, not 
> > in the Parser of course) it chooses one based on the source filename.  It 
> > would be most unfortunate if the Parser/Sema needed to understand platform 
> > filename rules.
> > 
> > When you do  'clang -module-file-info' (with the existing or updated 
> > version) you should see the module name as per the source code, the 
> > translation would only apply to the filename itself.
> > 
> > - to answer a later comment:
> > 
> > when we do -fmodule-file=A_B.pcm  and A_B.pcm contains a module named A:B 
> > the loader notices this pairing when it pre-loads the module, so that when 
> > we ask for "A:B" the loader already knows which on-disk file contains. it.
> > 
> > if we use the HeaderSearch mechanisms (when we want to figure out a 
> > module-name<=> filename pairing without loading the module) we can use 
> > -fmodule-name=A:B=A_B.pcm,
> > 
> > These mechanisms work today, but P1184 is a more flexible mechanism and 
> > avoids having  massive command lines with many -fmodule-file= cases.
> > 
> But what if we need to import multiple modules? In our current workflow, we 
> would like to compile importing module in following style:
> ```
> clang++ -std=c++20 -fprebuilt-module-path=path1 -fprebuilt-module-path=path2 
> ... unit.cpp(m) ...
> ```
> 
> And unit.cpp(m) may look like:
> ```
> export module M;
> import :parta;
> import :partb;
> import :partc;
> ```
> 
> And our compiler would lookup `M-parta.pcm`, `M-partb.pcm` and `M-partc.pcm` 
> in the path given in the command line. However, in current implementation, it 
> would lookup `M:parta.pcm`, `M:partb.pcm` and `M:partc.pcm` in the path but 
> it might fail. So my point here is that the current implementation doesn't 
> work well with `fprebuilt-module-path`. And I don't think we should give up 
> `fprebuilt-module-path` to support multiple `fmodule-file`. The 
> `fprebuilt-module-path` is easier to understand and use for real projects. 
> Even if we support multiple `-fmodule-file`, people might be frustrating to 
> add many `fmodule-file` if they need to import many units.
> 
> So I really insist on this. Or at least let's add one option, something like 
> `-fmodule-partition-separator` here.
The semantics of clang hierarchical module names are different from C++20 
module names.

C++20 does not specify any relationship between the module name and the 
filename - that task is for the user, build system (or in the case we have with 
clang where it can integrate some of that functionality, the lookup mechanism).

out of interest if the user puts:

```
export module A.B:C.D;
```

how do you represent that on disk in your implementation?



In my understanding, if the user does:

```
export module A.B:C.D;

clang -cc1 -std=c++20 -emit-module-interface foo.cpp -o xyzzy.pcm
```
then we should see :

```
clang -module-file-info xyzzy.pcm

Information for module file 'xyzzy.pcm':
  Module format: raw
  Generated by this Clang: (/repos/llvm-git.git 
97fc6544d6a09f161d90417db05674a2b3d2a5fe)
  Module name: A.B:C.D

```

I think it would be wrong if that printed:
` Module name: A.B-C.D`

The primary module name is A.B (_not_ A) and there is no implication that we 
would see
A/B/...

What would happen if we ported clang to some (unknown, unlikely) platform where 
'-' was not a legal pathname character - we should not expect to have to change 
Sema for this

[PATCH] D112916: Confusable identifiers detection

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D112916#3314184 , @cor3ntin wrote:

> @aaron.ballman Thanks for the ping.
>
> One one hand, I agree with you, on the other hand, this tries to stick to 
> TR39, and I think we should stick with that. It might be worth checking with 
> the Unicode consortium what they think of i/l as confusable.

Actually @aaron.ballman, Unicode does consider these confusables already

from https://www.unicode.org/Public/security/14.0.0/confusables.txt

  0031 ;006C ;  MA  # ( 1 → l ) DIGIT ONE → LATIN SMALL LETTER L
# 
  0030 ;004F ;  MA  # ( 0 → O ) DIGIT ZERO → LATIN CAPITAL LETTER O 
# 

So ASCII is already taken care of. No issue here :)


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

https://reviews.llvm.org/D112916

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

[PATCH] D119829: [Driver] Support Solaris/amd64 GetTls

[Rainer Orth via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/solaris-ld-sanitizer.c:1
+// General tests that the ld -z relax=transtls workaround is only applied
+// on Solaris/amd64. Note that we use sysroot to make these tests

MaskRay wrote:
> Optional: some folks prefer an alternative comment marker (`///` for C, `##` 
> for assembly, etc) for non-RUN non-CHECK comments. It makes the comments 
> stand out (they may render differently in an editor). Many tests have 
> check-prefix typos and if we teach lit or FileCheck to catch such typos, we 
> can let them skip `///` lines to avoid false positives.
Seems quite sensible: patch amended.



Comment at: clang/test/Driver/solaris-ld-sanitizer.c:6
+// Check sparc-sun-solaris2.11, 32bit
+// RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=sparc-sun-solaris2.11 \

MaskRay wrote:
> Remove -no-canonical-prefixes . See some cleanup advice on D119309.
Done, as well as `-o %t.o`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119829

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

[PATCH] D119829: [Driver] Support Solaris/amd64 GetTls

[Rainer Orth via Phabricator via cfe-commits]

ro updated this revision to Diff 409881.
ro marked 2 inline comments as done.
ro added a comment.

Test fixes as per review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119829

Files:
  clang/lib/Driver/ToolChains/Solaris.cpp
  clang/test/Driver/solaris-ld-sanitizer.c

Index: clang/test/Driver/solaris-ld-sanitizer.c
===
--- /dev/null
+++ clang/test/Driver/solaris-ld-sanitizer.c
@@ -0,0 +1,67 @@
+/// General tests that the ld -z relax=transtls workaround is only applied
+/// on Solaris/amd64. Note that we use sysroot to make these tests
+/// independent of the host system.
+
+/// Check sparc-sun-solaris2.11, 32bit
+// RUN: %clang %s -### 2>&1 \
+// RUN: --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_sparc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC32 %s
+// CHECK-LD-SPARC32-NOT: -zrelax=transtls
+
+/// Check sparc-sun-solaris2.11, 32bit
+// RUN: %clang -fsanitize=undefined %s -### 2>&1 \
+// RUN: --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_sparc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC32 %s
+// CHECK-LD-SPARC32-NOT: -zrelax=transtls
+
+/// Check sparc-sun-solaris2.11, 64bit
+// RUN: %clang -m64 %s -### 2>&1 \
+// RUN: --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_sparc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC64 %s
+// CHECK-LD-SPARC64-NOT: -zrelax=transtls
+
+/// Check sparc-sun-solaris2.11, 64bit
+// RUN: %clang -m64 -fsanitize=undefined %s -### 2>&1 \
+// RUN: --target=sparc-sun-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_sparc_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-SPARC64 %s
+// CHECK-LD-SPARC64-NOT: -zrelax=transtls
+
+/// Check i386-pc-solaris2.11, 32bit
+// RUN: %clang %s -### 2>&1 \
+// RUN: --target=i386-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_x86_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-X32 %s
+// CHECK-LD-X32-NOT: -zrelax=transtls
+
+/// Check i386-pc-solaris2.11, 32bit
+// RUN: %clang -fsanitize=undefined %s -### 2>&1 \
+// RUN: --target=i386-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_x86_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-X32 %s
+// CHECK-LD-X32-NOT: -zrelax=transtls
+
+/// Check i386-pc-solaris2.11, 64bit
+// RUN: %clang -m64 %s -### 2>&1 \
+// RUN: --target=i386-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_x86_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-X64 %s
+// CHECK-LD-X64-NOT: -zrelax=transtls
+
+/// Check i386-pc-solaris2.11, 64bit
+// RUN: %clang -m64 -fsanitize=undefined %s -### 2>&1 \
+// RUN: --target=i386-pc-solaris2.11 \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/solaris_x86_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-LD-X64-UBSAN %s
+// CHECK-LD-X64-UBSAN: -zrelax=transtls
Index: clang/lib/Driver/ToolChains/Solaris.cpp
===
--- clang/lib/Driver/ToolChains/Solaris.cpp
+++ clang/lib/Driver/ToolChains/Solaris.cpp
@@ -14,6 +14,8 @@
 #include "clang/Driver/Driver.h"
 #include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Driver/Options.h"
+#include "clang/Driver/SanitizerArgs.h"
+#include "clang/Driver/ToolChain.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -145,8 +147,18 @@
   CmdArgs.push_back("-lgcc");
   CmdArgs.push_back("-lm");
 }
-if (NeedsSanitizerDeps)
+if (NeedsSanitizerDeps) {
   linkSanitizerRuntimeDeps(getToolChain(), CmdArgs);
+
+  // Work around Solaris/amd64 ld bug when calling __tls_get_addr directly.
+  // However, ld -z relax=transtls is available since Solaris 11.2, but not
+  // in Illumos.
+  const SanitizerArgs &SA = getToolChain().getSanitizerArgs(Args);
+  if (getToolChain().getTriple().getArch() == llvm::Triple::x86_64 &&
+  (SA.needsAsanRt() || SA.needsStatsRt() ||
+   (SA.needsUbsanRt() && !SA.requiresMinimalRuntime(
+CmdArgs.push_back("-zrelax=transtls");
+}
   }
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles)) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

It appears that this is causing an assertion segfault in a `rustc` test over at 
our experimental rust + llvm@head bot:
https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/8430#167e6de5-2dd5-41c3-87d7-b6e3f3908371/262-706
The test is 
https://github.com/rust-lang/rust/blob/master/src/test/assembly/asm/riscv-types.rs.
 These two lines appear to cause it (code compiles fine when removed):

- `check_reg!(a0_f32 f32 "a0" "mv");` 
https://github.com/rust-lang/rust/blob/f838a425e3134d036a7d9632935111a569ac7446/src/test/assembly/asm/riscv-types.rs#L178
- `check_reg!(a0_f64 f64 "a0" "mv");` 
https://github.com/rust-lang/rust/blob/f838a425e3134d036a7d9632935111a569ac7446/src/test/assembly/asm/riscv-types.rs#L192

The assertion:

  Impossible reg-to-reg copy
  UNREACHABLE executed at 
[...]/rust/src/llvm-project/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:350

The IR for riscv-types.rs:

  ; ModuleID = 'riscv_types.4cedf4b7-cgu.0'
  source_filename = "riscv_types.4cedf4b7-cgu.0"
  target datalayout = "e-m:e-p:64:64-i64:64-i128:128-n64-S128"
  target triple = "riscv64"
  
  @extern_static = external dso_local global i8
  @alloc55 = private unnamed_addr constant <{ [6 x i8] }> <{ [6 x i8] c"reg_i8" 
}>, align 1
  @alloc56 = private unnamed_addr constant <{ [7 x i8] }> <{ [7 x i8] 
c"reg_i16" }>, align 1
  @alloc57 = private unnamed_addr constant <{ [7 x i8] }> <{ [7 x i8] 
c"reg_i32" }>, align 1
  @alloc58 = private unnamed_addr constant <{ [7 x i8] }> <{ [7 x i8] 
c"reg_f32" }>, align 1
  @alloc59 = private unnamed_addr constant <{ [7 x i8] }> <{ [7 x i8] 
c"reg_i64" }>, align 1
  @alloc60 = private unnamed_addr constant <{ [7 x i8] }> <{ [7 x i8] 
c"reg_f64" }>, align 1
  @alloc61 = private unnamed_addr constant <{ [7 x i8] }> <{ [7 x i8] 
c"reg_ptr" }>, align 1
  @alloc62 = private unnamed_addr constant <{ [8 x i8] }> <{ [8 x i8] 
c"freg_f32" }>, align 1
  @alloc63 = private unnamed_addr constant <{ [8 x i8] }> <{ [8 x i8] 
c"freg_f64" }>, align 1
  @alloc64 = private unnamed_addr constant <{ [5 x i8] }> <{ [5 x i8] c"a0_i8" 
}>, align 1
  @alloc65 = private unnamed_addr constant <{ [6 x i8] }> <{ [6 x i8] c"a0_i16" 
}>, align 1
  @alloc66 = private unnamed_addr constant <{ [6 x i8] }> <{ [6 x i8] c"a0_i32" 
}>, align 1
  @alloc67 = private unnamed_addr constant <{ [6 x i8] }> <{ [6 x i8] c"a0_f32" 
}>, align 1
  @alloc68 = private unnamed_addr constant <{ [6 x i8] }> <{ [6 x i8] c"a0_i64" 
}>, align 1
  @alloc69 = private unnamed_addr constant <{ [6 x i8] }> <{ [6 x i8] c"a0_f64" 
}>, align 1
  @alloc70 = private unnamed_addr constant <{ [6 x i8] }> <{ [6 x i8] c"a0_ptr" 
}>, align 1
  @alloc71 = private unnamed_addr constant <{ [7 x i8] }> <{ [7 x i8] 
c"fa0_f32" }>, align 1
  @alloc72 = private unnamed_addr constant <{ [7 x i8] }> <{ [7 x i8] 
c"fa0_f64" }>, align 1
  
  ; Function Attrs: nounwind
  define dso_local void @sym_fn() unnamed_addr #0 {
  start:
tail call void asm sideeffect alignstack "call ${0:c}", 
"s,~{vtype},~{vl},~{vxsat},~{vxrm},~{memory}"(void ()* nonnull @extern_func) 
#1, !srcloc !1
ret void
  }
  
  ; Function Attrs: nounwind
  define dso_local void @sym_static() unnamed_addr #0 {
  start:
tail call void asm sideeffect alignstack "lb t0, ${0:c}", 
"s,~{vtype},~{vl},~{vxsat},~{vxrm},~{memory}"(i8* nonnull @extern_static) #1, 
!srcloc !2
ret void
  }
  
  ; Function Attrs: nounwind
  define dso_local i8 @reg_i8(i8 %x) unnamed_addr #0 {
  start:
tail call void @dont_merge([0 x i8]* noalias noundef nonnull readonly align 
1 bitcast (<{ [6 x i8] }>* @alloc55 to [0 x i8]*), i64 6) #1
%0 = tail call i8 asm sideeffect alignstack "mv ${0}, ${1}", 
"=&r,r,~{vtype},~{vl},~{vxsat},~{vxrm},~{memory}"(i8 %x) #1, !srcloc !3
ret i8 %0
  }
  
  ; Function Attrs: nounwind
  define dso_local i16 @reg_i16(i16 %x) unnamed_addr #0 {
  start:
tail call void @dont_merge([0 x i8]* noalias noundef nonnull readonly align 
1 bitcast (<{ [7 x i8] }>* @alloc56 to [0 x i8]*), i64 7) #1
%0 = tail call i16 asm sideeffect alignstack "mv ${0}, ${1}", 
"=&r,r,~{vtype},~{vl},~{vxsat},~{vxrm},~{memory}"(i16 %x) #1, !srcloc !3
ret i16 %0
  }
  
  ; Function Attrs: nounwind
  define dso_local i32 @reg_i32(i32 %x) unnamed_addr #0 {
  start:
tail call void @dont_merge([0 x i8]* noalias noundef nonnull readonly align 
1 bitcast (<{ [7 x i8] }>* @alloc57 to [0 x i8]*), i64 7) #1
%0 = tail call i32 asm sideeffect alignstack "mv ${0}, ${1}", 
"=&r,r,~{vtype},~{vl},~{vxsat},~{vxrm},~{memory}"(i32 %x) #1, !srcloc !3
ret i32 %0
  }
  
  ; Function Attrs: nounwind
  define dso_local float @reg_f32(float %x) unnamed_addr #0 {
  start:
tail call void @dont_merge([0 x i8]* noalias noundef nonnull readonly align 
1 bitcast (<{ [7 x i8] }>* @alloc58 to [0 x i8]*), i64 7) #1
%0 = tail call float asm sideeffect alignstack "mv ${0}, ${1}", 
"=&r,r,~{vtype},~{vl},~{vxsat},~{vxrm},~{memory}"(float %x) #1, !srcloc !3
ret float %0
  }
  
  ; Fu

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

Well, this patch is just a band-aid and a disaster waiting to happen.
If kmalloc is tagged with an __attribute__ stating the allocation size, then 
you can't dereference beyond that limit or risk the alias analysis do something 
you don't want. You are triggering UB like that.
Can't you just remove the __attribute__ from kmalloc instead to avoid 
triggering UB?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119816

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

[clang] 5086cff - Revert "unbreak Modules/cxx20-export-import.cpp with LLVM_APPEND_VC_REV after 32b73bc6ab82"

[Florian Hahn via cfe-commits]

Author: Florian Hahn
Date: 2022-02-18T11:04:05Z
New Revision: 5086cff04eec4327acc22a90466854ad4d89d570

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

LOG: Revert "unbreak Modules/cxx20-export-import.cpp with LLVM_APPEND_VC_REV 
after 32b73bc6ab82"

This reverts commit 1689b1092ebb2c630f8ef1d3880a9fb4808d16fa.

This patch was only added to fix a failure with 32b73bc6ab8234b, which
has been reverted again.

Added: 


Modified: 
clang/include/clang/Serialization/ASTBitCodes.h

Removed: 




diff  --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index c94274ff34b8f..f98e173b158c1 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -41,7 +41,7 @@ namespace serialization {
 /// Version 4 of AST files also requires that the version control branch and
 /// revision match exactly, since there is no backward compatibility of
 /// AST files at this time.
-const unsigned VERSION_MAJOR = 16;
+const unsigned VERSION_MAJOR = 15;
 
 /// AST file minor version number supported by this version of
 /// Clang.



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

[clang] 09193f2 - Revert "Add support for floating-point option `ffp-eval-method` and for"

[Florian Hahn via cfe-commits]

Author: Florian Hahn
Date: 2022-02-18T11:04:00Z
New Revision: 09193f20a13e8c3f4196dcc0883d74396f44c3cf

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

LOG: Revert "Add support for floating-point option `ffp-eval-method` and for"

This reverts commit 32b73bc6ab8234b670c34d5ef999300e072cc706.

This breaks builds on macOS in some configurations, because
__FLT_EVAL_METHOD__ is set to an unexpected value.

E.g.
https://green.lab.llvm.org/green/job/clang-stage1-RA/28282/consoleFull#129538464349ba4694-19c4-4d7e-bec5-911270d8a58c

More details available in the review thread
https://reviews.llvm.org/D109239

Added: 


Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/UsersManual.rst
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/FPOptions.def
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/include/clang/Basic/TargetInfo.h
clang/include/clang/Driver/Options.td
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/lib/Basic/Targets/OSTargets.h
clang/lib/Basic/Targets/X86.h
clang/lib/Driver/ToolChains/Clang.cpp
clang/lib/Frontend/InitPreprocessor.cpp
clang/lib/Lex/PPMacroExpansion.cpp
clang/lib/Parse/ParsePragma.cpp
clang/lib/Parse/ParseStmt.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaAttr.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CodeGen/fp-floatcontrol-pragma.cpp
clang/test/Preprocessor/init-aarch64.c
clang/test/Preprocessor/init-arm.c
clang/test/Preprocessor/init-mips.c
clang/test/Preprocessor/init-ppc.c
clang/test/Preprocessor/init-ppc64.c
clang/test/Preprocessor/init-s390x.c
clang/test/Preprocessor/init-v7k-compat.c
clang/test/Preprocessor/init-x86.c
clang/test/Preprocessor/init.c

Removed: 
clang/test/CodeGen/X86/32bit-behavior-no-eval.c
clang/test/CodeGen/X86/32bit-behavior.c
clang/test/CodeGen/X86/fp-eval-method.c
clang/test/CodeGen/flt_eval_macro.cpp
clang/test/Preprocessor/flt_eval_macro.cpp
clang/test/Sema/fp-eval-pragma.cpp
clang/test/Sema/x86-eval-method.c
clang/test/Sema/x86_64-eval-method.c



diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 5249d3f3f7930..f45d88092eb4a 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -3907,38 +3907,6 @@ A ``#pragma clang fp`` pragma may contain any number of 
options:
 ...
   }
 
-``#pragma clang fp eval_method`` allows floating-point behavior to be specified
-for a section of the source code. This pragma can appear at file or namespace
-scope, or at the start of a compound statement (excluding comments).
-The pragma is active within the scope of the compound statement.
-
-When ``pragma clang fp eval_method(source)`` is enabled, the section of code
-governed by the pragma behaves as though the command-line option
-``-ffp-eval-method=source`` is enabled. Rounds intermediate results to
-source-defined precision.
-
-When ``pragma clang fp eval_method(double)`` is enabled, the section of code
-governed by the pragma behaves as though the command-line option
-``-ffp-eval-method=double`` is enabled. Rounds intermediate results to
-``double`` precision.
-
-When ``pragma clang fp eval_method(extended)`` is enabled, the section of code
-governed by the pragma behaves as though the command-line option
-``-ffp-eval-method=extended`` is enabled. Rounds intermediate results to
-target-dependent ``long double`` precision. In Win32 programming, for instance,
-the long double data type maps to the double, 64-bit precision data type.
-
-The full syntax this pragma supports is
-``#pragma clang fp eval_method(source|double|extended)``.
-
-.. code-block:: c++
-
-  for(...) {
-// The compiler will use long double as the floating-point evaluation
-// method.
-#pragma clang fp eval_method(extended)
-a = b[i] * c[i] + e;
-  }
 
 The ``#pragma float_control`` pragma allows precise floating-point
 semantics and floating-point exception behavior to be specified

diff  --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 4a776eb86775c..981909aa16eaf 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1566,22 +1566,6 @@ Note that floating-point operations performed as part of 
constant initialization
* ``maytrap`` The compiler avoids transformations that may raise exceptions 
that would not have been raised by the original code. Constant folding 
performed by the compiler is exempt from this option.
* ``strict`` The compiler ensures that all transformations strictly 
preserve the floati

[PATCH] D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references

[Sam McCall via Phabricator via cfe-commits]

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

Great!
This is worth mentioning in the clang release notes. (Along with any matchers 
that match these new nodes)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119363

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

[clang-tools-extra] 535e7b0 - [clangd] lookupSiblingsWithinContext - remove unnecessary nullptr check

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-18T11:12:53Z
New Revision: 535e7b09c189dd3a7ef65bd36a02962f0c98bd5e

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

LOG: [clangd] lookupSiblingsWithinContext - remove unnecessary nullptr check

The DC pointer is always dereferenced after the loop

Added: 


Modified: 
clang-tools-extra/clangd/refactor/Rename.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/refactor/Rename.cpp 
b/clang-tools-extra/clangd/refactor/Rename.cpp
index b106664f0a446..46d884578d462 100644
--- a/clang-tools-extra/clangd/refactor/Rename.cpp
+++ b/clang-tools-extra/clangd/refactor/Rename.cpp
@@ -389,7 +389,7 @@ const NamedDecl *lookupSiblingsWithinContext(ASTContext 
&Ctx,
   DeclarationName LookupName(&II);
   DeclContextLookupResult LookupResult;
   const auto *DC = RenamedDecl.getDeclContext();
-  while (DC && DC->isTransparentContext())
+  while (DC->isTransparentContext())
 DC = DC->getParent();
   switch (DC->getDeclKind()) {
   // The enclosing DeclContext may not be the enclosing scope, it might have



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

[clang] 977b1f5 - [clang][ASTReader] Fix memory leak while reading FriendTemplateDecls

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2022-02-18T12:16:38+01:00
New Revision: 977b1f574fa18219fde5f709b906c79202ef1916

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

LOG: [clang][ASTReader] Fix memory leak while reading FriendTemplateDecls

Allocate on ASTContext, rather than just on heap, so that template
parameter lists are freed up.

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

Added: 


Modified: 
clang/lib/Serialization/ASTReaderDecl.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReaderDecl.cpp 
b/clang/lib/Serialization/ASTReaderDecl.cpp
index 25d7e9e6a2e68..29bef2aa20897 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2103,7 +2103,7 @@ void 
ASTDeclReader::VisitFriendTemplateDecl(FriendTemplateDecl *D) {
   VisitDecl(D);
   unsigned NumParams = Record.readInt();
   D->NumParams = NumParams;
-  D->Params = new TemplateParameterList*[NumParams];
+  D->Params = new (Reader.getContext()) TemplateParameterList *[NumParams];
   for (unsigned i = 0; i != NumParams; ++i)
 D->Params[i] = Record.readTemplateParameterList();
   if (Record.readInt()) // HasFriendDecl



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

[PATCH] D120081: [clang][ASTReader] Fix memory leak while reading FriendTemplateDecls

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG977b1f574fa1: [clang][ASTReader] Fix memory leak while 
reading FriendTemplateDecls (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120081

Files:
  clang/lib/Serialization/ASTReaderDecl.cpp


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2103,7 +2103,7 @@
   VisitDecl(D);
   unsigned NumParams = Record.readInt();
   D->NumParams = NumParams;
-  D->Params = new TemplateParameterList*[NumParams];
+  D->Params = new (Reader.getContext()) TemplateParameterList *[NumParams];
   for (unsigned i = 0; i != NumParams; ++i)
 D->Params[i] = Record.readTemplateParameterList();
   if (Record.readInt()) // HasFriendDecl


Index: clang/lib/Serialization/ASTReaderDecl.cpp
===
--- clang/lib/Serialization/ASTReaderDecl.cpp
+++ clang/lib/Serialization/ASTReaderDecl.cpp
@@ -2103,7 +2103,7 @@
   VisitDecl(D);
   unsigned NumParams = Record.readInt();
   D->NumParams = NumParams;
-  D->Params = new TemplateParameterList*[NumParams];
+  D->Params = new (Reader.getContext()) TemplateParameterList *[NumParams];
   for (unsigned i = 0; i != NumParams; ++i)
 D->Params[i] = Record.readTemplateParameterList();
   if (Record.readInt()) // HasFriendDecl
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 409892.
hokein marked 14 inline comments as done.
hokein added a comment.

- address comments
- more api polishments
- LRGraph unittest => lit tests
- hide LRTable::Builder, move to LRTableBuild.cpp


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118196

Files:
  clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h
  clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h
  clang/lib/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/lib/Tooling/Syntax/Pseudo/Grammar.cpp
  clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
  clang/lib/Tooling/Syntax/Pseudo/LRTable.cpp
  clang/lib/Tooling/Syntax/Pseudo/LRTableBuild.cpp
  clang/test/Syntax/check-cxx-bnf.test
  clang/test/Syntax/lr-build-basic.test
  clang/test/Syntax/lr-build-conflicts.test
  clang/tools/clang-pseudo/ClangPseudo.cpp
  clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/unittests/Tooling/Syntax/Pseudo/LRGraphTest.cpp
  clang/unittests/Tooling/Syntax/Pseudo/LRTableTest.cpp

Index: clang/unittests/Tooling/Syntax/Pseudo/LRTableTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/Pseudo/LRTableTest.cpp
@@ -0,0 +1,56 @@
+//===--- LRTableTest.cpp - ---*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/Pseudo/LRTable.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Tooling/Syntax/Pseudo/Grammar.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace syntax {
+namespace pseudo {
+namespace {
+
+using testing::IsEmpty;
+using testing::UnorderedElementsAre;
+using Action = LRTable::Action;
+
+TEST(LRTable, Builder) {
+  GrammarTable GTable;
+
+  //   eof   semi  ...
+  // +---++---+---
+  // |state0 || s0,r0 |...
+  // |state1 | acc|   |...
+  // |state2 ||  r1   |...
+  // +---++---+---
+  std::vector Entries = {
+  {/* State */ 0, tokenSymbol(tok::semi), Action::shift(0)},
+  {/* State */ 0, tokenSymbol(tok::semi), Action::reduce(0)},
+  {/* State */ 1, tokenSymbol(tok::eof), Action::accept(2)},
+  {/* State */ 2, tokenSymbol(tok::semi), Action::reduce(1)}};
+  GrammarTable GT;
+  LRTable T = LRTable::buildForTests(GT, Entries);
+  EXPECT_THAT(T.find(0, tokenSymbol(tok::eof)), IsEmpty());
+  EXPECT_THAT(T.find(0, tokenSymbol(tok::semi)),
+  UnorderedElementsAre(Action::shift(0), Action::reduce(0)));
+  EXPECT_THAT(T.find(1, tokenSymbol(tok::eof)),
+  UnorderedElementsAre(Action::accept(2)));
+  EXPECT_THAT(T.find(1, tokenSymbol(tok::semi)), IsEmpty());
+  EXPECT_THAT(T.find(2, tokenSymbol(tok::semi)),
+  UnorderedElementsAre(Action::reduce(1)));
+  // Verify the behaivor for other non-available-actions terminals.
+  EXPECT_THAT(T.find(2, tokenSymbol(tok::kw_int)), IsEmpty());
+}
+
+} // namespace
+} // namespace pseudo
+} // namespace syntax
+} // namespace clang
Index: clang/unittests/Tooling/Syntax/Pseudo/LRGraphTest.cpp
===
--- clang/unittests/Tooling/Syntax/Pseudo/LRGraphTest.cpp
+++ /dev/null
@@ -1,84 +0,0 @@
-//===--- LRGraphTest.cpp - LRGraph tests -*- C++-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/Tooling/Syntax/Pseudo/LRGraph.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
-#include 
-
-namespace clang {
-namespace syntax {
-namespace pseudo {
-namespace {
-
-TEST(LRGraph, Build) {
-  struct TestCase {
-llvm::StringRef BNF;
-llvm::StringRef ExpectedStates;
-  };
-
-  TestCase Cases[] = {{
-  R"bnf(
-_ := expr
-expr := IDENTIFIER
-  )bnf",
-  R"(States:
-State 0
-_ :=  • expr
-expr :=  • IDENTIFIER
-State 1
-_ := expr • 
-State 2
-expr := IDENTIFIER • 
-0 ->[expr] 1
-0 ->[IDENTIFIER] 2
-)"},
-  {// A grammar with a S/R conflict in SLR table:
-   // (id-id)-id, or id-(id-id).
-   R"bnf(
-_ := expr
-expr := expr - expr  # S/R conflict at state 4 on '-' token
-expr := IDENTIFIER
-  )bnf",
-   R"(States:
-State 0
-_ :=  • expr
-expr :=  • expr - expr
-expr :=  • IDENTIFIER
-State 1
-_ := expr • 
-expr := expr • - expr
-State 2
-exp

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

[Haojian Wu via Phabricator via cfe-commits]

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

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118196

Files:
  clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h
  clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h
  clang/lib/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/lib/Tooling/Syntax/Pseudo/Grammar.cpp
  clang/lib/Tooling/Syntax/Pseudo/GrammarBNF.cpp
  clang/lib/Tooling/Syntax/Pseudo/LRTable.cpp
  clang/lib/Tooling/Syntax/Pseudo/LRTableBuild.cpp
  clang/test/Syntax/check-cxx-bnf.test
  clang/test/Syntax/lr-build-basic.test
  clang/test/Syntax/lr-build-conflicts.test
  clang/tools/clang-pseudo/ClangPseudo.cpp
  clang/unittests/Tooling/Syntax/Pseudo/CMakeLists.txt
  clang/unittests/Tooling/Syntax/Pseudo/LRGraphTest.cpp
  clang/unittests/Tooling/Syntax/Pseudo/LRTableTest.cpp

Index: clang/unittests/Tooling/Syntax/Pseudo/LRTableTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/Syntax/Pseudo/LRTableTest.cpp
@@ -0,0 +1,56 @@
+//===--- LRTableTest.cpp - ---*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/Syntax/Pseudo/LRTable.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Tooling/Syntax/Pseudo/Grammar.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+
+namespace clang {
+namespace syntax {
+namespace pseudo {
+namespace {
+
+using testing::IsEmpty;
+using testing::UnorderedElementsAre;
+using Action = LRTable::Action;
+
+TEST(LRTable, Builder) {
+  GrammarTable GTable;
+
+  //   eof   semi  ...
+  // +---++---+---
+  // |state0 || s0,r0 |...
+  // |state1 | acc|   |...
+  // |state2 ||  r1   |...
+  // +---++---+---
+  std::vector Entries = {
+  {/* State */ 0, tokenSymbol(tok::semi), Action::shift(0)},
+  {/* State */ 0, tokenSymbol(tok::semi), Action::reduce(0)},
+  {/* State */ 1, tokenSymbol(tok::eof), Action::accept(2)},
+  {/* State */ 2, tokenSymbol(tok::semi), Action::reduce(1)}};
+  GrammarTable GT;
+  LRTable T = LRTable::buildForTests(GT, Entries);
+  EXPECT_THAT(T.find(0, tokenSymbol(tok::eof)), IsEmpty());
+  EXPECT_THAT(T.find(0, tokenSymbol(tok::semi)),
+  UnorderedElementsAre(Action::shift(0), Action::reduce(0)));
+  EXPECT_THAT(T.find(1, tokenSymbol(tok::eof)),
+  UnorderedElementsAre(Action::accept(2)));
+  EXPECT_THAT(T.find(1, tokenSymbol(tok::semi)), IsEmpty());
+  EXPECT_THAT(T.find(2, tokenSymbol(tok::semi)),
+  UnorderedElementsAre(Action::reduce(1)));
+  // Verify the behaivor for other non-available-actions terminals.
+  EXPECT_THAT(T.find(2, tokenSymbol(tok::kw_int)), IsEmpty());
+}
+
+} // namespace
+} // namespace pseudo
+} // namespace syntax
+} // namespace clang
Index: clang/unittests/Tooling/Syntax/Pseudo/LRGraphTest.cpp
===
--- clang/unittests/Tooling/Syntax/Pseudo/LRGraphTest.cpp
+++ /dev/null
@@ -1,84 +0,0 @@
-//===--- LRGraphTest.cpp - LRGraph tests -*- C++-*-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "clang/Tooling/Syntax/Pseudo/LRGraph.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
-#include 
-
-namespace clang {
-namespace syntax {
-namespace pseudo {
-namespace {
-
-TEST(LRGraph, Build) {
-  struct TestCase {
-llvm::StringRef BNF;
-llvm::StringRef ExpectedStates;
-  };
-
-  TestCase Cases[] = {{
-  R"bnf(
-_ := expr
-expr := IDENTIFIER
-  )bnf",
-  R"(States:
-State 0
-_ :=  • expr
-expr :=  • IDENTIFIER
-State 1
-_ := expr • 
-State 2
-expr := IDENTIFIER • 
-0 ->[expr] 1
-0 ->[IDENTIFIER] 2
-)"},
-  {// A grammar with a S/R conflict in SLR table:
-   // (id-id)-id, or id-(id-id).
-   R"bnf(
-_ := expr
-expr := expr - expr  # S/R conflict at state 4 on '-' token
-expr := IDENTIFIER
-  )bnf",
-   R"(States:
-State 0
-_ :=  • expr
-expr :=  • expr - expr
-expr :=  • IDENTIFIER
-State 1
-_ := expr • 
-expr := expr • - expr
-State 2
-expr := IDENTIFIER • 
-State 3
-expr :=  • expr - expr
-expr := expr - • expr
-expr :=  • IDENTIFIER
-State 4
-expr := expr - expr • 
- 

[PATCH] D118196: [syntax][pseudo] Implement LR parsing table.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/Grammar.h:177
 };
+void initTerminals(std::vector &Terminals);
 

sammccall wrote:
> why is this exposed/required rather than being initialized by the 
> GrammarTable constructor?
> Since this is essentially static (must always correspond to tok::TokenKind) 
> it seems that GrammarTable could just have an ArrayRef and it could be 
> initialized by a lazy-init singleton:
> 
> ```
> // in Grammar.cpp
> static ArrayRef getTerminalNames() {
>   static std::vector *TerminalNames = []{
> 
>   };
>   return *TerminalNames;
> }
> ```
> 
> (I know eventually we'd like GrammarTable to be generated and have very 
> minimal dynamic init, but there are lots of other details needed e.g. we 
> can't statically initialize `vector` either, so we should cross that 
> bridge when we come to it)
fair enough, let's hide it in a GrammarTable contructor.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:68
+  // Terminal actions, corresponding to entries of ACTION table.
+  Error = 0,
+  // Shift to state n: move forward with the lookahead, and push state n

sammccall wrote:
> Error seems like an action we'd dynamically handle, rather than an 
> unreachable sentinel.
> 
> I'd prefer to call this sentinel and llvm_unreachable() on it in the relevant 
> places. Even if we do plan dynamic error actions, we have enough spare bits 
> to keep these two cases separate.
changed to Sentinel, and remove Error action for now.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:80
+  // Nonterminal actions, corresponding to entry of GOTO table.
+  // Go to state n: pust state n onto the state stack.
+  // Similar to Shift, but it is a nonterminal forward transition.

sammccall wrote:
> sammccall wrote:
> > again blank line between "nonterminal actions" and "go to state n"
> pust -> push?
> 
> I thought goto replaces the top of the stack rather than being pushed onto it.
> 
> e.g.
> stack is { stmt := . expr ; }, lookahead is IDENT, action is shift "expr := 
> IDENT ."
> stack is { stmt := . expr ; | expr := IDENT . }, lookahead is semi, action is 
> reduce
> stack is { stmt := . expr ; }, reduced symbol is expr, goto is state "stmt := 
> expr . ;"
> stack is { stmt := expr . ;}, lookahead is semi...
> 
> Line 3=>4 doesn't seem like a push
>  
> 
yeah, this is mostly right -- stack in step 2 has two frames, rather than a 
combined one.

The truth is:

1. stack is [{ _ := . stmt | stmt := . expr ; }], lookahead is IDENT, action is 
shift "expr := IDENT ." (push a new state to the stack)
2. stack is [{ _ := . stmt | stmt := . expr ; }, { expr := IDENT .  }], 
lookahead is semi, action is reduce
3. stack is [{ _ := . stmt | stmt := . expr ; }], reduced symbol is expr, goto 
is state "stmt := expr . ;"
4. stack is [{ _ := . stmt | stmt := . expr ;}, { stmt := expr . ;}], lookahead 
is semi, action is shift "stmt := expr ; ."
5. stack is  [{ _ := . stmt | stmt := . expr ;}, { stmt := expr . ;}, { stmt := 
expr ; .}], lookahead is eof, action is reduce "stmt := expr ;"
6. stack is [{ _ := . stmt  | stmt := . expr ;}], reduced symbol is stmt, goto 
state is "_ := stmt ."
7. stack is [{ _ := . stmt | stmt := . expr ;}, { _ := stmt .}], lookahead is 
eof, action is accept, the parsing is done

Step 3 => 4 is a push. 



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:106
+bool operator==(const Action &L) const { return value() == L.value(); }
+uint16_t value() const { return K << ValueBits | Value; };
+

sammccall wrote:
> maybe value()=>opaque() or asInteger()?
> 
> Partly to give a hint a bit more at the purpose, partly to avoid confusion of 
> `Value != value()`
renamed to `opaque`.



Comment at: clang/include/clang/Tooling/Syntax/Pseudo/LRTable.h:136
+
+  class Builder {
+  public:

sammccall wrote:
> This is quite a lot of surface (together with the DenseMapInfo stuff) to 
> expose.
> Currently it looks like it needs to be in the header so that the LRTableTest 
> can construct a table directly rather than going through LRGraph.
> 
> I think you could expose a narrower interface:
> ```
> struct Entry { ;... };
> // Specify exact table contents for testing.
> static LRTable buildForTests(ArrayRef);
> ```
> 
> Then the builder/densemapinfo can be moved to the cpp file, and both 
> `buildForTests` and `buildSLR` can use the builder internally. WDYT?
Yeah, I exposed the Builder mainly for testing purposes. The new interface 
looks good to me. 



Comment at: clang/lib/Tooling/Syntax/Pseudo/LRTable.cpp:97
+  auto Result = find(State, Nonterminal);
+  assert(Result.size() == 1 && Result.front().kind() == Action::GoTo);
+  return Result.front().getGoToState();

sammccall wrote:
> (moving the comment 

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

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

steakhal added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:368
   Loc makeNullWithType(QualType type) {
-return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type));
-  }
-
-  Loc makeNull() {
-return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
+assert((type->isPointerType() || type->isObjCObjectPointerType() ||
+type->isBlockPointerType() || type->isNullPtrType() ||

Add a comment that `isAnyPointerType()` won't work here.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:371
+type->isReferenceType()) &&
+   "makeNullWithType must use pointer type");
+QualType nullType = type;

But you also let reference types.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:372
-  Loc makeNull() {
-return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
   }

steakhal wrote:
> 
You should also remove the `BasicValueFactor::getZeroWithPtrWidth()` along with 
`BasicValueFactor::getIntWithPtrWidth()` since those suffer from the same issue.
Feel free to do that in a separate patch.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:372-376
+QualType nullType = type;
+if (type->isReferenceType()) {
+  nullType = Context.getPointerType(type->getPointeeType());
+}
+return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(nullType));





Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:123
 
+static QualType getTemplateSpecializationArgType(CheckerContext &C,
+ const CallEvent &Call,

You don't need the whole `CheckerContext`. Pass the `ASTContext` instead.
I would also recommend using `Idx` instead of `ndx`



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:133
+  assert(T.getKind() == TemplateArgument::Type);
+  QualType Ty = C.getASTContext().getPointerType(T.getAsType());
+  return Ty;

You should just return it. Oh wait, you also wrap it inside a pointer. The name 
of the function does not reflect this. Either rename it or hoist this to the 
callsite.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:402
+  auto NullVal = C.getSValBuilder().makeNullWithType(
+  CC->getCXXThisVal().getType(C.getASTContext()));
   State = State->set(ThisRegion, NullVal);

You should use the `CC->getDecl()->getThisType()` instead.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:660
+  auto ValueToUpdate =
+  C.getSValBuilder().makeNullWithType(Call.getResultType());
   State = State->set(ThisRegion, ValueToUpdate);

Similarly to the previous one, use `getThisType()`.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:764
   return false;
-auto NullVal = C.getSValBuilder().makeNull();
+auto NullVal = C.getSValBuilder().makeNullWithType(Call.getResultType());
 State = State->set(ThisRegion, NullVal);

Same here.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:799
+auto NullVal = C.getSValBuilder().makeNullWithType(
+getTemplateSpecializationArgType(C, Call, 0));
 State = State->set(OtherSmartPtrRegion, NullVal);

At first glance, all callsites have access to the `getThisType()`, so we 
shouldn't dig this up from the specialization decl.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:885
 
-auto NullVal = C.getSValBuilder().makeNull();
+auto NullVal = C.getSValBuilder().makeNullWithType(CallExpr->getType());
 // Explicitly tracking the region as null.

NoQ wrote:
> NoQ wrote:
> > I suspect that this type is going to be `bool` which is probably not what 
> > you want.
> I think it makes sense to add an assertion in `makeNullWithType()` to protect 
> against such cases. This will probably also allow you to cover this with a 
> test case.
You probably never want to use `Call.getResultType()` in this file for this 
context.
Something like this would be more appropriate: 
`cast(Call.getDecl())->getThisType()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601

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

[PATCH] D118352: [clang][ABI] New c++20 modules mangling scheme

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added a comment.

In D118352#3331396 , @ChuanqiXu wrote:

> Thanks for explanation. Now it looks good to me. Let's accept it formally 
> after the series of partition landed and so that we could add test about 
> partitions.

thanks,

We're going to have to adjust a bit when partitions land, and the global 
initializer is a separate set of patches only tangentially impacting this.  I 
don't think the project benefits from waiting.


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

https://reviews.llvm.org/D118352

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

[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:177
+  if (IsPartition) {
+ModuleName += ":";
+ModuleName += stringFromPath(Partition);

iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > urnathan wrote:
> > > > iains wrote:
> > > > > ChuanqiXu wrote:
> > > > > > I chose '-' in my implementation since here ModuleName refers to 
> > > > > > the name in filesystem, right? And I remember '-' is the choice of 
> > > > > > GCC. So let's keep consistency.
> > > > > This is not my understanding.
> > > > > 
> > > > > ModuleName is the "user-facing" name of the module that is described 
> > > > > in the source, and we use this in diagnostics etc.
> > > > > 
> > > > > The translation of that name to an on-disk name can be arbitrary and 
> > > > > there are several mechanisms in clang already (e.g. 
> > > > > -fmodule-file=A:B=foo.pcm) the module loader uses these to find the 
> > > > > module on the basis of its user-facing name where required.
> > > > > 
> > > > > When we have P1184, it is even more important that the interface is 
> > > > > supplied with the name that the user will put into the source code.
> > > > > 
> > > > > 
> > > > I agree with Iain, we should use ':' is module names here.  When 
> > > > mapping a named module to the file system some translation is needed 
> > > > because ':' is not permitted in file names on some filesystems (you 
> > > > know the ones!)
> > > (just to expand a little more)
> > > 
> > > the on-disk name needs to be chosen suitably for the platform and by the 
> > > user and/or the build system.
> > > 
> > > When the FE chooses a default filename (which is done in building jobs, 
> > > not in the Parser of course) it chooses one based on the source filename. 
> > >  It would be most unfortunate if the Parser/Sema needed to understand 
> > > platform filename rules.
> > > 
> > > When you do  'clang -module-file-info' (with the existing or updated 
> > > version) you should see the module name as per the source code, the 
> > > translation would only apply to the filename itself.
> > > 
> > > - to answer a later comment:
> > > 
> > > when we do -fmodule-file=A_B.pcm  and A_B.pcm contains a module named A:B 
> > > the loader notices this pairing when it pre-loads the module, so that 
> > > when we ask for "A:B" the loader already knows which on-disk file 
> > > contains. it.
> > > 
> > > if we use the HeaderSearch mechanisms (when we want to figure out a 
> > > module-name<=> filename pairing without loading the module) we can use 
> > > -fmodule-name=A:B=A_B.pcm,
> > > 
> > > These mechanisms work today, but P1184 is a more flexible mechanism and 
> > > avoids having  massive command lines with many -fmodule-file= cases.
> > > 
> > But what if we need to import multiple modules? In our current workflow, we 
> > would like to compile importing module in following style:
> > ```
> > clang++ -std=c++20 -fprebuilt-module-path=path1 
> > -fprebuilt-module-path=path2 ... unit.cpp(m) ...
> > ```
> > 
> > And unit.cpp(m) may look like:
> > ```
> > export module M;
> > import :parta;
> > import :partb;
> > import :partc;
> > ```
> > 
> > And our compiler would lookup `M-parta.pcm`, `M-partb.pcm` and 
> > `M-partc.pcm` in the path given in the command line. However, in current 
> > implementation, it would lookup `M:parta.pcm`, `M:partb.pcm` and 
> > `M:partc.pcm` in the path but it might fail. So my point here is that the 
> > current implementation doesn't work well with `fprebuilt-module-path`. And 
> > I don't think we should give up `fprebuilt-module-path` to support multiple 
> > `fmodule-file`. The `fprebuilt-module-path` is easier to understand and use 
> > for real projects. Even if we support multiple `-fmodule-file`, people 
> > might be frustrating to add many `fmodule-file` if they need to import many 
> > units.
> > 
> > So I really insist on this. Or at least let's add one option, something 
> > like `-fmodule-partition-separator` here.
> The semantics of clang hierarchical module names are different from C++20 
> module names.
> 
> C++20 does not specify any relationship between the module name and the 
> filename - that task is for the user, build system (or in the case we have 
> with clang where it can integrate some of that functionality, the lookup 
> mechanism).
> 
> out of interest if the user puts:
> 
> ```
> export module A.B:C.D;
> ```
> 
> how do you represent that on disk in your implementation?
> 
> 
> 
> In my understanding, if the user does:
> 
> ```
> export module A.B:C.D;
> 
> clang -cc1 -std=c++20 -emit-module-interface foo.cpp -o xyzzy.pcm
> ```
> then we should see :
> 
> ```
> clang -module-file-info xyzzy.pcm
> 
> Information for module file 'xyzzy.pcm':
>   Module format: raw
>   Generated by this Clang: (/repos/llvm-git.git 
> 97fc6544d6a09f161d90417db05674a2b3d2a5fe)
>   Module name: A.B:C.D
> 
> ```
> 
> I think it would be wrong if that printed:
> ` Module name: A.B-C.

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

[Vince Bridgers via Phabricator via cfe-commits]

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

refactor based on @steakhal comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
  clang/lib/StaticAnalyzer/Checkers/CastValueChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/lib/StaticAnalyzer/Core/SValBuilder.cpp

Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
@@ -61,7 +61,7 @@
 
 DefinedOrUnknownSVal SValBuilder::makeZeroVal(QualType type) {
   if (Loc::isLocType(type))
-return makeNull();
+return makeNullWithType(type);
 
   if (type->isIntegralOrEnumerationType())
 return makeIntVal(0, type);
@@ -359,7 +359,7 @@
 return makeBoolVal(cast(E));
 
   case Stmt::CXXNullPtrLiteralExprClass:
-return makeNull();
+return makeNullWithType(E->getType());
 
   case Stmt::CStyleCastExprClass:
   case Stmt::CXXFunctionalCastExprClass:
@@ -399,7 +399,7 @@
 
 if (Loc::isLocType(E->getType()))
   if (E->isNullPointerConstant(Ctx, Expr::NPC_ValueDependentIsNotNull))
-return makeNull();
+return makeNullWithType(E->getType());
 
 return None;
   }
Index: clang/lib/StaticAnalyzer/Core/RegionStore.cpp
===
--- clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -2410,7 +2410,7 @@
   SVal V;
 
   if (Loc::isLocType(T))
-V = svalBuilder.makeNull();
+V = svalBuilder.makeNullWithType(T);
   else if (T->isIntegralOrEnumerationType())
 V = svalBuilder.makeZeroVal(T);
   else if (T->isStructureOrClassType() || T->isArrayType()) {
Index: clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -460,7 +460,8 @@
 continue;
   } else {
 // If the cast fails on a pointer, bind to 0.
-state = state->BindExpr(CastE, LCtx, svalBuilder.makeNull());
+state = state->BindExpr(CastE, LCtx,
+svalBuilder.makeNullWithType(resultType));
   }
 } else {
   // If we don't know if the cast succeeded, conjure a new symbol.
@@ -498,7 +499,7 @@
 continue;
   }
   case CK_NullToPointer: {
-SVal V = svalBuilder.makeNull();
+SVal V = svalBuilder.makeNullWithType(CastE->getType());
 state = state->BindExpr(CastE, LCtx, V);
 Bldr.generateNode(CastE, Pred, state);
 continue;
Index: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp
@@ -549,8 +549,9 @@
   State->BindExpr(CE, C.getLocationContext(), *StreamVal);
   // Generate state for NULL return value.
   // Stream switches to OpenFailed state.
-  ProgramStateRef StateRetNull = State->BindExpr(CE, C.getLocationContext(),
- C.getSValBuilder().makeNull());
+  ProgramStateRef StateRetNull =
+  State->BindExpr(CE, C.getLocationContext(),
+  C.getSValBuilder().makeNullWithType(CE->getType()));
 
   StateRetNotNull =
   StateRetNotNull->set(StreamSym, StreamState::getOpened(Desc));
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -71,7 +71,8 @@
   bool handleMoveCtr(const CallEvent &Call, CheckerContext &C,
  const MemRegion *ThisRegion) const;
   bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
-const MemRegion *OtherSmartPtrRegion) const;
+const MemRegion *OtherSmartPtrRegion,
+const CallEvent &Call) const;
   void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
   bool handleComparisionOp(const CallEvent &Call, CheckerContext &C) const;
   bool handleOstreamOperator(const CallEvent &Call, CheckerContext &C) const;
@@ -379,11 +380,13 @@
 if (!ThisRegion)
   return fal

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/include/clang/Format/Format.h:2540-2541
   ///}
   /// \endcode
-  /// \version 13
+  /// \version 15
   bool IndentRequiresClause;

HazardyKnusperkeks wrote:
> 
I like that suggestion..


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119682

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

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I like `/// In clang-format 13 and 14 it was named ``IndentRequires``.` The 
code in theory should still support it so it should be documented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119682

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

[PATCH] D120115: [clangd] Tweak --query-driver to ignore slash direction on windows

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

See https://github.com/clangd/clangd/issues/1022


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120115

Files:
  clang-tools-extra/clangd/QueryDriverDatabase.cpp


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -38,12 +38,10 @@
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Types.h"
 #include "clang/Tooling/CompilationDatabase.h"
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -276,6 +274,10 @@
 // Single star, accept any sequence without a slash.
 RegStream << "[^/]*";
   }
+} else if (llvm::sys::path::is_separator(Glob[I]) &&
+   llvm::sys::path::is_separator('/') &&
+   llvm::sys::path::is_separator('\\')) {
+  RegStream << R"([/\\])"; // Accept either slash on windows.
 } else {
   RegStream << llvm::Regex::escape(Glob.substr(I, 1));
 }
@@ -293,6 +295,7 @@
   for (llvm::StringRef Glob : Globs)
 RegTexts.push_back(convertGlobToRegex(Glob));
 
+  // Tempting to pass IgnoreCase, but we don't the FS sensitivity precisely.
   llvm::Regex Reg(llvm::join(RegTexts, "|"));
   assert(Reg.isValid(RegTexts.front()) &&
  "Created an invalid regex from globs");


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -38,12 +38,10 @@
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Types.h"
 #include "clang/Tooling/CompilationDatabase.h"
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -276,6 +274,10 @@
 // Single star, accept any sequence without a slash.
 RegStream << "[^/]*";
   }
+} else if (llvm::sys::path::is_separator(Glob[I]) &&
+   llvm::sys::path::is_separator('/') &&
+   llvm::sys::path::is_separator('\\')) {
+  RegStream << R"([/\\])"; // Accept either slash on windows.
 } else {
   RegStream << llvm::Regex::escape(Glob.substr(I, 1));
 }
@@ -293,6 +295,7 @@
   for (llvm::StringRef Glob : Globs)
 RegTexts.push_back(convertGlobToRegex(Glob));
 
+  // Tempting to pass IgnoreCase, but we don't the FS sensitivity precisely.
   llvm::Regex Reg(llvm::join(RegTexts, "|"));
   assert(Reg.isValid(RegTexts.front()) &&
  "Created an invalid regex from globs");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

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

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

What about this test then: 
https://github.com/llvm/llvm-project/blob/b0a0df980927ca54a7840a1b0c9766e98c05039b/clang/test/CodeGen/sanitize-coverage.c#L74

Can you show an independent C reproducer where no_sanitize does not work for 
you?

Is there an LKML discussion?

I also think sprinkling no_sanitize for UBSAN is again the wrong solution, as 
it was also for [1]. UBSAN has a tendency to generate too many false positives 
in the kernel, and we just have to work on finding solutions that tackle the 
problem wholesale rather than adding more band-aids.

In this case, I think the right solution is to simply make ksize() kill the 
optimizer's ability to know the object size. By definition ksize() will return 
the "true" allocation size, and it is fair to assume once that's called, the 
caller wants to use the full object size for its size-class.

[1] 
https://lore.kernel.org/all/2021003519.1050494-1-tadeusz.st...@linaro.org/T/#u


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119816

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

[PATCH] D119537: [clangd] Treat 'auto' params as deduced if there's a single instantiation.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 409902.
sammccall marked 4 inline comments as done.
sammccall added a comment.

address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119537

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp
  clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp

Index: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp
@@ -80,8 +80,18 @@
   EXPECT_THAT(apply("template  void x() { ^auto y = T::z(); }"),
   StartsWith("fail: Could not deduce type for 'auto' type"));
 
-  ExtraArgs.push_back("-std=c++17");
+  ExtraArgs.push_back("-std=c++20");
   EXPECT_UNAVAILABLE("template  class Y;");
+
+  EXPECT_THAT(apply("auto X = [](^auto){};"),
+  StartsWith("fail: Could not deduce"));
+  EXPECT_EQ(apply("auto X = [](^auto){return 0;}; int Y = X(42);"),
+"auto X = [](int){return 0;}; int Y = X(42);");
+  EXPECT_THAT(apply("auto X = [](^auto){return 0;}; int Y = X(42) + X('c');"),
+  StartsWith("fail: Could not deduce"));
+  // FIXME: should work on constrained auto params, once SourceRange is fixed.
+  EXPECT_UNAVAILABLE("template concept C = true;"
+ "auto X = [](C ^auto *){return 0;};");
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -815,6 +815,12 @@
 }
   )cpp",
 
+  R"cpp(// auto lambda param where there's a single instantiation
+struct [[Bar]] {};
+auto Lambda = [](^auto){ return 0; };
+int x = Lambda(Bar{});
+  )cpp",
+
   R"cpp(// decltype(auto) in function return
 struct [[Bar]] {};
 ^decltype(auto) test() {
Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -179,20 +179,76 @@
   )cpp",
   "Bar",
   },
+  {
+  R"cpp(
+// Generic lambda param.
+struct Foo{};
+auto Generic = [](^auto x) { return 0; };
+int m = Generic(Foo{});
+  )cpp",
+  "struct Foo",
+  },
+  {
+  R"cpp(
+// Generic lambda instantiated twice, matching deduction.
+struct Foo{};
+using Bar = Foo;
+auto Generic = [](^auto x, auto y) { return 0; };
+int m = Generic(Bar{}, "one");
+int n = Generic(Foo{}, 2);
+  )cpp",
+  "struct Foo",
+  },
+  {
+  R"cpp(
+// Generic lambda instantiated twice, conflicting deduction.
+struct Foo{};
+auto Generic = [](^auto y) { return 0; };
+int m = Generic("one");
+int n = Generic(2);
+  )cpp",
+  nullptr,
+  },
+  {
+  R"cpp(
+// Generic function param.
+struct Foo{};
+int generic(^auto x) { return 0; }
+int m = generic(Foo{});
+  )cpp",
+  "struct Foo",
+  },
+  {
+  R"cpp(
+// More complicated param type involving auto.
+template  concept C = true;
+struct Foo{};
+int generic(C ^auto *x) { return 0; }
+const Foo *Ptr = nullptr;
+int m = generic(Ptr);
+  )cpp",
+  "const struct Foo",
+  },
   };
   for (Test T : Tests) {
 Annotations File(T.AnnotatedCode);
-auto AST = TestTU::withCode(File.code()).build();
+auto TU = TestTU::withCode(File.code());
+TU.ExtraArgs.push_back("-std=c++20");
+auto AST = TU.build();
 SourceManagerForFile SM("foo.cpp", File.code());
 
-SCOPED_TRACE(File.code());
+SCOPED_TRACE(T.AnnotatedCode);
 EXPECT_FALSE(File.points().empty());
 for (Position Pos : File.points()) {
   auto Location = sourceLocationInMainFile(SM.get(), Pos);
   ASSERT_TRUE(!!Location) << llvm::toString(Location.takeError());
   auto DeducedType = getDeducedType(AST.getASTContext(), *Location);
-  ASSERT_TRUE(DeducedType);
-  EXPECT_EQ(DeducedType->getAsString(), T.DeducedType);
+  if (T.DeducedType == nullptr) {
+EXPECT_FALSE(DeducedType);
+  } else {
+ASSERT_TRUE(DeducedType);
+EXPECT_EQ(DeducedType->getAsString(), T.DeducedType);
+  

[PATCH] D119537: [clangd] Treat 'auto' params as deduced if there's a single instantiation.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D119537#3323096 , @Trass3r wrote:

> Is it intentional that the resolved type is not shown in the variable hover 
> (I guess, looking at the code):
>
>   param payload
>   Type: const auto &
>   
>   // In subscribe::(anonymous class)::operator()
>   const auto& payload
>
> Only when hovering over `auto`?
>
>   type-alias auto
>   struct Foo

Not intended per se but a limitation of the implementation approach: we do the 
analysis, but it's not feasible to track how the results would filter through 
the AST in general.
Maybe it's possible to do the easy+important cases, but this patch doesn't 
attempt it.

An alternative here is to have the selection walk the single instantiation, if 
there is one, instead of the template.
This is a more general/powerful approach.
I will try it out before landing this patch, but I suspect it'll cause some 
stuff to break in which case I'll move forward with this one.




Comment at: clang-tools-extra/clangd/AST.cpp:519
+  if (Spec->getTemplateSpecializationKind() != TSK_ImplicitInstantiation)
+continue;
+  // Find the type for this specialization.

nridge wrote:
> Should we `break` in this case? Otherwise there could be an explicit 
> specialization with a second type
I guess we're talking about this case:

```
void foo(auto X) {}
template <> void foo(char Y) {}
foo(0);
```

Now the meaning of the `auto` is tricky... if we're reading the body of the 
function, it's `int`. If we're reading the signature of foo it could be int or 
char. And obviously expand-auto-ing this into `int` would break the program.

I think it makes sense to be conservative and bail out if there are any 
explicit specializations for now. I expect this case to be rare.

On the other hand explicit instantiation is actually fine, so changed the 
condition here to fail on explicit specialization only.



Comment at: clang-tools-extra/clangd/AST.cpp:545
+  // TemplateTypeParmTypes for implicit TTPs, instead of AutoTypes.
+  // Also we don't look very hard, just stripping const, references, pointers.
+  static TemplateTypeParmTypeLoc findContainedAutoTTPLoc(TypeLoc TL) {

nridge wrote:
> Maybe call out an example like `vector` as a future nice-to-handle?
Done. Note that clang currently rejects `vector`, AFAICT it's valid 
though.



Comment at: clang-tools-extra/clangd/unittests/tweaks/ExpandAutoTypeTests.cpp:88
+  StartsWith("fail: Could not deduce"));
+  EXPECT_EQ(apply("auto X = [](^auto){return 0;}; int Y = X(42);"),
+"auto X = [](int){return 0;}; int Y = X(42);");

nridge wrote:
> Maybe add a conflicting-deduction case? (I know the GetDeducedType test 
> already has one, but here it would be especially wrong.)
> 
> Also, should we perhaps disable this in header files (maybe excepting 
> function-local symbols), since an including source file could have additional 
> instantiations?
> Maybe add a conflicting-deduction case? (I know the GetDeducedType test 
> already has one, but here it would be especially wrong.)
Done.

> Also, should we perhaps disable this in header files (maybe excepting 
> function-local symbols), since an including source file could have additional 
> instantiations?

Disabling features in headers seems sad. Detecting when the function is local 
doesn't seem like a great solution to this:
 - it's going to be hard to detect the common ways functions are hidden within 
headers (`detail` namespaces, inaccessible members, ...)
 - it's still going to be wrong when some instantiations come via (local) 
templates that are themselves instantiated from the including file

I suspect in most cases where multiple instantiations are intended we're going 
to see either no instantiations or many. Can we try it and see if there are 
false positive complaints?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119537

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

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

[Marco Elver via Phabricator via cfe-commits]

melver added a comment.

Right, I was able to repro this. The problem is the trap, which generally sucks 
that no_sanitize still leaves in the trap.

We also have -fno-sanitize-undefined-trap-on-error, which seems to have no 
effect either (should it?).

So I think there are 2 problems:

1. Clang still emitting traps even though it shouldn't.

2. The Linux kernel problem.

I think it's fine if you address problem 1 with this, as it's an oversight. But 
I think problem 2 wants to be solved differently as I suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119816

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

[PATCH] D119392: [Clang][OpenMP][Sema] Remove support for floating point values in atomic compare

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119392

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

[PATCH] D120106: [OpenMP] Add flag for disabling thread state in runtime

[Joseph Huber via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0870a4f59aef: [OpenMP] Add flag for disabling thread state 
in runtime (authored by jhuber6).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120106

Files:
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/OpenMP/target_globals_codegen.cpp
  openmp/libomptarget/DeviceRTL/include/Configuration.h
  openmp/libomptarget/DeviceRTL/src/Configuration.cpp
  openmp/libomptarget/DeviceRTL/src/State.cpp

Index: openmp/libomptarget/DeviceRTL/src/State.cpp
===
--- openmp/libomptarget/DeviceRTL/src/State.cpp
+++ openmp/libomptarget/DeviceRTL/src/State.cpp
@@ -285,7 +285,8 @@
 #pragma omp allocate(ThreadStates) allocator(omp_pteam_mem_alloc)
 
 uint32_t &lookupForModify32Impl(uint32_t ICVStateTy::*Var, IdentTy *Ident) {
-  if (OMP_LIKELY(TeamState.ICVState.LevelVar == 0))
+  if (OMP_LIKELY(!config::mayUseThreadStates() ||
+ TeamState.ICVState.LevelVar == 0))
 return TeamState.ICVState.*Var;
   uint32_t TId = mapping::getThreadIdInBlock();
   if (!ThreadStates[TId]) {
@@ -299,13 +300,13 @@
 
 uint32_t &lookup32Impl(uint32_t ICVStateTy::*Var) {
   uint32_t TId = mapping::getThreadIdInBlock();
-  if (OMP_UNLIKELY(ThreadStates[TId]))
+  if (OMP_UNLIKELY(config::mayUseThreadStates() && ThreadStates[TId]))
 return ThreadStates[TId]->ICVState.*Var;
   return TeamState.ICVState.*Var;
 }
 uint64_t &lookup64Impl(uint64_t ICVStateTy::*Var) {
   uint64_t TId = mapping::getThreadIdInBlock();
-  if (OMP_UNLIKELY(ThreadStates[TId]))
+  if (OMP_UNLIKELY(config::mayUseThreadStates() && ThreadStates[TId]))
 return ThreadStates[TId]->ICVState.*Var;
   return TeamState.ICVState.*Var;
 }
@@ -380,6 +381,9 @@
 }
 
 void state::enterDataEnvironment(IdentTy *Ident) {
+  ASSERT(config::mayUseThreadStates() &&
+ "Thread state modified while explicitly disabled!");
+
   unsigned TId = mapping::getThreadIdInBlock();
   ThreadStateTy *NewThreadState =
   static_cast(__kmpc_alloc_shared(sizeof(ThreadStateTy)));
@@ -388,6 +392,9 @@
 }
 
 void state::exitDataEnvironment() {
+  ASSERT(config::mayUseThreadStates() &&
+ "Thread state modified while explicitly disabled!");
+
   unsigned TId = mapping::getThreadIdInBlock();
   resetStateForThread(TId);
 }
Index: openmp/libomptarget/DeviceRTL/src/Configuration.cpp
===
--- openmp/libomptarget/DeviceRTL/src/Configuration.cpp
+++ openmp/libomptarget/DeviceRTL/src/Configuration.cpp
@@ -20,7 +20,9 @@
 
 #pragma omp declare target
 
-extern uint32_t __omp_rtl_debug_kind; // defined by CGOpenMPRuntimeGPU
+// defined by CGOpenMPRuntimeGPU
+extern uint32_t __omp_rtl_debug_kind;
+extern uint32_t __omp_rtl_assume_no_thread_state;
 
 // TODO: We want to change the name as soon as the old runtime is gone.
 // This variable should be visibile to the plugin so we override the default
@@ -48,4 +50,6 @@
   return config::getDebugKind() & Kind;
 }
 
+bool config::mayUseThreadStates() { return !__omp_rtl_assume_no_thread_state; }
+
 #pragma omp end declare target
Index: openmp/libomptarget/DeviceRTL/include/Configuration.h
===
--- openmp/libomptarget/DeviceRTL/include/Configuration.h
+++ openmp/libomptarget/DeviceRTL/include/Configuration.h
@@ -38,8 +38,13 @@
 /// Return the amount of dynamic shared memory that was allocated at launch.
 uint64_t getDynamicMemorySize();
 
+/// Return if debugging is enabled for the given debug kind.
 bool isDebugMode(DebugKind Level);
 
+/// Indicates if this kernel may require thread-specific states, or if it was
+/// explicitly disabled by the user.
+bool mayUseThreadStates();
+
 } // namespace config
 } // namespace _OMP
 
Index: clang/test/OpenMP/target_globals_codegen.cpp
===
--- clang/test/OpenMP/target_globals_codegen.cpp
+++ clang/test/OpenMP/target_globals_codegen.cpp
@@ -6,6 +6,7 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix=CHECK-DEFAULT
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-assume-threads-oversubscription -fopenmp-is-device -fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s --check-prefix=CHECK-THREADS
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown -fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-assume-teams-oversubscription -fopenmp-is-device -fopenmp-host-i

[clang] 0870a4f - [OpenMP] Add flag for disabling thread state in runtime

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2022-02-18T08:35:05-05:00
New Revision: 0870a4f59aef21bf7707b00ebd4dcad7ce7ef807

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

LOG: [OpenMP] Add flag for disabling thread state in runtime

The runtime uses thread state values to indicate when we use an ICV or
are in nested parallelism. This is done for OpenMP correctness, but it
not needed in the majority of cases. The new flag added is
`-fopenmp-assume-no-thread-state`.

Reviewed By: jdoerfert

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

Added: 


Modified: 
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/OpenMP/target_globals_codegen.cpp
openmp/libomptarget/DeviceRTL/include/Configuration.h
openmp/libomptarget/DeviceRTL/src/Configuration.cpp
openmp/libomptarget/DeviceRTL/src/State.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 4651f4fff6aa0..e21998860f217 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -246,6 +246,7 @@ LANGOPT(OpenMPTargetDebug , 32, 0, "Enable debugging in the 
OpenMP offloading de
 LANGOPT(OpenMPOptimisticCollapse  , 1, 0, "Use at most 32 bits to represent 
the collapsed loop nest counter.")
 LANGOPT(OpenMPThreadSubscription  , 1, 0, "Assume work-shared loops do not 
have more iterations than participating threads.")
 LANGOPT(OpenMPTeamSubscription  , 1, 0, "Assume distributed loops do not have 
more iterations than participating teams.")
+LANGOPT(OpenMPNoThreadState  , 1, 0, "Assume that no thread in a parallel 
region will modify an ICV.")
 LANGOPT(RenderScript  , 1, 0, "RenderScript")
 
 LANGOPT(CUDAIsDevice  , 1, 0, "compiling for CUDA device")

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 76cfdbcd85f26..c377329e8f6f4 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2473,6 +2473,10 @@ def fno_openmp_assume_teams_oversubscription : 
Flag<["-"], "fno-openmp-assume-te
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_assume_threads_oversubscription : Flag<["-"], 
"fno-openmp-assume-threads-oversubscription">,
   Group, Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
+def fopenmp_assume_no_thread_state : Flag<["-"], 
"fopenmp-assume-no-thread-state">, Group, 
+  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>, 
+  HelpText<"Assert no thread in a parallel region modifies an ICV">,
+  MarshallingInfoFlag>;
 defm openmp_target_new_runtime: BoolFOption<"openmp-target-new-runtime",
   LangOpts<"OpenMPTargetNewRuntime">, DefaultTrue,
   PosFlag,

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
index bb6847ab87319..fcaf9d4ed77b3 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
@@ -1210,6 +1210,8 @@ CGOpenMPRuntimeGPU::CGOpenMPRuntimeGPU(CodeGenModule &CGM)
 "__omp_rtl_assume_teams_oversubscription");
 OMPBuilder.createGlobalFlag(CGM.getLangOpts().OpenMPThreadSubscription,
 "__omp_rtl_assume_threads_oversubscription");
+OMPBuilder.createGlobalFlag(CGM.getLangOpts().OpenMPNoThreadState,
+"__omp_rtl_assume_no_thread_state");
   }
 }
 

diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index a16175ebebbca..32cbb7936f7ee 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5995,6 +5995,8 @@ void Clang::ConstructJob(Compilation &C, const JobAction 
&JA,
options::OPT_fno_openmp_assume_threads_oversubscription,
/*Default=*/false))
 CmdArgs.push_back("-fopenmp-assume-threads-oversubscription");
+  if (Args.hasArg(options::OPT_fopenmp_assume_no_thread_state))
+CmdArgs.push_back("-fopenmp-assume-no-thread-state");
   break;
 default:
   // By default, if Clang doesn't know how to generate useful OpenMP code

diff  --git a/clang/test/OpenMP/target_globals_codegen.cpp 
b/clang/test/OpenMP/target_globals_codegen.cpp
index fa7569cd4ca6b..3c5d4b8ed3984 100644
--- a/clang/test/OpenMP/target_globals_codegen.cpp
+++ b/clang/test/OpenMP/target_globals_codegen.cpp
@@ -6,6 +6,7 @@
 // RUN: %clang_cc1 -verify -fopenmp -x c++ -triple nvptx64-unknown-unknown 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-ppc-host.bc -o - | FileCheck %s 
--check-prefix=C

[PATCH] D120070: [HIP] Support linking archive of bundled bitcode

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/hip-link-bundle-archive.hip:3
+
+// RUN: touch %T/libhipBundled.a
+

yaxunl wrote:
> tra wrote:
> > Is this file necessary? `clang -###` should not need the file to be present 
> > in order to print out command lines.
> You are right. Will remove it when committing.
Sorry, AddStaticDeviceLibsLinking needs to determine the real path of the 
library by looking up multiple -L options, therefore it requires the file to 
exist.

I forgot to delete the file when testing, so I thought the test passes without 
creating the file. After I delete it, the test fails. Therefore I have to add 
back the touch command.


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

https://reviews.llvm.org/D120070

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

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Thank you for reverting my follow-up as well!

zahiraam, for the reland please include that follow-up fix in your commit :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109239

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

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D109239#3331914 , @thakis wrote:

> Thank you for reverting my follow-up as well!
>
> zahiraam, for the reland please include that follow-up fix in your commit :)

Yes. Will do that. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109239

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

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

[Vince Bridgers via Phabricator via cfe-commits]

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



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:372
-  Loc makeNull() {
-return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
   }

steakhal wrote:
> steakhal wrote:
> > 
> You should also remove the `BasicValueFactor::getZeroWithPtrWidth()` along 
> with `BasicValueFactor::getIntWithPtrWidth()` since those suffer from the 
> same issue.
> Feel free to do that in a separate patch.
Will do. I had also left "TODO" comments in a different source file. I'll find 
those, clean those up also.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119601

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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D93298#3331641 , @krasimir wrote:

> It appears that this is causing an assertion segfault in a `rustc` test over 
> at our experimental rust + llvm@head bot:
> https://buildkite.com/llvm-project/rust-llvm-integrate-prototype/builds/8430#167e6de5-2dd5-41c3-87d7-b6e3f3908371/262-706
> The test is 
> https://github.com/rust-lang/rust/blob/master/src/test/assembly/asm/riscv-types.rs.
>  These two lines appear to cause it (code compiles fine when removed):
>
> - `check_reg!(a0_f32 f32 "a0" "mv");` 
> https://github.com/rust-lang/rust/blob/f838a425e3134d036a7d9632935111a569ac7446/src/test/assembly/asm/riscv-types.rs#L178
> - `check_reg!(a0_f64 f64 "a0" "mv");` 
> https://github.com/rust-lang/rust/blob/f838a425e3134d036a7d9632935111a569ac7446/src/test/assembly/asm/riscv-types.rs#L192
>
> The assertion:
>
>   Impossible reg-to-reg copy
>   UNREACHABLE executed at 
> [...]/rust/src/llvm-project/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:350
>
> ...

This is just MC layer support. Inline assembly interacts with CodeGen; I assume 
the register classes/constraints still need updating to support Zfinx.




Comment at: llvm/test/MC/RISCV/rv32i-invalid.s:1
-# RUN: not llvm-mc -triple riscv32 < %s 2>&1 | FileCheck %s
 

This is an unrelated change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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

[PATCH] D99134: Lambdas are not necessarily locals. This resolves DR48250.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D99134#3331359 , @davidstone wrote:

> In D99134#3331259 , @rsmith wrote:
>
>> Looks good. Do you need someone to land this for you?
>
> Yes, I do.

I'll do that for you, working on it now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99134

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

[clang] 0bff3a9 - Lambdas are not necessarily locals. This resolves DR48250.

[Erich Keane via cfe-commits]

Author: David Stone
Date: 2022-02-18T06:11:28-08:00
New Revision: 0bff3a965022647fcdd17cc8f2217f5a2cd30b4c

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

LOG: Lambdas are not necessarily locals. This resolves DR48250.

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

Added: 


Modified: 
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 2e8ddc8242fb6..237886c906a5b 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6034,7 +6034,9 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, 
NamedDecl *D,
   (ParentDependsOnArgs && (ParentDC->isFunctionOrMethod() ||
isa(ParentDC) ||
isa(ParentDC))) ||
-  (isa(D) && cast(D)->isLambda())) {
+  (isa(D) && cast(D)->isLambda() &&
+   cast(D)->getTemplateDepth() >
+   TemplateArgs.getNumRetainedOuterLevels())) {
 // D is a local of some kind. Look into the map of local
 // declarations to their instantiations.
 if (CurrentInstantiationScope) {

diff  --git a/clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp 
b/clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
index 13fe12abe9e9d..a5410d2aed597 100644
--- a/clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
+++ b/clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
@@ -39,3 +39,13 @@ void c2() {
   const auto lambda = [&](auto arg1) {};
   [&](auto arg2) { lambda.operator()(arg2); }(0);
 }
+
+auto d = [](auto) {};
+
+template 
+void d1(T x) { d.operator()(x); }
+
+void d2() { d1(0); }
+
+template  int e1 = [](auto){ return T(); }.operator()(T());
+int e2 = e1;



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

[PATCH] D99134: Lambdas are not necessarily locals. This resolves DR48250.

[Erich Keane via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0bff3a965022: Lambdas are not necessarily locals. This 
resolves DR48250. (authored by davidstone, committed by erichkeane).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99134

Files:
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp


Index: clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
===
--- clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
+++ clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
@@ -39,3 +39,13 @@
   const auto lambda = [&](auto arg1) {};
   [&](auto arg2) { lambda.operator()(arg2); }(0);
 }
+
+auto d = [](auto) {};
+
+template 
+void d1(T x) { d.operator()(x); }
+
+void d2() { d1(0); }
+
+template  int e1 = [](auto){ return T(); }.operator()(T());
+int e2 = e1;
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6034,7 +6034,9 @@
   (ParentDependsOnArgs && (ParentDC->isFunctionOrMethod() ||
isa(ParentDC) ||
isa(ParentDC))) ||
-  (isa(D) && cast(D)->isLambda())) {
+  (isa(D) && cast(D)->isLambda() &&
+   cast(D)->getTemplateDepth() >
+   TemplateArgs.getNumRetainedOuterLevels())) {
 // D is a local of some kind. Look into the map of local
 // declarations to their instantiations.
 if (CurrentInstantiationScope) {


Index: clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
===
--- clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
+++ clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp
@@ -39,3 +39,13 @@
   const auto lambda = [&](auto arg1) {};
   [&](auto arg2) { lambda.operator()(arg2); }(0);
 }
+
+auto d = [](auto) {};
+
+template 
+void d1(T x) { d.operator()(x); }
+
+void d2() { d1(0); }
+
+template  int e1 = [](auto){ return T(); }.operator()(T());
+int e2 = e1;
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6034,7 +6034,9 @@
   (ParentDependsOnArgs && (ParentDC->isFunctionOrMethod() ||
isa(ParentDC) ||
isa(ParentDC))) ||
-  (isa(D) && cast(D)->isLambda())) {
+  (isa(D) && cast(D)->isLambda() &&
+   cast(D)->getTemplateDepth() >
+   TemplateArgs.getNumRetainedOuterLevels())) {
 // D is a local of some kind. Look into the map of local
 // declarations to their instantiations.
 if (CurrentInstantiationScope) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99134: Lambdas are not necessarily locals. This resolves DR48250.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D99134#3311363 , @jloser wrote:

> Would love to see this cherry-picked into the llvm 14 release branch if 
> possible!

I believe there is a bug in github that you can use to request that.  This just 
landed as 0bff3a965022647fcdd17cc8f2217f5a2cd30b4c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99134

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

[PATCH] D118034: [C++20] [Modules] Don't complain about duplicated default template argument across modules

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:2658-2682
+Expr::EvalResult EVRX, EVRY;
+if (!DefaultArgumentX->EvaluateAsConstantExpr(EVRX, C) ||
+!DefaultArgumentY->EvaluateAsConstantExpr(EVRY, C))
+  return false;
+
+APValue VX = EVRX.Val, VY = EVRY.Val;
+if (VX.getKind() != VY.getKind())

ChuanqiXu wrote:
> urnathan wrote:
> > urnathan wrote:
> > > ChuanqiXu wrote:
> > > > urnathan wrote:
> > > > > ChuanqiXu wrote:
> > > > > > urnathan wrote:
> > > > > > > I'm kind of surprised how complex this check is.  Isn't there an 
> > > > > > > AST comparator available somewhere?
> > > > > > I found ODRHash. I think it is much better now.
> > > > > hm that suggests there there must be a comparator too -- this isn't a 
> > > > > cryptographically strong hash is it?  How would the compiler 
> > > > > currently make use of 'definitely different' and 'probably the same' 
> > > > > without such a comparator?
> > > > Yeah, I am sure there is not an such comparator. Or it has some methods 
> > > > like: `ASTContext::hasSameType` for type and 
> > > > `ASTContext::isSameEntity()` for Decl. But it lacks such methods now 
> > > > for Stmt and Expr.
> > > > 
> > > > > How would the compiler currently make use of 'definitely different' 
> > > > > and 'probably the same' without such a comparator?
> > > > 
> > > > Now it uses the two methods I listed above and ODRHash to compare. I 
> > > > think the two methods works for  'definitely different' and ODRHash 
> > > > works for 'probably the same'. So it's the reason why my previous 
> > > > implementation looks lengthy. Since I want to handle it by hand. (The 
> > > > previous method only works for simple Expr. I think it would be large 
> > > > work to implement comparator for whole Expr or Stmt).
> > > Hm, how do template instantations work -- there must be some way of 
> > > determining 'is this instantation just here the same as one I've already 
> > > seen'
> > also, the same functionality (with more generality) is needed for comparing 
> > regular function default arguments with multiple definitions in the GM.  
> > How is that done (or is it yet to be implemented?)
> > also, the same functionality (with more generality) is needed for comparing 
> > regular function default arguments with multiple definitions in the GM. How 
> > is that done (or is it yet to be implemented?)
> 
> We implemented ODR check in ASTReader by ODRHash.
> 
> > Hm, how do template instantations work -- there must be some way of 
> > determining 'is this instantation just here the same as one I've already 
> > seen'
> 
> First, previously we don't allow the redefinition of template default 
> argument. And for the case without template default argument, previously we 
> would try find the definition for the synthesized type, if we could find one, 
> use it. And if we couldn't find one, initialize one.
> 
> ---
> 
> Given we already checks ODR by ODRHash in ASTReader, I think what we do here 
> might not be bad. I agree that it is odd at the first sight to use Hash to 
> detect difference from the perspective of CS. But if this is what we used to 
> do, I think it is acceptable.
indeed.  I would be more comfortable with a comment by someone more familiar 
with this @rsmith ?


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

https://reviews.llvm.org/D118034

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

[clang-tools-extra] c24e197 - [clangd] getHover - pass FormatStyle argument by const reference

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-18T14:45:51Z
New Revision: c24e197aaad5333717a27f6e4346f97da84821ed

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

LOG: [clangd] getHover - pass FormatStyle argument by const reference

Reported by coverity

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index dda5ad36e9b89..8b22018411404 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -963,7 +963,7 @@ void maybeAddCalleeArgInfo(const SelectionTree::Node *N, 
HoverInfo &HI,
 } // namespace
 
 llvm::Optional getHover(ParsedAST &AST, Position Pos,
-   format::FormatStyle Style,
+   const format::FormatStyle &Style,
const SymbolIndex *Index) {
   PrintingPolicy PP =
   getPrintingPolicy(AST.getASTContext().getPrintingPolicy());



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

[PATCH] D119537: [clangd] Treat 'auto' params as deduced if there's a single instantiation.

[Trass3r via Phabricator via cfe-commits]

Trass3r added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:491
+  // We'll examine visible specializations and see if they yield a unique type.
+  bool VisitParmVarDecl(ParmVarDecl *PVD) {
+if (!PVD->getType()->isDependentType())

Since this is implemented generically in the AST, can this functionality easily 
be reused for a type inlay hint?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119537

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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Shao-Ce SUN via Phabricator via cfe-commits]

achieveartificialintelligence added a comment.

@krasimir Since I don't have a rust environment, can you help me to test if 
D120130  works?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D93298#3332020 , 
@achieveartificialintelligence wrote:

> @krasimir Since I don't have a rust environment, can you help me to test if 
> D120130  works?

Hm, actually, looking at the log, it doesn't appear to be trying to use Zfinx? 
My guess is that putting a float in a GPR has broken as that's now legal but 
should only be legal for Zfinx?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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

[clang-tools-extra] 707157f - Revert rGc24e197aaad5333717a27f6e4346f97da84821ed "[clangd] getHover - pass FormatStyle argument by const reference"

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-18T14:59:57Z
New Revision: 707157f24834e814243c90cf1f5f50c75f3abcb9

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

LOG: Revert rGc24e197aaad5333717a27f6e4346f97da84821ed "[clangd] getHover - 
pass FormatStyle argument by const reference"

There are a number of buildbot build failures on non MSVC compilers

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 8b22018411404..dda5ad36e9b89 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -963,7 +963,7 @@ void maybeAddCalleeArgInfo(const SelectionTree::Node *N, 
HoverInfo &HI,
 } // namespace
 
 llvm::Optional getHover(ParsedAST &AST, Position Pos,
-   const format::FormatStyle &Style,
+   format::FormatStyle Style,
const SymbolIndex *Index) {
   PrintingPolicy PP =
   getPrintingPolicy(AST.getASTContext().getPrintingPolicy());



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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

>> It appears that this is causing an assertion segfault in a rustc test over 
>> at our experimental rust + llvm@head bot:

I dont think that patch author is required to debug this issue for 
"experimental rust + llvm@head" - downstream.

>> Since I don't have a rust environment

There is no requirement, indeed. LLVM devs are not responsible to keep 
""experimental rust + llvm@head" working.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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

[PATCH] D120131: [clangd] prototype: for singly-instantiated templates, examine the instantiation

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added projects: clang, clang-tools-extra.

This prototype is crude and breaks things (e.g. including rename).
It has this behavior for all selections and nothing else.
A real version would use it for *some* selections (not rename!) and some other
traversals too (syntax highlighting).

It does allow hover/go-to-definition to work in cases like:

  void callMeWithFoos(std::function);
  ...
  callMeWithFoos([](auto &&x) {
x.^bar(); // will jump to Foo::bar
  });

as well as other cases handled in D119537 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120131

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang/include/clang/AST/RecursiveASTVisitor.h

Index: clang/include/clang/AST/RecursiveASTVisitor.h
===
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -94,6 +94,28 @@
   SecondMethodPtrTy>::value>::isSameMethod(FirstMethodPtr, SecondMethodPtr);
 }
 
+// Paper over API difference between Function templates and others.
+template 
+TemplateSpecializationKind getTemplateSpecializationKind(TemplatedDeclTy *TD) {
+  return TD->getSpecializationKind();
+}
+
+inline TemplateSpecializationKind
+getTemplateSpecializationKind(FunctionDecl *FD) {
+  return FD->getTemplateSpecializationKind();
+}
+
+template 
+auto getTemplatedDecl(TemplateTy *D, bool SingleInstantiationInstead)
+-> decltype(D->getTemplatedDecl()) {
+  if (SingleInstantiationInstead && llvm::size(D->specializations()) == 1) {
+auto *Candidate = *D->spec_begin();
+if (getTemplateSpecializationKind(Candidate) != TSK_ExplicitSpecialization)
+  return Candidate;
+  }
+  return D->getTemplatedDecl();
+}
+
 } // end namespace detail
 
 /// A class that does preorder or postorder
@@ -178,6 +200,11 @@
   /// template instantiations.
   bool shouldVisitTemplateInstantiations() const { return false; }
 
+  /// When encountering a singly-instantiated template definition,
+  /// should we walk the instantiated body instead of the template body?
+  /// Ignored if shouldVisitTemplateInstantiations() is true.
+  bool shouldVisitSingleTemplateInstantiationInstead() const { return false; }
+
   /// Return whether this visitor should recurse into the types of
   /// TypeLocs.
   bool shouldWalkTypesOfTypeLocs() const { return true; }
@@ -1761,24 +1788,27 @@
 
 // This macro unifies the traversal of class, variable and function
 // template declarations.
-#define DEF_TRAVERSE_TMPL_DECL(TMPLDECLKIND)   \
-  DEF_TRAVERSE_DECL(TMPLDECLKIND##TemplateDecl, {  \
-TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));   \
-TRY_TO(TraverseDecl(D->getTemplatedDecl()));   \
-   \
-/* By default, we do not traverse the instantiations of\
-   class templates since they do not appear in the user code. The  \
-   following code optionally traverses them.   \
-   \
-   We only traverse the class instantiations when we see the canonical \
-   declaration of the template, to ensure we only visit them once. */  \
-if (getDerived().shouldVisitTemplateInstantiations() &&\
-D == D->getCanonicalDecl())\
-  TRY_TO(TraverseTemplateInstantiations(D));   \
-   \
-/* Note that getInstantiatedFromMemberTemplate() is just a link\
-   from a template instantiation back to the template from which   \
-   it was instantiated, and thus should not be traversed. */   \
+#define DEF_TRAVERSE_TMPL_DECL(TMPLDECLKIND) \
+  DEF_TRAVERSE_DECL(TMPLDECLKIND##TemplateDecl, {\
+TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters())); \
+TRY_TO(TraverseDecl(detail::getTemplatedDecl(\
+D,   \
+!getDerived().shouldVisitTemplateInstantiations() && \
+getDerived().shouldVisitSingleTemplateInstantiationInstead(; \
+ \
+/* By default, we do not traverse the instantiations of  

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

In D93298#3332038 , @xbolva00 wrote:

>>> It appears that this is causing an assertion segfault in a rustc test over 
>>> at our experimental rust + llvm@head bot:
>
> I dont think that patch author is required to debug this issue for 
> "experimental rust + llvm@head" - downstream.
>
>>> Since I don't have a rust environment
>
> There is no requirement, indeed. LLVM devs are not responsible to keep 
> ""experimental rust + llvm@head" working.

They're required to assist with fixing regressions that show when compiling 
legitimate IR. An IR reproducer was given, so no Rust toolchain knowledge 
should be needed, at that point it's the LLVM dev's responsibility, and if they 
can't fix it in a timely manner then it should be backed out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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

[PATCH] D93298: [RISCV] add the MC layer support of Zfinx extension

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In D93298#3332058 , @jrtc27 wrote:

> In D93298#3332038 , @xbolva00 wrote:
>
 It appears that this is causing an assertion segfault in a rustc test over 
 at our experimental rust + llvm@head bot:
>>
>> I dont think that patch author is required to debug this issue for 
>> "experimental rust + llvm@head" - downstream.
>>
 Since I don't have a rust environment
>>
>> There is no requirement, indeed. LLVM devs are not responsible to keep 
>> ""experimental rust + llvm@head" working.
>
> They're required to assist with fixing regressions that show when compiling 
> legitimate IR. An IR reproducer was given, so no Rust toolchain knowledge 
> should be needed, at that point it's the LLVM dev's responsibility, and if 
> they can't fix it in a timely manner then it should be backed out.

Then yeah, if IR is provided and is standard one, this is okay. Consider to add 
that IR as testcase for the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93298

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

[PATCH] D120132: [HIP] Fix HIP include path

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added subscribers: kerbowa, jvesely.
yaxunl requested review of this revision.

The clang compiler prepends the HIP header include paths to the search
list using -internal-isystem when building for the HIP language. This
prevents warnings related to things like reserved identifiers when
including the HIP headers even when ROCm is installed in a non-system
directory, such as /opt/rocm.

However, when HIP is installed in /usr, then the prepended include
path would be /usr/include. That is a problem, because the C standard
library headers are stored in /usr/include and the C++ standard
library headers must come before the C library headers in the search
path list (because the C++ standard library headers use #include_next
to include the C standard library headers).

While the HIP wrapper headers _do_ need to be earlier in the search
than the C++ headers, those headers get their own subdirectory and
their own explicit -internal-isystem argument. This include path is for
 and , which do not require a
particular search ordering with respect to the C or C++ headers. Thus,
when we know the path will be included anyway, we can skip adding it
to the list.

Patch by Cordell Bloor.


https://reviews.llvm.org/D120132

Files:
  clang/lib/Driver/ToolChains/AMDGPU.cpp
  clang/lib/Driver/ToolChains/ROCm.h


Index: clang/lib/Driver/ToolChains/ROCm.h
===
--- clang/lib/Driver/ToolChains/ROCm.h
+++ clang/lib/Driver/ToolChains/ROCm.h
@@ -154,6 +154,8 @@
   llvm::SmallString<0> findSPACKPackage(const Candidate &Cand,
 StringRef PackageName);
 
+  static bool isStdlibIncludePath(StringRef PathString);
+
 public:
   RocmInstallationDetector(const Driver &D, const llvm::Triple &HostTriple,
const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -483,6 +483,14 @@
<< DetectedVersion << '\n';
 }
 
+/*static*/ bool
+RocmInstallationDetector::isStdlibIncludePath(StringRef PathString) {
+  static const StringRef SystemDirs[] = {"/usr/local/include", "/usr/include"};
+
+  return std::find(std::begin(SystemDirs), std::end(SystemDirs), PathString) !=
+ std::end(SystemDirs);
+}
+
 void RocmInstallationDetector::AddHIPIncludeArgs(const ArgList &DriverArgs,
  ArgStringList &CC1Args) const 
{
   bool UsesRuntimeWrapper = VersionMajorMinor > llvm::VersionTuple(3, 5) &&
@@ -517,8 +525,12 @@
 return;
   }
 
-  CC1Args.push_back("-internal-isystem");
-  CC1Args.push_back(DriverArgs.MakeArgString(getIncludePath()));
+  const char *HipIncludePath = DriverArgs.MakeArgString(getIncludePath());
+  if (!isStdlibIncludePath(HipIncludePath) ||
+  DriverArgs.hasArg(options::OPT_nostdlibinc)) {
+CC1Args.push_back("-internal-isystem");
+CC1Args.push_back(HipIncludePath);
+  }
   if (UsesRuntimeWrapper)
 CC1Args.append({"-include", "__clang_hip_runtime_wrapper.h"});
 }


Index: clang/lib/Driver/ToolChains/ROCm.h
===
--- clang/lib/Driver/ToolChains/ROCm.h
+++ clang/lib/Driver/ToolChains/ROCm.h
@@ -154,6 +154,8 @@
   llvm::SmallString<0> findSPACKPackage(const Candidate &Cand,
 StringRef PackageName);
 
+  static bool isStdlibIncludePath(StringRef PathString);
+
 public:
   RocmInstallationDetector(const Driver &D, const llvm::Triple &HostTriple,
const llvm::opt::ArgList &Args,
Index: clang/lib/Driver/ToolChains/AMDGPU.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPU.cpp
+++ clang/lib/Driver/ToolChains/AMDGPU.cpp
@@ -483,6 +483,14 @@
<< DetectedVersion << '\n';
 }
 
+/*static*/ bool
+RocmInstallationDetector::isStdlibIncludePath(StringRef PathString) {
+  static const StringRef SystemDirs[] = {"/usr/local/include", "/usr/include"};
+
+  return std::find(std::begin(SystemDirs), std::end(SystemDirs), PathString) !=
+ std::end(SystemDirs);
+}
+
 void RocmInstallationDetector::AddHIPIncludeArgs(const ArgList &DriverArgs,
  ArgStringList &CC1Args) const {
   bool UsesRuntimeWrapper = VersionMajorMinor > llvm::VersionTuple(3, 5) &&
@@ -517,8 +525,12 @@
 return;
   }
 
-  CC1Args.push_back("-internal-isystem");
-  CC1Args.push_back(DriverArgs.MakeArgString(getIncludePath()));
+  const char *HipIncludePath = DriverArgs.MakeArgString(getIncludePath());
+  if (!isStdlibIncludePath(HipIncludePath) ||
+  DriverArgs.hasArg(options::OPT_nostdlibinc)) {
+CC1Args.push_back("-internal-isystem");
+CC1Args.push_back(HipIncludePath);
+  }
   if (UsesRuntimeWra

[PATCH] D116261: [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian 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 rGccebf8ac8c61: [Clang][OpenMP] Add support for compare 
capture in parser (authored by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116261

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_ast_print.cpp
  clang/test/OpenMP/atomic_messages.cpp

Index: clang/test/OpenMP/atomic_messages.cpp
===
--- clang/test/OpenMP/atomic_messages.cpp
+++ clang/test/OpenMP/atomic_messages.cpp
@@ -954,6 +954,10 @@
 // expected-note@+1 {{'read' clause used here}}
 #pragma omp atomic read compare
   a = b;
+// expected-error@+2 {{directive '#pragma omp atomic' cannot contain more than one 'compare' clause}}
+// expected-error@+1 {{directive '#pragma omp atomic' cannot contain more than one 'capture' clause}}
+#pragma omp atomic compare compare capture capture
+  a = b;
 #endif
   // expected-note@+1 {{in instantiation of function template specialization 'mixed' requested here}}
   return mixed();
Index: clang/test/OpenMP/atomic_ast_print.cpp
===
--- clang/test/OpenMP/atomic_ast_print.cpp
+++ clang/test/OpenMP/atomic_ast_print.cpp
@@ -20,6 +20,7 @@
 
 template 
 T foo(T argc) {
+  T v = T();
   T c = T();
   T b = T();
   T a = T();
@@ -45,6 +46,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic seq_cst
   a++;
@@ -68,6 +75,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare seq_cst
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture seq_cst
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare seq_cst capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture seq_cst
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic
   a++;
@@ -91,6 +104,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare acq_rel
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture acq_rel
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare acq_rel capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture acq_rel
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic
   a++;
@@ -114,6 +133,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare acquire
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture acquire
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare acquire capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture acquire
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic release
   a++;
@@ -137,6 +162,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare release
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture release
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare release capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture release
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic relaxed
   a++;
@@ -160,6 +191,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare relaxed
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture relaxed
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare relaxed capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture relaxed
+  { v = a == b; if (v) a = c; }
 #endif
 #pragma omp atomic hint(6)
   a++;
@@ -183,6 +220,12 @@
   { a = a < b ? b : a; }
 #pragma omp atomic compare hint(6)
   { a = a == b ? c : a; }
+#pragma omp atomic compare capture hint(6)
+  { v = a; a = a > b ? b : a; }
+#pragma omp atomic compare hint(6) capture
+  { v = a; a = a < b ? b : a; }
+#pragma omp atomic compare capture hint(6)
+  { v = a == b; if (v) a = c; }
 #endif
   return T();
 }
@@ -215,6 +258,22 @@
 // CHECK-51-NEXT: {
 // CHECK-51-NEXT: a = a == b ? c : a;
 // CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a > b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a;
+// CHECK-51-NEXT: a = a < b ? b : a;
+// CHECK-51-NEXT: }
+// CHECK-51-NEXT: #pragma omp atomic compare capture
+// CHECK-51-NEXT: {
+// CHECK-51-NEXT: v = a == b;
+// CHECK-51-NEXT: if (v)
+// CHECK-51-NEXT: a = c;
+// CHECK-51-NEXT: }
 // CHECK-NEXT: #pragma omp atomic seq_cst
 // CHECK-NEXT: a++;
 // CHECK-NEXT: #pragma omp atomic read seq_cst
@@ -242,6 +301,22 @@
 // CHECK-51-NEXT: {
 // CHECK-51-NEXT: a = a == b ? c : a;
 // CHECK-51-NEXT:

[clang] ccebf8a - [Clang][OpenMP] Add support for compare capture in parser

[Shilei Tian via cfe-commits]

Author: Shilei Tian
Date: 2022-02-18T10:23:59-05:00
New Revision: ccebf8ac8c61cbd46223abbeb4f29f4e1f7b490c

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

LOG: [Clang][OpenMP] Add support for compare capture in parser

This patch adds the support for `atomic compare capture` in parser and part of
sema. We don't create an AST node for this because the spec doesn't say 
`compare`
and `capture` clauses should be used tightly, so we cannot look one more token
ahead in the parser.

Reviewed By: ABataev

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

Added: 


Modified: 
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/atomic_ast_print.cpp
clang/test/OpenMP/atomic_messages.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGStmtOpenMP.cpp 
b/clang/lib/CodeGen/CGStmtOpenMP.cpp
index b491642871ce..8ea4968f4b11 100644
--- a/clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ b/clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -24,6 +24,7 @@
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/OpenMPKinds.h"
 #include "clang/Basic/PrettyStackTrace.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/BinaryFormat/Dwarf.h"
 #include "llvm/Frontend/OpenMP/OMPConstants.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
@@ -6020,7 +6021,7 @@ static void emitOMPAtomicExpr(CodeGenFunction &CGF, 
OpenMPClauseKind Kind,
   llvm::AtomicOrdering AO, bool IsPostfixUpdate,
   const Expr *X, const Expr *V, const Expr *E,
   const Expr *UE, bool IsXLHSInRHSPart,
-  SourceLocation Loc) {
+  bool IsCompareCapture, SourceLocation Loc) {
   switch (Kind) {
   case OMPC_read:
 emitOMPAtomicReadExpr(CGF, AO, X, V, Loc);
@@ -6037,10 +6038,19 @@ static void emitOMPAtomicExpr(CodeGenFunction &CGF, 
OpenMPClauseKind Kind,
  IsXLHSInRHSPart, Loc);
 break;
   case OMPC_compare: {
-// Emit an error here.
-unsigned DiagID = CGF.CGM.getDiags().getCustomDiagID(
-DiagnosticsEngine::Error, "'atomic compare' is not supported for now");
-CGF.CGM.getDiags().Report(DiagID);
+if (IsCompareCapture) {
+  // Emit an error here.
+  unsigned DiagID = CGF.CGM.getDiags().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "'atomic compare capture' is not supported for now");
+  CGF.CGM.getDiags().Report(DiagID);
+} else {
+  // Emit an error here.
+  unsigned DiagID = CGF.CGM.getDiags().getCustomDiagID(
+  DiagnosticsEngine::Error,
+  "'atomic compare' is not supported for now");
+  CGF.CGM.getDiags().Report(DiagID);
+}
 break;
   }
   case OMPC_if:
@@ -6153,18 +6163,23 @@ void CodeGenFunction::EmitOMPAtomicDirective(const 
OMPAtomicDirective &S) {
 AO = llvm::AtomicOrdering::Monotonic;
 MemOrderingSpecified = true;
   }
+  llvm::SmallSet KindsEncountered;
   OpenMPClauseKind Kind = OMPC_unknown;
   for (const OMPClause *C : S.clauses()) {
 // Find first clause (skip seq_cst|acq_rel|aqcuire|release|relaxed clause,
 // if it is first).
-if (C->getClauseKind() != OMPC_seq_cst &&
-C->getClauseKind() != OMPC_acq_rel &&
-C->getClauseKind() != OMPC_acquire &&
-C->getClauseKind() != OMPC_release &&
-C->getClauseKind() != OMPC_relaxed && C->getClauseKind() != OMPC_hint) 
{
-  Kind = C->getClauseKind();
-  break;
-}
+OpenMPClauseKind K = C->getClauseKind();
+if (K == OMPC_seq_cst || K == OMPC_acq_rel || K == OMPC_acquire ||
+K == OMPC_release || K == OMPC_relaxed || K == OMPC_hint)
+  continue;
+Kind = K;
+KindsEncountered.insert(K);
+  }
+  bool IsCompareCapture = false;
+  if (KindsEncountered.contains(OMPC_compare) &&
+  KindsEncountered.contains(OMPC_capture)) {
+IsCompareCapture = true;
+Kind = OMPC_compare;
   }
   if (!MemOrderingSpecified) {
 llvm::AtomicOrdering DefaultOrder =
@@ -6188,7 +6203,7 @@ void CodeGenFunction::EmitOMPAtomicDirective(const 
OMPAtomicDirective &S) {
   EmitStopPoint(S.getAssociatedStmt());
   emitOMPAtomicExpr(*this, Kind, AO, S.isPostfixUpdate(), S.getX(), S.getV(),
 S.getExpr(), S.getUpdateExpr(), S.isXLHSInRHSPart(),
-S.getBeginLoc());
+IsCompareCapture, S.getBeginLoc());
 }
 
 static void emitCommonOMPTargetDirective(CodeGenFunction &CGF,

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 64647f59fcb5..686cef249f3a 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -35,6 +35,7 @@
 #include "llvm/ADT/IndexedMap.h"
 #include "llvm/ADT/PointerEmbeddedInt.h"
 #include "llvm/ADT/STLEx

[clang] 68b7b35 - [Clang][OpenMP][Sema] Remove support for floating point values in atomic compare

[Shilei Tian via cfe-commits]

Author: Shilei Tian
Date: 2022-02-18T10:24:29-05:00
New Revision: 68b7b357fdfc00ed8807a887eba363d67c9dc3f5

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

LOG: [Clang][OpenMP][Sema] Remove support for floating point values in atomic 
compare

This is a follow-up patch of D119378.

Reviewed By: ABataev

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

Added: 


Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/atomic_messages.c

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 686cef249f3a..ec0d095e8995 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -11015,8 +11015,6 @@ class OpenMPAtomicCompareChecker {
   Expr *C = nullptr;
   /// True if the cond expr is in the form of 'x ordop expr'.
   bool IsXBinopExpr = true;
-  /// The atomic compare operator.
-  OMPAtomicCompareOp Op;
 
   /// Check if it is a valid conditional update statement (cond-update-stmt).
   bool checkCondUpdateStmt(IfStmt *S, ErrorInfoTy &ErrorInfo);
@@ -11073,23 +11071,7 @@ bool 
OpenMPAtomicCompareChecker::checkCondUpdateStmt(IfStmt *S,
   }
 
   switch (Cond->getOpcode()) {
-  case BO_EQ:
-Op = OMPAtomicCompareOp::EQ;
-break;
-  case BO_LT:
-Op = OMPAtomicCompareOp::MIN;
-break;
-  case BO_GT:
-Op = OMPAtomicCompareOp::MAX;
-break;
-  default:
-ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
-return false;
-  }
-
-  if (Cond->getOpcode() == BO_EQ) {
+  case BO_EQ: {
 C = Cond;
 D = BO->getRHS();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
@@ -11102,7 +11084,10 @@ bool 
OpenMPAtomicCompareChecker::checkCondUpdateStmt(IfStmt *S,
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
-  } else {
+break;
+  }
+  case BO_LT:
+  case BO_GT: {
 E = BO->getRHS();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS()) &&
 checkIfTwoExprsAreSame(ContextRef, E, Cond->getRHS())) {
@@ -7,6 +11102,13 @@ bool 
OpenMPAtomicCompareChecker::checkCondUpdateStmt(IfStmt *S,
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
+break;
+  }
+  default:
+ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
+return false;
   }
 
   return true;
@@ -11167,23 +11159,7 @@ bool 
OpenMPAtomicCompareChecker::checkCondExprStmt(Stmt *S,
   }
 
   switch (Cond->getOpcode()) {
-  case BO_EQ:
-Op = OMPAtomicCompareOp::EQ;
-break;
-  case BO_LT:
-Op = OMPAtomicCompareOp::MIN;
-break;
-  case BO_GT:
-Op = OMPAtomicCompareOp::MAX;
-break;
-  default:
-ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
-return false;
-  }
-
-  if (Cond->getOpcode() == BO_EQ) {
+  case BO_EQ: {
 C = Cond;
 D = CO->getTrueExpr();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
@@ -11196,7 +11172,10 @@ bool 
OpenMPAtomicCompareChecker::checkCondExprStmt(Stmt *S,
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
-  } else {
+break;
+  }
+  case BO_LT:
+  case BO_GT: {
 E = CO->getTrueExpr();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS()) &&
 checkIfTwoExprsAreSame(ContextRef, E, Cond->getRHS())) {
@@ -11211,6 +11190,13 @@ bool 
OpenMPAtomicCompareChecker::checkCondExprStmt(Stmt *S,
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
+break;
+  }
+  default:
+ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
+return false;
   }
 
   return true;
@@ -11220,8 +11206,7 @@ bool OpenMPAtomicCompareChecker::checkType(ErrorInfoTy 
&ErrorInfo) const {
   // 'x' and 'e' cannot be nullptr
   assert(X && E && "X and E cannot be nullptr");
 
-  auto CheckValue = [&ErrorInfo](const Expr *E, OMPAtomicCompareOp Op,
- bool ShouldBeLValue) {
+  auto CheckValue = [&ErrorInfo](const Expr *E, bool ShouldBeLValue) {
 if (ShouldBeLValue && !E->isLValue()) {
   ErrorInfo.Error = ErrorTy::XNotLValue;
   ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E->getExprLoc();
@@ -11238,7 +11223,7 @@ bool OpenMPAtomicCompareChecker::che

[PATCH] D119392: [Clang][OpenMP][Sema] Remove support for floating point values in atomic compare

[Shilei Tian via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG68b7b357fdfc: [Clang][OpenMP][Sema] Remove support for 
floating point values in atomic compare (authored by tianshilei1992).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119392

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/atomic_messages.c

Index: clang/test/OpenMP/atomic_messages.c
===
--- clang/test/OpenMP/atomic_messages.c
+++ clang/test/OpenMP/atomic_messages.c
@@ -483,5 +483,12 @@
 if (fx > fe)
   fx = fe;
   }
+// omp51-error@+5 {{the statement for 'atomic compare' must be a compound statement of form '{x = expr ordop x ? expr : x;}', '{x = x ordop expr? expr : x;}', '{x = x == e ? d : x;}', '{x = e == x ? d : x;}', or 'if(expr ordop x) {x = expr;}', 'if(x ordop expr) {x = expr;}', 'if(x == e) {x = d;}', 'if(e == x) {x = d;}' where 'x' is an lvalue expression with scalar type, 'expr', 'e', and 'd' are expressions with scalar type, and 'ordop' is one of '<' or '>'.}}
+// omp51-note@+4 {{expect integer value}}
+#pragma omp atomic compare
+  {
+if (fx == fe)
+  fx = fe;
+  }
 }
 #endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -11015,8 +11015,6 @@
   Expr *C = nullptr;
   /// True if the cond expr is in the form of 'x ordop expr'.
   bool IsXBinopExpr = true;
-  /// The atomic compare operator.
-  OMPAtomicCompareOp Op;
 
   /// Check if it is a valid conditional update statement (cond-update-stmt).
   bool checkCondUpdateStmt(IfStmt *S, ErrorInfoTy &ErrorInfo);
@@ -11073,23 +11071,7 @@
   }
 
   switch (Cond->getOpcode()) {
-  case BO_EQ:
-Op = OMPAtomicCompareOp::EQ;
-break;
-  case BO_LT:
-Op = OMPAtomicCompareOp::MIN;
-break;
-  case BO_GT:
-Op = OMPAtomicCompareOp::MAX;
-break;
-  default:
-ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
-return false;
-  }
-
-  if (Cond->getOpcode() == BO_EQ) {
+  case BO_EQ: {
 C = Cond;
 D = BO->getRHS();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
@@ -11102,7 +11084,10 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
-  } else {
+break;
+  }
+  case BO_LT:
+  case BO_GT: {
 E = BO->getRHS();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS()) &&
 checkIfTwoExprsAreSame(ContextRef, E, Cond->getRHS())) {
@@ -7,6 +11102,13 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
+break;
+  }
+  default:
+ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
+return false;
   }
 
   return true;
@@ -11167,23 +11159,7 @@
   }
 
   switch (Cond->getOpcode()) {
-  case BO_EQ:
-Op = OMPAtomicCompareOp::EQ;
-break;
-  case BO_LT:
-Op = OMPAtomicCompareOp::MIN;
-break;
-  case BO_GT:
-Op = OMPAtomicCompareOp::MAX;
-break;
-  default:
-ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
-ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
-ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
-return false;
-  }
-
-  if (Cond->getOpcode() == BO_EQ) {
+  case BO_EQ: {
 C = Cond;
 D = CO->getTrueExpr();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS())) {
@@ -11196,7 +11172,10 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
-  } else {
+break;
+  }
+  case BO_LT:
+  case BO_GT: {
 E = CO->getTrueExpr();
 if (checkIfTwoExprsAreSame(ContextRef, X, Cond->getLHS()) &&
 checkIfTwoExprsAreSame(ContextRef, E, Cond->getRHS())) {
@@ -11211,6 +11190,13 @@
   ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
   return false;
 }
+break;
+  }
+  default:
+ErrorInfo.Error = ErrorTy::InvalidBinaryOp;
+ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = Cond->getExprLoc();
+ErrorInfo.ErrorRange = ErrorInfo.NoteRange = Cond->getSourceRange();
+return false;
   }
 
   return true;
@@ -11220,8 +11206,7 @@
   // 'x' and 'e' cannot be nullptr
   assert(X && E && "X and E cannot be nullptr");
 
-  auto CheckValue = [&ErrorInfo](const Expr *E, OMPAtomicCompareOp Op,
- bool ShouldBeLValue) {
+  auto CheckValue = [&ErrorInfo](const Expr *E, bool ShouldBeLValue) {
 if (ShouldBeLValue && !E->isLValue()) {
   ErrorInfo.Error = ErrorTy::XNotLValue;
   ErrorInfo.ErrorLoc = ErrorInfo.NoteLoc = E-

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry updated this revision to Diff 409935.
kuzkry added a comment.

Review fix: added note about IndentRequires. The generated HTML looks like this:
F22153448: IndentRequires note.png 


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

https://reviews.llvm.org/D119682

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1791,7 +1791,7 @@
   };
 
   /// The concept declaration style to use.
-  /// \version 13
+  /// \version 12
   BreakBeforeConceptDeclarationsStyle BreakBeforeConceptDeclarations;
 
   /// If ``true``, ternary operators will be placed after line breaks.
@@ -2185,7 +2185,7 @@
   };
 
   /// Defines in which cases to put empty line before access modifiers.
-  /// \version 13
+  /// \version 12
   EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
 
   /// If ``true``, clang-format detects whether function calls and
@@ -2523,6 +2523,8 @@
 
   /// Indent the requires clause in a template. This only applies when
   /// ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
+  ///
+  /// In clang-format 13 and 14 it was named ``IndentRequires``.
   /// \code
   ///true:
   ///template 
@@ -2538,7 +2540,7 @@
   ///  //
   ///}
   /// \endcode
-  /// \version 13
+  /// \version 15
   bool IndentRequiresClause;
 
   /// The number of columns to use for indentation.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1988,7 +1988,7 @@
 
 
 
-**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) 
:versionbadge:`clang-format 13`
+**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) 
:versionbadge:`clang-format 12`
   The concept declaration style to use.
 
   Possible values:
@@ -2278,7 +2278,7 @@
 
 
 
-**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) 
:versionbadge:`clang-format 13`
+**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) 
:versionbadge:`clang-format 12`
   Defines in which cases to put empty line before access modifiers.
 
   Possible values:
@@ -2706,10 +2706,12 @@
 
 
 
-**IndentRequiresClause** (``Boolean``) :versionbadge:`clang-format 13`
+**IndentRequiresClause** (``Boolean``) :versionbadge:`clang-format 15`
   Indent the requires clause in a template. This only applies when
   ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
 
+  In clang-format 13 and 14 it was named ``IndentRequires``.
+
   .. code-block:: c++
 
  true:


Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1791,7 +1791,7 @@
   };
 
   /// The concept declaration style to use.
-  /// \version 13
+  /// \version 12
   BreakBeforeConceptDeclarationsStyle BreakBeforeConceptDeclarations;
 
   /// If ``true``, ternary operators will be placed after line breaks.
@@ -2185,7 +2185,7 @@
   };
 
   /// Defines in which cases to put empty line before access modifiers.
-  /// \version 13
+  /// \version 12
   EmptyLineBeforeAccessModifierStyle EmptyLineBeforeAccessModifier;
 
   /// If ``true``, clang-format detects whether function calls and
@@ -2523,6 +2523,8 @@
 
   /// Indent the requires clause in a template. This only applies when
   /// ``RequiresClausePosition`` is ``OwnLine``, or ``WithFollowing``.
+  ///
+  /// In clang-format 13 and 14 it was named ``IndentRequires``.
   /// \code
   ///true:
   ///template 
@@ -2538,7 +2540,7 @@
   ///  //
   ///}
   /// \endcode
-  /// \version 13
+  /// \version 15
   bool IndentRequiresClause;
 
   /// The number of columns to use for indentation.
Index: clang/docs/ClangFormatStyleOptions.rst
===
--- clang/docs/ClangFormatStyleOptions.rst
+++ clang/docs/ClangFormatStyleOptions.rst
@@ -1988,7 +1988,7 @@
 
 
 
-**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) :versionbadge:`clang-format 13`
+**BreakBeforeConceptDeclarations** (``BreakBeforeConceptDeclarationsStyle``) :versionbadge:`clang-format 12`
   The concept declaration style to use.
 
   Possible values:
@@ -2278,7 +2278,7 @@
 
 
 
-**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) :versionbadge:`clang-format 13`
+**EmptyLineBeforeAccessModifier** (``EmptyLineBeforeAccessModifierStyle``) :versionbadge:`clang-format 12`
   Defines in which cases to put empty line before access modifiers.
 
   Possible values:
@@ -2706,10 +2706,12 @@
 
 
 
-**IndentRequiresClaus

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

[Krystian Kuzniarek via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Format/Format.h:2540-2541
   ///}
   /// \endcode
-  /// \version 13
+  /// \version 15
   bool IndentRequiresClause;

MyDeveloperDay wrote:
> HazardyKnusperkeks wrote:
> > 
> I like that suggestion..
Sorry, I didn't notice this inline comment before.

@HazardyKnusperkeks, I put it in a slightly different place, i.e. above the 
example code as Sphinx complained about this particular placement.


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

https://reviews.llvm.org/D119682

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

[clang] 91cf639 - Fix Wdocumentation unknown parameter warning

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-18T15:38:16Z
New Revision: 91cf639ac069a797b1fac4134cf121bc9db6dff6

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

LOG: Fix Wdocumentation unknown parameter warning

Added: 


Modified: 
clang/include/clang/Driver/Driver.h

Removed: 




diff  --git a/clang/include/clang/Driver/Driver.h 
b/clang/include/clang/Driver/Driver.h
index 93e1eca6a981..6f24f649ea54 100644
--- a/clang/include/clang/Driver/Driver.h
+++ b/clang/include/clang/Driver/Driver.h
@@ -614,9 +614,9 @@ class Driver {
   ///
   /// \param[in] HostTC is the host ToolChain paired with the device
   ///
-  /// \param[in] Action (e.g. OFK_Cuda/OFK_OpenMP/OFK_SYCL) is an Offloading
-  /// action that is optionally passed to a ToolChain (used by CUDA, to specify
-  /// if it's used in conjunction with OpenMP)
+  /// \param[in] TargetDeviceOffloadKind (e.g. OFK_Cuda/OFK_OpenMP/OFK_SYCL) is
+  /// an Offloading action that is optionally passed to a ToolChain (used by
+  /// CUDA, to specify if it's used in conjunction with OpenMP)
   ///
   /// Will cache ToolChains for the life of the driver object, and create them
   /// on-demand.



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

[PATCH] D120115: [clangd] Tweak --query-driver to ignore slash direction on windows

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks, lgtm!




Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:278
+} else if (llvm::sys::path::is_separator(Glob[I]) &&
+   llvm::sys::path::is_separator('/') &&
+   llvm::sys::path::is_separator('\\')) {

nit: maybe extract `llvm::sys::path::is_separator('/') && 
llvm::sys::path::is_separator('\\')` into a `bool PlatformIsSlashInsensitive` ?



Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:298
 
+  // Tempting to pass IgnoreCase, but we don't the FS sensitivity precisely.
   llvm::Regex Reg(llvm::join(RegTexts, "|"));

we don't `know` the FS .. ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120115

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

[PATCH] D119144: [tests][Driver] Pass an empty sysroot for `DEFAULT_SYSROOT` builds

[Carlo Cabrera via Phabricator via cfe-commits]

carlocab added a comment.

Thanks Petr. I don’t have commit access, so I’d appreciate it if you could 
commit this for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119144

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

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

[Vince Bridgers via Phabricator via cfe-commits]

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

This is a NFC refactoring to change makeIntValWithPtrWidth
and remove getZeroWithPtrWidth to use types when forming values to match
pointer widths. Some targets may have different pointer widths depending
upon address space, so this needs to be comprehended.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120134

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


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


Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
===
--- clang/lib/StaticA

[PATCH] D119221: [clang][lexer] Allow u8 character literal prefixes in C2x

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added inline comments.



Comment at: clang/lib/Sema/SemaExpr.cpp:3565
   else
 Ty = Context.CharTy;  // 'x' -> char in C++
 

Perhaps worth updating this comment? e.g., 
  // 'x' -> char in C++; u8'x' -> char in C11-C17 and in C++ without char8_t.



Comment at: clang/test/Lexer/utf8-char-literal.cpp:19
+char b = u8'\x80';   // ok
+char c = u8'\u0080'; // expected-error {{universal character name refers to a 
control character}}
+char d = u8'\u1234'; // expected-error {{character too large for enclosing 
character literal type}}

This is an interesting difference between C and C++ that I was not aware of. 
Looks right though!


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

https://reviews.llvm.org/D119221

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

[PATCH] D120115: [clangd] Tweak --query-driver to ignore slash direction on windows

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:278
+} else if (llvm::sys::path::is_separator(Glob[I]) &&
+   llvm::sys::path::is_separator('/') &&
+   llvm::sys::path::is_separator('\\')) {

kadircet wrote:
> nit: maybe extract `llvm::sys::path::is_separator('/') && 
> llvm::sys::path::is_separator('\\')` into a `bool PlatformIsSlashInsensitive` 
> ?
We require the separators to be exactly \ and / for the regex to be correct, so 
I'd rather keep the literals here.

(Call me paranoid, but I grew up on a system with `:` for separators...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120115

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

[PATCH] D120115: [clangd] Tweak --query-driver to ignore slash direction on windows

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG47b749e5be21: [clangd] Tweak --query-driver to ignore slash 
direction on windows (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D120115?vs=409900&id=409957#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120115

Files:
  clang-tools-extra/clangd/QueryDriverDatabase.cpp


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -38,12 +38,10 @@
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Types.h"
 #include "clang/Tooling/CompilationDatabase.h"
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -276,6 +274,10 @@
 // Single star, accept any sequence without a slash.
 RegStream << "[^/]*";
   }
+} else if (llvm::sys::path::is_separator(Glob[I]) &&
+   llvm::sys::path::is_separator('/') &&
+   llvm::sys::path::is_separator('\\')) {
+  RegStream << R"([/\\])"; // Accept either slash on windows.
 } else {
   RegStream << llvm::Regex::escape(Glob.substr(I, 1));
 }
@@ -293,6 +295,7 @@
   for (llvm::StringRef Glob : Globs)
 RegTexts.push_back(convertGlobToRegex(Glob));
 
+  // Tempting to pass IgnoreCase, but we don't know the FS sensitivity.
   llvm::Regex Reg(llvm::join(RegTexts, "|"));
   assert(Reg.isValid(RegTexts.front()) &&
  "Created an invalid regex from globs");


Index: clang-tools-extra/clangd/QueryDriverDatabase.cpp
===
--- clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -38,12 +38,10 @@
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Types.h"
 #include "clang/Tooling/CompilationDatabase.h"
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -276,6 +274,10 @@
 // Single star, accept any sequence without a slash.
 RegStream << "[^/]*";
   }
+} else if (llvm::sys::path::is_separator(Glob[I]) &&
+   llvm::sys::path::is_separator('/') &&
+   llvm::sys::path::is_separator('\\')) {
+  RegStream << R"([/\\])"; // Accept either slash on windows.
 } else {
   RegStream << llvm::Regex::escape(Glob.substr(I, 1));
 }
@@ -293,6 +295,7 @@
   for (llvm::StringRef Glob : Globs)
 RegTexts.push_back(convertGlobToRegex(Glob));
 
+  // Tempting to pass IgnoreCase, but we don't know the FS sensitivity.
   llvm::Regex Reg(llvm::join(RegTexts, "|"));
   assert(Reg.isValid(RegTexts.front()) &&
  "Created an invalid regex from globs");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 47b749e - [clangd] Tweak --query-driver to ignore slash direction on windows

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-02-18T18:06:36+01:00
New Revision: 47b749e5be2190d1ccb214fd6364da462a9098cf

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

LOG: [clangd] Tweak --query-driver to ignore slash direction on windows

See https://github.com/clangd/clangd/issues/1022

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

Added: 


Modified: 
clang-tools-extra/clangd/QueryDriverDatabase.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/QueryDriverDatabase.cpp 
b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
index 5e51837b4820f..0daa0c5cd0be0 100644
--- a/clang-tools-extra/clangd/QueryDriverDatabase.cpp
+++ b/clang-tools-extra/clangd/QueryDriverDatabase.cpp
@@ -38,12 +38,10 @@
 #include "clang/Basic/TargetOptions.h"
 #include "clang/Driver/Types.h"
 #include "clang/Tooling/CompilationDatabase.h"
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/Path.h"
@@ -276,6 +274,10 @@ std::string convertGlobToRegex(llvm::StringRef Glob) {
 // Single star, accept any sequence without a slash.
 RegStream << "[^/]*";
   }
+} else if (llvm::sys::path::is_separator(Glob[I]) &&
+   llvm::sys::path::is_separator('/') &&
+   llvm::sys::path::is_separator('\\')) {
+  RegStream << R"([/\\])"; // Accept either slash on windows.
 } else {
   RegStream << llvm::Regex::escape(Glob.substr(I, 1));
 }
@@ -293,6 +295,7 @@ llvm::Regex convertGlobsToRegex(llvm::ArrayRef 
Globs) {
   for (llvm::StringRef Glob : Globs)
 RegTexts.push_back(convertGlobToRegex(Glob));
 
+  // Tempting to pass IgnoreCase, but we don't know the FS sensitivity.
   llvm::Regex Reg(llvm::join(RegTexts, "|"));
   assert(Reg.isValid(RegTexts.front()) &&
  "Created an invalid regex from globs");



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

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

(maybe relevant: For what it's worth: I originally implemented inline function 
homing in modules codegen for Clang Header Modules - the results I got for 
object file size in an -O0 build were marginal - a /slight/ win in object file 
size, but not as much as we might've expected. Part of the reason might be that 
there can be inline functions that are never called, or at higher optimization 
levels, inline functions that always get inlined (via "available externally" 
definitions) - in that case, providing "homed" definitions creates inline 
function definitions that are unused during linking/a waste of space. It's 
possible the workload I was dealing with (common Google internal programs) 
skewed compared to broader C++ code - for instance heavy use of protobufs could 
be leading to a lot of generated code/inline functions that are mostly unused. 
I didn't iterate further to tweak/implement heuristics about which inline 
functions should be homed. I'm not sure if Richard Smith made a choice about 
not homing inline functions in C++20 modules because of these results, or for 
other reasons, or just as a consequence of the implementation - but given we 
had the logic in Clang to do inline function homing for Clang Header Modules, 
I'm guessing it was an intentional choice to not use that functionality in 
C++20 modules when they have to have an object file anyway)


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

https://reviews.llvm.org/D119409

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

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

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

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

>> That's not the first time we renamed something. And most likely not the last 
>> time.
>
> But that doesn't mean we can't add "Previously known as IndentRequires", does 
> it? :)

Of course! I just wanted to say that we can and should create precedence here.




Comment at: clang/include/clang/Format/Format.h:2540-2541
   ///}
   /// \endcode
-  /// \version 13
+  /// \version 15
   bool IndentRequiresClause;

kuzkry wrote:
> MyDeveloperDay wrote:
> > HazardyKnusperkeks wrote:
> > > 
> > I like that suggestion..
> Sorry, I didn't notice this inline comment before.
> 
> @HazardyKnusperkeks, I put it in a slightly different place, i.e. above the 
> example code as Sphinx complained about this particular placement.
Fine by me. I have absolutely no idea about this sphinx or `.rst` stuff.


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

https://reviews.llvm.org/D119682

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

[PATCH] D120140: [clang-format] Avoid inserting space after C++ casts.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius created this revision.
curdeius added reviewers: MyDeveloperDay, HazardyKnusperkeks, owenpan.
curdeius requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

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

This is a solution for standard C++ casts: const_cast, dynamic_cast, 
reinterpret_cast, static_cast.

A general approach handling all possible casts is not possible without semantic 
information.
Consider the code:

  static_cast(*function_pointer_variable)(arguments);

vs.

  some_return_type (*function_pointer_variable)(parameters);
  // Later used as:
  function_pointer_variable = &some_function;
  return function_pointer_variable(args);

In the latter case, it's not a cast but a variable declaration of a pointer to 
function.
Without knowing what `some_return_type` is (and clang-format does not know 
it), it's hard to distinguish between the two cases. Theoretically, one could 
check whether "parameters" are types (not a cast) and "arguments" are 
value/expressions (a cast), but that might be inefficient (needs lots of 
lookahead).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120140

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10565,6 +10565,13 @@
 
 TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("Type *A = static_cast(P);");
+  verifyFormat("static_cast(P);");
+  verifyFormat("static_cast(Fun)(Args);");
+  verifyFormat("static_cast(*Fun)(Args);");
+  verifyFormat("a = static_cast(*Fun)(Args);");
+  verifyFormat("const_cast(*Fun)(Args);");
+  verifyFormat("dynamic_cast(*Fun)(Args);");
+  verifyFormat("reinterpret_cast(*Fun)(Args);");
   verifyFormat("Type *A = (Type *)P;");
   verifyFormat("Type *A = (vector)P;");
   verifyFormat("int a = (int)(2.0f);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1734,8 +1734,11 @@
   else
 Current.setType(TT_LineComment);
 } else if (Current.is(tok::r_paren)) {
-  if (rParenEndsCast(Current))
+  if (rParenEndsCast(Current)) {
 Current.setType(TT_CastRParen);
+assert(Current.MatchingParen);
+Current.MatchingParen->setType(TT_Unknown);
+  }
   if (Current.MatchingParen && Current.Next &&
   !Current.Next->isBinaryOperator() &&
   !Current.Next->isOneOf(tok::semi, tok::colon, tok::l_brace,
@@ -1938,8 +1941,20 @@
 
   // Certain other tokens right before the parentheses are also signals 
that
   // this cannot be a cast.
+  if (LeftOfParens->is(TT_TemplateCloser)) {
+if (LeftOfParens->MatchingParen) {
+  auto *Prev = LeftOfParens->MatchingParen->getPreviousNonComment();
+  if (Prev &&
+  Prev->isOneOf(tok::kw_const_cast, tok::kw_dynamic_cast,
+tok::kw_reinterpret_cast, tok::kw_static_cast))
+// FIXME: Maybe we should handle identifiers ending with "_cast",
+// e.g. bit_cast?
+return true;
+}
+return false;
+  }
   if (LeftOfParens->isOneOf(tok::at, tok::r_square, TT_OverloadedOperator,
-TT_TemplateCloser, tok::ellipsis))
+tok::ellipsis))
 return false;
 }
 


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -10565,6 +10565,13 @@
 
 TEST_F(FormatTest, FormatsCasts) {
   verifyFormat("Type *A = static_cast(P);");
+  verifyFormat("static_cast(P);");
+  verifyFormat("static_cast(Fun)(Args);");
+  verifyFormat("static_cast(*Fun)(Args);");
+  verifyFormat("a = static_cast(*Fun)(Args);");
+  verifyFormat("const_cast(*Fun)(Args);");
+  verifyFormat("dynamic_cast(*Fun)(Args);");
+  verifyFormat("reinterpret_cast(*Fun)(Args);");
   verifyFormat("Type *A = (Type *)P;");
   verifyFormat("Type *A = (vector)P;");
   verifyFormat("int a = (int)(2.0f);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1734,8 +1734,11 @@
   else
 Current.setType(TT_LineComment);
 } else if (Current.is(tok::r_paren)) {
-  if (rParenEndsCast(Current))
+  if (rParenEndsCast(Current)) {
 Current.setType(TT_CastRParen);
+assert(Current.MatchingParen);
+Current.MatchingParen->setType(TT_Unknown);
+  }
   if (Current.MatchingParen && Current.Next &&
   !Current.Next->isBinaryOperator() &&
   

[PATCH] D120140: [clang-format] Avoid inserting space after C++ casts.

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1740
+assert(Current.MatchingParen);
+Current.MatchingParen->setType(TT_Unknown);
+  }

To add some context, in the failing cases, the opening parenthesis was set to 
type `TT_FunctionTypeLParen` that we don't want because combined with a `*` 
after the next paren, it adds a space (cf. 
https://github.com/llvm/llvm-project/blob/331e8e4e27be5dd673898a89a7cf00e76903216a/clang/lib/Format/TokenAnnotator.cpp#L3105-L3109).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120140

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

[clang] 74036db - Fix Wdocumentation unknown parameter warning

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-18T17:27:45Z
New Revision: 74036dbafd3d742e86464043f3e2f4d52bf79f1e

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

LOG: Fix Wdocumentation unknown parameter warning

Added: 


Modified: 
clang/lib/Sema/SemaTemplateDeduction.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplateDeduction.cpp 
b/clang/lib/Sema/SemaTemplateDeduction.cpp
index 22dd395d99439..a53d83ea700b6 100644
--- a/clang/lib/Sema/SemaTemplateDeduction.cpp
+++ b/clang/lib/Sema/SemaTemplateDeduction.cpp
@@ -533,9 +533,9 @@ DeduceTemplateArguments(Sema &S,
 ///
 /// \param TemplateParams the template parameters that we are deducing
 ///
-/// \param Param the parameter type
+/// \param P the parameter type
 ///
-/// \param Arg the argument type
+/// \param A the argument type
 ///
 /// \param Info information about the template argument deduction itself
 ///
@@ -1199,11 +1199,11 @@ static CXXRecordDecl *getCanonicalRD(QualType T) {
 ///
 /// \param S the semantic analysis object within which we are deducing.
 ///
-/// \param RecordT the top level record object we are deducing against.
+/// \param RD the top level record object we are deducing against.
 ///
 /// \param TemplateParams the template parameters that we are deducing.
 ///
-/// \param SpecParam the template specialization parameter type.
+/// \param P the template specialization parameter type.
 ///
 /// \param Info information about the template argument deduction itself.
 ///
@@ -1315,9 +1315,9 @@ DeduceTemplateBases(Sema &S, const CXXRecordDecl *RD,
 ///
 /// \param TemplateParams the template parameters that we are deducing
 ///
-/// \param ParamIn the parameter type
+/// \param P the parameter type
 ///
-/// \param ArgIn the argument type
+/// \param A the argument type
 ///
 /// \param Info information about the template argument deduction itself
 ///



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

[clang-tools-extra] 9415fbb - [clangd] getHover - pass FormatStyle argument by const reference

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-02-18T17:27:45Z
New Revision: 9415fbbbcf73ab90692fafdac5bd6e302d07ba4b

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

LOG: [clangd] getHover - pass FormatStyle argument by const reference

Reported by coverity

Added: 


Modified: 
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/Hover.h

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index dda5ad36e9b8..8b2201841140 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -963,7 +963,7 @@ void maybeAddCalleeArgInfo(const SelectionTree::Node *N, 
HoverInfo &HI,
 } // namespace
 
 llvm::Optional getHover(ParsedAST &AST, Position Pos,
-   format::FormatStyle Style,
+   const format::FormatStyle &Style,
const SymbolIndex *Index) {
   PrintingPolicy PP =
   getPrintingPolicy(AST.getASTContext().getPrintingPolicy());

diff  --git a/clang-tools-extra/clangd/Hover.h 
b/clang-tools-extra/clangd/Hover.h
index 7478ede88a46..1a46ff5ad772 100644
--- a/clang-tools-extra/clangd/Hover.h
+++ b/clang-tools-extra/clangd/Hover.h
@@ -137,7 +137,7 @@ inline bool operator==(const HoverInfo::Param &LHS,
 
 /// Get the hover information when hovering at \p Pos.
 llvm::Optional getHover(ParsedAST &AST, Position Pos,
-   format::FormatStyle Style,
+   const format::FormatStyle &Style,
const SymbolIndex *Index);
 
 } // namespace clangd



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

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Chris Lattner via Phabricator via cfe-commits]

lattner added a comment.

Hi all, I'm commenting on this based on my personal opinion, I don't speak for 
the LLVM board or anyone else.  I am also not a lawyer :)

This isn’t a clear cut case (as is typical!).  LLVM's approach on patents 
protection revolves primarily around the terms in the Apache 2 license, which 
is based on the owner of patents contributing code.  Beyond that, we 
can’t/don't proactively do all the research of potential patent infringement of 
contributed code.

That said, we have historically NOT taken code that is known to infringe on 
high risk patents.  The one that comes to mind is the (now expired) patents on 
Steensgaard’s unification-based pointer analysis work.  We rejected taking 
related work until those patents expire.

In this case, AUTOSAR/Parasoft looks like a little company, their spec is 
clearly saying they have IP rights associated with it, and the risks seem very 
high to me.  I think we should ask for a legal statement signed by a director 
of the company (not an informal email) saying we can use this in LLVM.  The 
risk is just otherwise too high for the vast community of people who use LLVM.

-Chris


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

https://reviews.llvm.org/D112730

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

[PATCH] D120111: [AArch64] Default HBC/MOPS features in clang

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:464-473
+  const char *v8691OrLater[] = {"+v8.6a", "+v8.7a", "+v8.8a",
+   "+v9.1a", "+v9.2a", "+v9.3a"};
   auto Pos = std::find_first_of(Features.begin(), Features.end(),
-std::begin(Archs), std::end(Archs));
+std::begin(v8691OrLater), 
std::end(v8691OrLater));
   if (Pos != std::end(Features))
 Pos = Features.insert(std::next(Pos), {"+i8mm", "+bf16"});
 

Is it possible to re-use some of the many calls to std::find (L395-403)? Seems 
like we re-scan the feature list A LOT.



Comment at: clang/lib/Driver/ToolChains/Arch/AArch64.cpp:466-474
   auto Pos = std::find_first_of(Features.begin(), Features.end(),
-std::begin(Archs), std::end(Archs));
+std::begin(v8691OrLater), 
std::end(v8691OrLater));
   if (Pos != std::end(Features))
 Pos = Features.insert(std::next(Pos), {"+i8mm", "+bf16"});
 
+  // For Armv8.8-a/Armv9.3-a or later, FEAT_HBC and FEAT_MOPS are enabled by
+  // default.

Can we reuse `ItBegin` and `ItEnd` here rather than `Features.begin()` and 
`Features.end()`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120111

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

[PATCH] D119682: [clang-format][docs] Fix incorrect 'clang-format 13' configuration options markers

[Krystian Kuzniarek via Phabricator via cfe-commits]

kuzkry added a comment.

Can I ask you to deliver this one for me? My name and email in git format is 
"Krystian Kuzniarek "


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

https://reviews.llvm.org/D119682

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

[PATCH] D112916: Confusable identifiers detection

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D112916#3331592 , @cor3ntin wrote:

> In D112916#3314184 , @cor3ntin 
> wrote:
>
>> @aaron.ballman Thanks for the ping.
>>
>> One one hand, I agree with you, on the other hand, this tries to stick to 
>> TR39, and I think we should stick with that. It might be worth checking with 
>> the Unicode consortium what they think of i/l as confusable.
>
> Actually @aaron.ballman, Unicode does consider these confusables already
>
> from https://www.unicode.org/Public/security/14.0.0/confusables.txt
>
>   0031 ;  006C ;  MA  # ( 1 → l ) DIGIT ONE → LATIN SMALL LETTER L
> # 
>   0049 ;  006C ;  MA  # ( I → l ) LATIN CAPITAL LETTER I → LATIN 
> SMALL LETTER L   # 
>   0030 ;  004F ;  MA  # ( 0 → O ) DIGIT ZERO → LATIN CAPITAL LETTER O 
> # 
>
> So ASCII is already taken care of. No issue here :)

Oh, so it is, good catch! I looked for it before, but missed it.

FWIW, I agree with sticking with what Unicode defines here. (I am worried about 
the results in practice and whether enough people will use this check to 
warrant its inclusion in clang-tidy, but not enough to block this patch.)


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

https://reviews.llvm.org/D112916

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

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

[Tong Zhang via Phabricator via cfe-commits]

ztong0001 added a comment.

In D119816#3331658 , @nlopes wrote:

> Well, this patch is just a band-aid and a disaster waiting to happen.
> If kmalloc is tagged with an `__attribute__` stating the allocation size, 
> then you can't dereference beyond that limit or risk the alias analysis do 
> something you don't want. You are triggering UB like that.
> Can't you just remove the `__attribute__` from kmalloc instead to avoid 
> triggering UB?

I am not sure I fully get what you are saying.
What I am suggesting is something like below to avoid bounds check pass from 
running on this function.

  diff --git a/net/core/skbuff.c b/net/core/skbuff.c
  index 9d0388bed0c1..186fca35266d 100644
  --- a/net/core/skbuff.c
  +++ b/net/core/skbuff.c
  @@ -1679,7 +1679,7 @@ EXPORT_SYMBOL(__pskb_copy_fclone);
* All the pointers pointing into skb header may change and must be
* reloaded after call to this function.
*/
  -
  +__attribute__((no_sanitize("bounds")))
   int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
   gfp_t gfp_mask)
   {


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119816

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

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

[Nuno Lopes via Phabricator via cfe-commits]

nlopes added a comment.

In D119816#3332414 , @ztong0001 wrote:

> In D119816#3331658 , @nlopes wrote:
>
>> Well, this patch is just a band-aid and a disaster waiting to happen.
>> If kmalloc is tagged with an `__attribute__` stating the allocation size, 
>> then you can't dereference beyond that limit or risk the alias analysis do 
>> something you don't want. You are triggering UB like that.
>> Can't you just remove the `__attribute__` from kmalloc instead to avoid 
>> triggering UB?
>
> I am not sure I fully get what you are saying.
> What I am suggesting is something like below to avoid bounds check pass from 
> running on this function.
>
>   diff --git a/net/core/skbuff.c b/net/core/skbuff.c
>   index 9d0388bed0c1..186fca35266d 100644
>   --- a/net/core/skbuff.c
>   +++ b/net/core/skbuff.c
>   @@ -1679,7 +1679,7 @@ EXPORT_SYMBOL(__pskb_copy_fclone);
> * All the pointers pointing into skb header may change and must be
> * reloaded after call to this function.
> */
>   -
>   +__attribute__((no_sanitize("bounds")))
>int pskb_expand_head(struct sk_buff *skb, int nhead, int ntail,
>gfp_t gfp_mask)
>{

The main issue is that the kernel is wrong. It has a bug. The sanitizer's error 
is not a false-positive!
So what you are proposing is a band-aid. It's not a real solution and it's just 
masking a wider problem. LLVM knows that kmalloc(x) allocates x bytes because 
someone placed an `__attribute__ ((alloc_size (1)))` on kmalloc. That attribute 
is just wrong and should be removed. It allows LLVM to mark all accesses beyond 
`kmalloc(x) + x - 1` as undefined behavior.

TL;DR: this patch is not the solution for your problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119816

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

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

[Tong Zhang via Phabricator via cfe-commits]

ztong0001 added a comment.

In D119816#3331797 , @melver wrote:

> Right, I was able to repro this. The problem is the trap, which generally 
> sucks that no_sanitize still leaves in the trap.
>
> We also have -fno-sanitize-undefined-trap-on-error, which seems to have no 
> effect either (should it?).
>
> So I think there are 2 problems:
>
> 1. Clang still emitting traps even though it shouldn't.
>
> 2. The Linux kernel problem.
>
> I think it's fine if you address problem 1 with this, as it's an oversight. 
> But I think problem 2 wants to be solved differently as I suggested.

I haven't tried -fno-sanitize-undefined-trap-on-error yet.

IMO trap in kernel gives a generic crash message which is... hard to tell from 
other cases without further investigating. If I enable KASAN kernel will print 
out something like

`
[1.197953] BUG: KASAN: use-after-free in __pci_enable_msi_range+0x234/0x320
[1.198327] Freed by task 1:
[1.198327]  kfree+0x8f/0x2b0
[1.198327]  msi_free_msi_descs_range+0xf5/0x130
`

I agree with you that there are two problems.
I think it makes sense to let optimizer aware of `ksize()` if the kernel API 
won't change dramatically in the future.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119816

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

[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

[Tong Zhang via Phabricator via cfe-commits]

ztong0001 added a comment.

In D119816#3332441 , @nlopes wrote:

> The main issue is that the kernel is wrong. It has a bug. The sanitizer's 
> error is not a false-positive!
> So what you are proposing is a band-aid. It's not a real solution and it's 
> just masking a wider problem. LLVM knows that kmalloc(x) allocates x bytes 
> because someone placed an `__attribute__ ((alloc_size (1)))` on kmalloc. That 
> attribute is just wrong and should be removed. It allows LLVM to mark all 
> accesses beyond `kmalloc(x) + x - 1` as undefined behavior.

But isn't this something the author intended to do in order to catch an error? 
`ksize()` case makes some exceptions out of this.

> TL;DR: this patch is not the solution for your problems.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119816

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

[PATCH] D119409: [C++20] [Modules] Remain dynamic initializing internal-linkage variables in module interface unit

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added a comment.

In D119409#3332313 , @dblaikie wrote:

> 

That's interesting data.  I guess one could emit the out-of-line bodies into 
their own sections, and then rely on linker section GC to elide them in the 
static link.  But if you're building an SO, you presumably need to provide them 
unilaterally in the static link, and callers would use a PLT to get to them.  
One could continue emitting them COMDAT, and unilaterally emit them in the 
module interface's object file, but that will still leave the SO problem.  I 
think.

> I'm not sure if Richard Smith made a choice about not homing inline functions 
> in C++20 modules because of these results, or for other reasons, or just as a 
> consequence of the implementation -

Richard and I discussed taking advantage of this kind of new home location, 
certainly for key-less polymorphic classes.  I was against it as it was more 
work :) Blame me.


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

https://reviews.llvm.org/D119409

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

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

FWIW, https://github.com/llvm/llvm-project/issues/53931


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109239

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

[PATCH] D120140: [clang-format] Avoid inserting space after C++ casts.

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:1951
+// FIXME: Maybe we should handle identifiers ending with "_cast",
+// e.g. bit_cast?
+return true;

Very unlikely `bit_cast` gives you something callable; `any_cast` may. I think 
keywords are ok for now, and cheap. Others may need a different strategy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120140

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

[PATCH] D120149: [clang][dataflow] Add support for global storage values

[Stanislav Gatev via Phabricator via cfe-commits]

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

This is part of the implementation of the dataflow analysis framework.
See "[RFC] A dataflow analysis framework for Clang AST" on cfe-dev.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120149

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2153,4 +2153,171 @@
   });
 }
 
+TEST_F(TransferTest, StaticIntSingleVarDecl) {
+  std::string Code = R"(
+void target() {
+  static int Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+EXPECT_TRUE(isa_and_nonnull(FooVal));
+  });
+}
+
+TEST_F(TransferTest, StaticIntGroupVarDecl) {
+  std::string Code = R"(
+void target() {
+  static int Foo, Bar;
+  (void)0;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const StorageLocation *FooLoc =
+Env.getStorageLocation(*FooDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(FooLoc));
+
+const StorageLocation *BarLoc =
+Env.getStorageLocation(*BarDecl, SkipPast::None);
+ASSERT_TRUE(isa_and_nonnull(BarLoc));
+
+const Value *FooVal = Env.getValue(*FooLoc);
+EXPECT_TRUE(isa_and_nonnull(FooVal));
+
+const Value *BarVal = Env.getValue(*BarLoc);
+EXPECT_TRUE(isa_and_nonnull(BarVal));
+
+EXPECT_NE(FooVal, BarVal);
+  });
+}
+
+TEST_F(TransferTest, GlobalIntVarDecl) {
+  std::string Code = R"(
+static int Foo;
+
+void target() {
+  int Bar = Foo;
+  int Baz = Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const Value *BarVal =
+cast(Env.getValue(*BarDecl, SkipPast::None));
+const Value *BazVal =
+cast(Env.getValue(*BazDecl, SkipPast::None));
+EXPECT_EQ(BarVal, BazVal);
+  });
+}
+
+TEST_F(TransferTest, StaticMemberIntVarDecl) {
+  std::string Code = R"(
+struct A {
+  static int Foo;
+};
+
+void target(A a) {
+  int Bar = a.Foo;
+  int Baz = a.Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
+ASSERT_THAT(BazDecl, NotNull());
+
+const Value *BarVal =
+  

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp abandoned this revision.
carlosgalvezp added a comment.

Understood, thanks a lot for the clarification and for the time taken!

I will then abandon this patch given the high risk involved. I will forward 
this feedback to MISRA in case there is a possibility to publish the upcoming 
revision in open-source-friendly terms.


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

https://reviews.llvm.org/D112730

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

[PATCH] D109239: Add support for floating-option `-ffp-eval-method` and for new `pragma clang fp eval-method`

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D109239#3332521 , @jdoerfert wrote:

> FWIW, https://github.com/llvm/llvm-project/issues/53931

Yes. Working on it. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109239

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

[PATCH] D120149: [clang][dataflow] Add support for global storage values

[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/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:74-77
+  auto *V = dyn_cast(&D);
+  if (V == nullptr)
+return;
+  initGlobalVar(*V, Env);





Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:80
+
+/// Initializes global storage values in sub-expressions of `S`.
+static void initGlobalVars(const Stmt &S, Environment &Env) {

maybe clarify that it intializes both declarations that are present *in* the 
substatements and those referenced *from* the sub statements?



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:140
+
+// Global vars are initialized in `Environment`.
+if (D.hasGlobalStorage())





Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:305
+
+auto &Loc = Env.createStorageLocation(*S);
+if (VarDeclLoc->getType()->isReferenceType()) {

should this be in the `else` branch? As is, `Loc` looks unused if the condition 
is true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120149

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

[PATCH] D119061: [Clang] noinline call site attribute

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

ping :)


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

https://reviews.llvm.org/D119061

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

[clang] 805f7a4 - [clang] Add `ObjCProtocolLoc` to represent protocol references

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2022-02-18T15:24:00-05:00
New Revision: 805f7a4fa4ce97277c3b73d0c204fc3aa4b072e1

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

LOG: [clang] Add `ObjCProtocolLoc` to represent protocol references

Add `ObjCProtocolLoc` which behaves like `TypeLoc` but for
`ObjCProtocolDecl` references.

RecursiveASTVisitor now synthesizes `ObjCProtocolLoc` during traversal
and the `ObjCProtocolLoc` can be stored in a `DynTypedNode`.

In a follow up patch, I'll update clangd to make use of this
to properly support protocol references for hover + goto definition.

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

Added: 


Modified: 
clang/include/clang/AST/ASTFwd.h
clang/include/clang/AST/ASTTypeTraits.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/include/clang/AST/TypeLoc.h
clang/lib/AST/ASTTypeTraits.cpp
clang/lib/AST/ParentMapContext.cpp
clang/unittests/AST/RecursiveASTVisitorTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTFwd.h 
b/clang/include/clang/AST/ASTFwd.h
index fdbd603ce5d04..f84b3238e32b5 100644
--- a/clang/include/clang/AST/ASTFwd.h
+++ b/clang/include/clang/AST/ASTFwd.h
@@ -33,6 +33,7 @@ class OMPClause;
 class Attr;
 #define ATTR(A) class A##Attr;
 #include "clang/Basic/AttrList.inc"
+class ObjCProtocolLoc;
 
 } // end namespace clang
 

diff  --git a/clang/include/clang/AST/ASTTypeTraits.h 
b/clang/include/clang/AST/ASTTypeTraits.h
index 6d96146a4d455..cd6b5143bf790 100644
--- a/clang/include/clang/AST/ASTTypeTraits.h
+++ b/clang/include/clang/AST/ASTTypeTraits.h
@@ -160,6 +160,7 @@ class ASTNodeKind {
 NKI_Attr,
 #define ATTR(A) NKI_##A##Attr,
 #include "clang/Basic/AttrList.inc"
+NKI_ObjCProtocolLoc,
 NKI_NumberOfKinds
   };
 
@@ -213,6 +214,7 @@ KIND_TO_KIND_ID(Stmt)
 KIND_TO_KIND_ID(Type)
 KIND_TO_KIND_ID(OMPClause)
 KIND_TO_KIND_ID(Attr)
+KIND_TO_KIND_ID(ObjCProtocolLoc)
 KIND_TO_KIND_ID(CXXBaseSpecifier)
 #define DECL(DERIVED, BASE) KIND_TO_KIND_ID(DERIVED##Decl)
 #include "clang/AST/DeclNodes.inc"
@@ -499,7 +501,7 @@ class DynTypedNode {
   /// have storage or unique pointers and thus need to be stored by value.
   llvm::AlignedCharArrayUnion
+  QualType, TypeLoc, ObjCProtocolLoc>
   Storage;
 };
 
@@ -570,6 +572,10 @@ template <>
 struct DynTypedNode::BaseConverter
 : public PtrConverter {};
 
+template <>
+struct DynTypedNode::BaseConverter
+: public ValueConverter {};
+
 // The only operation we allow on unsupported types is \c get.
 // This allows to conveniently use \c DynTypedNode when having an arbitrary
 // AST node that is not supported, but prevents misuse - a user cannot create

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index f62dc36de556e..16da64100d424 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -324,6 +324,12 @@ template  class RecursiveASTVisitor {
   /// \returns false if the visitation was terminated early, true otherwise.
   bool TraverseConceptReference(const ConceptReference &C);
 
+  /// Recursively visit an Objective-C protocol reference with location
+  /// information.
+  ///
+  /// \returns false if the visitation was terminated early, true otherwise.
+  bool TraverseObjCProtocolLoc(ObjCProtocolLoc ProtocolLoc);
+
   //  Methods on Attrs 
 
   // Visit an attribute.
@@ -1340,7 +1346,12 @@ 
DEF_TRAVERSE_TYPELOC(DependentTemplateSpecializationType, {
 DEF_TRAVERSE_TYPELOC(PackExpansionType,
  { TRY_TO(TraverseTypeLoc(TL.getPatternLoc())); })
 
-DEF_TRAVERSE_TYPELOC(ObjCTypeParamType, {})
+DEF_TRAVERSE_TYPELOC(ObjCTypeParamType, {
+  for (unsigned I = 0, N = TL.getNumProtocols(); I != N; ++I) {
+ObjCProtocolLoc ProtocolLoc(TL.getProtocol(I), TL.getProtocolLoc(I));
+TRY_TO(TraverseObjCProtocolLoc(ProtocolLoc));
+  }
+})
 
 DEF_TRAVERSE_TYPELOC(ObjCInterfaceType, {})
 
@@ -1351,6 +1362,10 @@ DEF_TRAVERSE_TYPELOC(ObjCObjectType, {
 TRY_TO(TraverseTypeLoc(TL.getBaseLoc()));
   for (unsigned i = 0, n = TL.getNumTypeArgs(); i != n; ++i)
 TRY_TO(TraverseTypeLoc(TL.getTypeArgTInfo(i)->getTypeLoc()));
+  for (unsigned I = 0, N = TL.getNumProtocols(); I != N; ++I) {
+ObjCProtocolLoc ProtocolLoc(TL.getProtocol(I), TL.getProtocolLoc(I));
+TRY_TO(TraverseObjCProtocolLoc(ProtocolLoc));
+  }
 })
 
 DEF_TRAVERSE_TYPELOC(ObjCObjectPointerType,
@@ -1541,12 +1556,16 @@ DEF_TRAVERSE_DECL(
 DEF_TRAVERSE_DECL(ObjCCompatibleAliasDecl, {// FIXME: implement
})
 
-DEF_TRAVERSE_DECL(ObjCCategoryDecl, {// FIXME: implement
+DEF_TRAVERSE_DECL(ObjCCategoryDecl, {
   if (ObjCTypeParamList *typeParamList = D->getTypeP

[clang-tools-extra] 54a962b - [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support

[David Goldman via cfe-commits]

Author: David Goldman
Date: 2022-02-18T15:24:00-05:00
New Revision: 54a962bbfee86d5af90d5fdd39b4ff4ec8030f12

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

LOG: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support

This removes clangd's existing workaround in favor of proper support
via the newly added `ObjCProtocolLoc`. This improves support by
allowing clangd to properly identify which protocol is selected
now that `ObjCProtocolLoc` gets its own ASTNode.

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

Added: 


Modified: 
clang-tools-extra/clangd/FindTarget.cpp
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/FindTargetTests.cpp
clang-tools-extra/clangd/unittests/HoverTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/FindTarget.cpp 
b/clang-tools-extra/clangd/FindTarget.cpp
index e96aa25fd780c..1b7b7de4f9047 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -453,15 +453,6 @@ struct TargetFinder {
   void VisitObjCInterfaceType(const ObjCInterfaceType *OIT) {
 Outer.add(OIT->getDecl(), Flags);
   }
-  void VisitObjCObjectType(const ObjCObjectType *OOT) {
-// Make all of the protocols targets since there's no child nodes for
-// protocols. This isn't needed for the base type, which *does* have a
-// child `ObjCInterfaceTypeLoc`. This structure is a hack, but it works
-// well for go-to-definition.
-unsigned NumProtocols = OOT->getNumProtocols();
-for (unsigned I = 0; I < NumProtocols; I++)
-  Outer.add(OOT->getProtocol(I), Flags);
-  }
 };
 Visitor(*this, Flags).Visit(T.getTypePtr());
   }
@@ -547,6 +538,8 @@ allTargetDecls(const DynTypedNode &N, const 
HeuristicResolver *Resolver) {
 Finder.add(TAL->getArgument(), Flags);
   else if (const CXXBaseSpecifier *CBS = N.get())
 Finder.add(CBS->getTypeSourceInfo()->getType(), Flags);
+  else if (const ObjCProtocolLoc *PL = N.get())
+Finder.add(PL->getProtocol(), Flags);
   return Finder.takeDecls();
 }
 
@@ -669,25 +662,7 @@ llvm::SmallVector refInDecl(const Decl *D,
   {OMD}});
 }
 
-void visitProtocolList(
-llvm::iterator_range Protocols,
-llvm::iterator_range Locations) {
-  for (const auto &P : llvm::zip(Protocols, Locations)) {
-Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
-std::get<1>(P),
-/*IsDecl=*/false,
-{std::get<0>(P)}});
-  }
-}
-
-void VisitObjCInterfaceDecl(const ObjCInterfaceDecl *OID) {
-  if (OID->isThisDeclarationADefinition())
-visitProtocolList(OID->protocols(), OID->protocol_locs());
-  Base::VisitObjCInterfaceDecl(OID); // Visit the interface's name.
-}
-
 void VisitObjCCategoryDecl(const ObjCCategoryDecl *OCD) {
-  visitProtocolList(OCD->protocols(), OCD->protocol_locs());
   // getLocation is the extended class's location, not the category's.
   Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
   OCD->getLocation(),
@@ -709,12 +684,6 @@ llvm::SmallVector refInDecl(const Decl *D,
   /*IsDecl=*/true,
   {OCID->getCategoryDecl()}});
 }
-
-void VisitObjCProtocolDecl(const ObjCProtocolDecl *OPD) {
-  if (OPD->isThisDeclarationADefinition())
-visitProtocolList(OPD->protocols(), OPD->protocol_locs());
-  Base::VisitObjCProtocolDecl(OPD); // Visit the protocol's name.
-}
   };
 
   Visitor V{Resolver};
@@ -944,16 +913,6 @@ refInTypeLoc(TypeLoc L, const HeuristicResolver *Resolver) 
{
   /*IsDecl=*/false,
   {L.getIFaceDecl()}});
 }
-
-void VisitObjCObjectTypeLoc(ObjCObjectTypeLoc L) {
-  unsigned NumProtocols = L.getNumProtocols();
-  for (unsigned I = 0; I < NumProtocols; I++) {
-Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
-L.getProtocolLoc(I),
-/*IsDecl=*/false,
-{L.getProtocol(I)}});
-  }
-}
   };
 
   Visitor V{Resolver};
@@ -1049,6 +1008,11 @@ class ExplicitReferenceCollector
 return RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(L);
   }
 
+  bool TraverseObjCProtocolLoc(ObjCProtocolLoc ProtocolLoc) {
+visitNode(DynTypedNode::create(ProtocolLoc));
+return true;
+  }
+
   bool TraverseConstructorInitializer(CXXCtorInitializer *Init) {
 visitNode(DynTypedNode::create(*Init));
 return RecursiveAS

[PATCH] D119363: [clang] Add `ObjCProtocolLoc` to represent protocol references

[David Goldman 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 rG805f7a4fa4ce: [clang] Add `ObjCProtocolLoc` to represent 
protocol references (authored by dgoldman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119363

Files:
  clang/include/clang/AST/ASTFwd.h
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/AST/TypeLoc.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/AST/ParentMapContext.cpp
  clang/unittests/AST/RecursiveASTVisitorTest.cpp

Index: clang/unittests/AST/RecursiveASTVisitorTest.cpp
===
--- clang/unittests/AST/RecursiveASTVisitorTest.cpp
+++ clang/unittests/AST/RecursiveASTVisitorTest.cpp
@@ -60,6 +60,12 @@
   EndTraverseEnum,
   StartTraverseTypedefType,
   EndTraverseTypedefType,
+  StartTraverseObjCInterface,
+  EndTraverseObjCInterface,
+  StartTraverseObjCProtocol,
+  EndTraverseObjCProtocol,
+  StartTraverseObjCProtocolLoc,
+  EndTraverseObjCProtocolLoc,
 };
 
 class CollectInterestingEvents
@@ -97,18 +103,43 @@
 return Ret;
   }
 
+  bool TraverseObjCInterfaceDecl(ObjCInterfaceDecl *ID) {
+Events.push_back(VisitEvent::StartTraverseObjCInterface);
+bool Ret = RecursiveASTVisitor::TraverseObjCInterfaceDecl(ID);
+Events.push_back(VisitEvent::EndTraverseObjCInterface);
+
+return Ret;
+  }
+
+  bool TraverseObjCProtocolDecl(ObjCProtocolDecl *PD) {
+Events.push_back(VisitEvent::StartTraverseObjCProtocol);
+bool Ret = RecursiveASTVisitor::TraverseObjCProtocolDecl(PD);
+Events.push_back(VisitEvent::EndTraverseObjCProtocol);
+
+return Ret;
+  }
+
+  bool TraverseObjCProtocolLoc(ObjCProtocolLoc ProtocolLoc) {
+Events.push_back(VisitEvent::StartTraverseObjCProtocolLoc);
+bool Ret = RecursiveASTVisitor::TraverseObjCProtocolLoc(ProtocolLoc);
+Events.push_back(VisitEvent::EndTraverseObjCProtocolLoc);
+
+return Ret;
+  }
+
   std::vector takeEvents() && { return std::move(Events); }
 
 private:
   std::vector Events;
 };
 
-std::vector collectEvents(llvm::StringRef Code) {
+std::vector collectEvents(llvm::StringRef Code,
+  const Twine &FileName = "input.cc") {
   CollectInterestingEvents Visitor;
   clang::tooling::runToolOnCode(
   std::make_unique(
   [&](clang::ASTContext &Ctx) { Visitor.TraverseAST(Ctx); }),
-  Code);
+  Code, FileName);
   return std::move(Visitor).takeEvents();
 }
 } // namespace
@@ -139,3 +170,28 @@
   VisitEvent::EndTraverseTypedefType,
   VisitEvent::EndTraverseEnum));
 }
+
+TEST(RecursiveASTVisitorTest, InterfaceDeclWithProtocols) {
+  // Check interface and its protocols are visited.
+  llvm::StringRef Code = R"cpp(
+  @protocol Foo
+  @end
+  @protocol Bar
+  @end
+
+  @interface SomeObject 
+  @end
+  )cpp";
+
+  EXPECT_THAT(collectEvents(Code, "input.m"),
+  ElementsAre(VisitEvent::StartTraverseObjCProtocol,
+  VisitEvent::EndTraverseObjCProtocol,
+  VisitEvent::StartTraverseObjCProtocol,
+  VisitEvent::EndTraverseObjCProtocol,
+  VisitEvent::StartTraverseObjCInterface,
+  VisitEvent::StartTraverseObjCProtocolLoc,
+  VisitEvent::EndTraverseObjCProtocolLoc,
+  VisitEvent::StartTraverseObjCProtocolLoc,
+  VisitEvent::EndTraverseObjCProtocolLoc,
+  VisitEvent::EndTraverseObjCInterface));
+}
Index: clang/lib/AST/ParentMapContext.cpp
===
--- clang/lib/AST/ParentMapContext.cpp
+++ clang/lib/AST/ParentMapContext.cpp
@@ -330,6 +330,9 @@
 DynTypedNode createDynTypedNode(const NestedNameSpecifierLoc &Node) {
   return DynTypedNode::create(Node);
 }
+template <> DynTypedNode createDynTypedNode(const ObjCProtocolLoc &Node) {
+  return DynTypedNode::create(Node);
+}
 /// @}
 
 /// A \c RecursiveASTVisitor that builds a map from nodes to their
@@ -398,11 +401,14 @@
 }
   }
 
+  template  static bool isNull(T Node) { return !Node; }
+  static bool isNull(ObjCProtocolLoc Node) { return false; }
+
   template 
   bool TraverseNode(T Node, MapNodeTy MapNode, BaseTraverseFn BaseTraverse,
 MapTy *Parents) {
-if (!Node)
+if (isNull(Node))
   return true;
 addParent(MapNode, Parents);
 ParentStack.push_back(createDynTypedNode(Node));
@@ -433,6 +439,12 @@
 AttrNode, AttrNode, [&] { return VisitorBase::TraverseAttr(AttrNode); },
 &Map.PointerParents);
   }
+  bool TraverseObjCProtocolLoc(ObjCProtocolLoc ProtocolLocNode) {
+return TraverseNode(
+ProtocolLocNode, DynTypedNode::create(Protoco

[PATCH] D119366: [clangd] Use `ObjCProtocolLoc` for generalized ObjC protocol support

[David Goldman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG54a962bbfee8: [clangd] Use `ObjCProtocolLoc` for generalized 
ObjC protocol support (authored by dgoldman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119366

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/Selection.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -2522,6 +2522,22 @@
 HI.Definition = "@property(nonatomic, assign, unsafe_unretained, "
 "readwrite) int prop1;";
   }},
+  {
+  R"cpp(
+  @protocol MYProtocol
+  @end
+  @interface MYObject
+  @end
+
+  @interface MYObject (Ext) <[[MYProt^ocol]]>
+  @end
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "MYProtocol";
+HI.Kind = index::SymbolKind::Protocol;
+HI.NamespaceScope = "";
+HI.Definition = "@protocol MYProtocol\n@end";
+  }},
   {R"objc(
 @interface Foo
 @end
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -946,11 +946,9 @@
   EXPECT_DECLS("ObjCCategoryImplDecl", "@interface Foo(Ext)");
 
   Code = R"cpp(
-@protocol Foo
-@end
-void test([[id]] p);
+void test(id p);
   )cpp";
-  EXPECT_DECLS("ObjCObjectTypeLoc", "@protocol Foo");
+  EXPECT_DECLS("ParmVarDecl", "id p");
 
   Code = R"cpp(
 @class C;
@@ -966,7 +964,7 @@
 @end
 void test(C<[[Foo]]> *p);
   )cpp";
-  EXPECT_DECLS("ObjCObjectTypeLoc", "@protocol Foo");
+  EXPECT_DECLS("ObjCProtocolLoc", "@protocol Foo");
 
   Code = R"cpp(
 @class C;
@@ -976,8 +974,17 @@
 @end
 void test(C<[[Foo]], Bar> *p);
   )cpp";
-  // FIXME: We currently can't disambiguate between multiple protocols.
-  EXPECT_DECLS("ObjCObjectTypeLoc", "@protocol Foo", "@protocol Bar");
+  EXPECT_DECLS("ObjCProtocolLoc", "@protocol Foo");
+
+  Code = R"cpp(
+@class C;
+@protocol Foo
+@end
+@protocol Bar
+@end
+void test(C *p);
+  )cpp";
+  EXPECT_DECLS("ObjCProtocolLoc", "@protocol Bar");
 
   Code = R"cpp(
 @interface Foo
Index: clang-tools-extra/clangd/Selection.cpp
===
--- clang-tools-extra/clangd/Selection.cpp
+++ clang-tools-extra/clangd/Selection.cpp
@@ -684,6 +684,9 @@
 return traverseNode(
 &QX, [&] { return TraverseTypeLoc(QX.getUnqualifiedLoc()); });
   }
+  bool TraverseObjCProtocolLoc(ObjCProtocolLoc PL) {
+return traverseNode(&PL, [&] { return Base::TraverseObjCProtocolLoc(PL); });
+  }
   // Uninteresting parts of the AST that don't have locations within them.
   bool TraverseNestedNameSpecifier(NestedNameSpecifier *) { return true; }
   bool TraverseType(QualType) { return true; }
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -453,15 +453,6 @@
   void VisitObjCInterfaceType(const ObjCInterfaceType *OIT) {
 Outer.add(OIT->getDecl(), Flags);
   }
-  void VisitObjCObjectType(const ObjCObjectType *OOT) {
-// Make all of the protocols targets since there's no child nodes for
-// protocols. This isn't needed for the base type, which *does* have a
-// child `ObjCInterfaceTypeLoc`. This structure is a hack, but it works
-// well for go-to-definition.
-unsigned NumProtocols = OOT->getNumProtocols();
-for (unsigned I = 0; I < NumProtocols; I++)
-  Outer.add(OOT->getProtocol(I), Flags);
-  }
 };
 Visitor(*this, Flags).Visit(T.getTypePtr());
   }
@@ -547,6 +538,8 @@
 Finder.add(TAL->getArgument(), Flags);
   else if (const CXXBaseSpecifier *CBS = N.get())
 Finder.add(CBS->getTypeSourceInfo()->getType(), Flags);
+  else if (const ObjCProtocolLoc *PL = N.get())
+Finder.add(PL->getProtocol(), Flags);
   return Finder.takeDecls();
 }
 
@@ -669,25 +662,7 @@
   {OMD}});
 }
 
-void visitProtocolList(
-llvm::iterator_range Protocols,
-llvm::iterator_range Locations) {
-  for (const auto &P : llvm::zip(Protocols, Locations)) {
-Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
-std::get<1>(P),
-