[PATCH] D109621: [clang][Driver] Default to loading clang.cfg if config file not specified

[Michał Górny via Phabricator via cfe-commits]

mgorny added a comment.

As @sepavloff points out in the linked discussion, we probably need an option 
to disable loading default configuration files too, and we should use it in the 
test suite (via modifying the `%clang` substitutions, I guess), as otherwise 
someone enabling config file support is going to see random test failures.


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

https://reviews.llvm.org/D109621

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

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:5779
 QualType ParamTy = Proto->getParamType(ArgIdx);
+if (ParamTy->containsErrors())
+  continue;

hokein wrote:
> It looks like for the failure case the `ParamTy` for the parameter is a 
> dependent array type, and it violates the "non-dependent" assumption of 
> `clang::ASTContext::getTypeInfoImpl` which is called by  
> `getTypeAlignInChars` in `CheckArgAlignment`.
> 
> so I'd suggest moving the fix to `CheckArgAlignment` line 5685 (adding a 
> `ParamTy->isDependentType()` to the `if` condition).
When I found this problem I fixed it like you are suggesting. But after that I 
replaced it with this check, because it seems there is no reason to try to 
check something on code with errors.

I mean that even if we are not crashing, results from `CheckArgAlignment()` 
can't be trusted if we are passing  something with errors to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133886

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

[PATCH] D133757: [clangd] Turn QueryDriverDatabase into a CompileCommandsAdjuster

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

I think the sticking point for just having `QueryDriverDatabase` run after the 
entirety of `CommandMangler` is this check 

 in `resolveDriver()`, which replaces e.g. `gcc` with `/path/to/clang/bin/gcc`, 
which likely does not actually exist (i.e. `QueryDriverDatabase` will not be 
able to query it, when it might have been able to look up `gcc` in `PATH`).

Do you know what the purpose of this logic in `resolveDriver()` is, and in 
particular:

- why is `gcc` treated differently than say `gcc-12`?
- why is turning `gcc` into `/path/to/clang/bin/gcc` performed **before** 
trying to resolve `gcc` against `PATH`?

If we can change the order of the checks in `resolveDriver()` such that we try 
to resolve `gcc` against `PATH` first, and only turn it into 
`/path/to/clang/bin/gcc` if it was not found in `PATH`, I think that would not 
interfere with `QueryDriverDatabase`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757

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

[PATCH] D109621: [clang][Driver] Default to loading clang.cfg if config file not specified

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

On this topic, it would be great if we could pick up a per-target default 
config file too, if clang is invoked with `clang -target `. Currently 
this is done automatically if clang is invoked as `-clang`, but not 
with an explicit `-target` parameter.


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

https://reviews.llvm.org/D109621

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

[PATCH] D133865: [clang][dataflow] Replace usage of the deprecated overload of `checkDataflow`.

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added inline comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupportTest.cpp:83-88
+AnalysisInputs(
+Code, hasName(Target),
+[](ASTContext &Context, Environment &) {
+  return NoopAnalysis(
+  Context, /*ApplyBuiltinTransfer=*/false);
+}),

Do we need `.withASTBuildArgs({"-fsyntax-only", "-std=c++17"})` here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133865

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

[PATCH] D133843: [clangd] Prefer definitions for gototype and implementation

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D133843#3791127 , @sammccall wrote:

> There's no test here, can you describe the cases you expect this to affect 
> and why the new behavior is better?

right, sorry for the obscure patch. giving examples/details in particular cases 
below.

> For types this seems doubly-dubious at first glance:
>
> - when we've decided to "prefer" a non-definition declaration, why make an 
> exception here?
> - I'd expect the definition to almost-always be the preferred declaration 
> anyway.

So apart from the special cases this happens, at the moment we have a 
discrepancy between go-to-def on a type and go-to-type on a decl, which to me 
seems like a surprise and definitely unexpected. At least to me the main point 
of go-to-type interaction is to get rid of the extra jump, and when it takes me 
to declaration for whatever reason, that benefit is lost.
As for the particular cases I've noticed this happening, it's forward 
declarations visible from the definition of a type, which manifests in a couple 
different forms:

- A header depending on the type forward declaring it, and also being included 
by the TU defining the type (sort of a circular dependency, so might not be as 
important).
- A type that's being forward declared by the same header defining it, while 
the type still being public (which I think is common when there are 
interactions between two different types).
- A private type that's being forward declared in a header and defined in a 
private header or implementation file.

To me it seems like only the last case **might** warrant a jump to declaration, 
but I think it's still to prefer definition in that case. In other cases we 
most definitely want the definition directly.

> For implementation, I think I there's a good argument for this behavior: the 
> purpose of the command is to cut through abstractions to see what this method 
> does in practice, and finding the definition fits with that. I'm happy with 
> this behavior but I think it deserves a comment.
> (There's also a bad argument: users think "implementation" means definition - 
> just because the name is ambiguous doesn't mean we should implement both 
> behaviors!)

Right, this was my motivation as well. Especially in UI-rich editors like 
VSCode, one actually gets snippets around the locations and seeing snippets 
around the definition definitely feels more helpful.

happy to add some comments for the reasoning if you (and possibly others seeing 
this change) also agree that this is more useful


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133843

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

[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 460315.
ChuanqiXu marked 6 inline comments as done.
ChuanqiXu added a comment.

Address comments.


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

https://reviews.llvm.org/D133341

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Builtins.def
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCoroutine.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/CodeGenCoroutines/coro-aligned-alloc-2.cpp
  clang/test/CodeGenCoroutines/coro-aligned-alloc.cpp
  clang/test/SemaCXX/coroutine-alloc-4.cpp

Index: clang/test/SemaCXX/coroutine-alloc-4.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/coroutine-alloc-4.cpp
@@ -0,0 +1,116 @@
+// Tests that we'll find aligned allocation funciton properly.
+// RUN: %clang_cc1 %s -std=c++20 %s -fsyntax-only -verify -fcoro-aligned-allocation
+
+#include "Inputs/std-coroutine.h"
+
+namespace std {
+typedef __SIZE_TYPE__ size_t;
+enum class align_val_t : size_t {};
+}
+
+struct task {
+  struct promise_type {
+auto initial_suspend() { return std::suspend_always{}; }
+auto final_suspend() noexcept { return std::suspend_always{}; }
+auto get_return_object() { return task{}; }
+void unhandled_exception() {}
+void return_value(int) {}
+void *operator new(std::size_t); // expected-warning 1+{{under -fcoro-aligned-allocation, the non-aligned allocation function for the promise type 'f' has higher precedence than the global aligned allocation function}}
+  };
+};
+
+task f() {
+co_return 43;
+}
+
+struct task2 {
+  struct promise_type {
+auto initial_suspend() { return std::suspend_always{}; }
+auto final_suspend() noexcept { return std::suspend_always{}; }
+auto get_return_object() { return task2{}; }
+void unhandled_exception() {}
+void return_value(int) {}
+void *operator new(std::size_t, std::align_val_t);
+  };
+};
+
+// no diagnostic expected
+task2 f1() {
+co_return 43;
+}
+
+struct task3 {
+  struct promise_type {
+auto initial_suspend() { return std::suspend_always{}; }
+auto final_suspend() noexcept { return std::suspend_always{}; }
+auto get_return_object() { return task3{}; }
+void unhandled_exception() {}
+void return_value(int) {}
+void *operator new(std::size_t, std::align_val_t) noexcept;
+void *operator new(std::size_t) noexcept;
+static auto get_return_object_on_allocation_failure() { return task3{}; }
+  };
+};
+
+// no diagnostic expected
+task3 f2() {
+co_return 43;
+}
+
+struct task4 {
+  struct promise_type {
+auto initial_suspend() { return std::suspend_always{}; }
+auto final_suspend() noexcept { return std::suspend_always{}; }
+auto get_return_object() { return task4{}; }
+void unhandled_exception() {}
+void return_value(int) {}
+void *operator new(std::size_t, std::align_val_t, int, double, int) noexcept;
+  };
+};
+
+// no diagnostic expected
+task4 f3(int, double, int) {
+co_return 43;
+}
+
+struct task5 {
+  struct promise_type {
+auto initial_suspend() { return std::suspend_always{}; }
+auto final_suspend() noexcept { return std::suspend_always{}; }
+auto get_return_object() { return task5{}; }
+void unhandled_exception() {}
+void return_value(int) {}
+  };
+};
+
+// no diagnostic expected.
+// The aligned allocation will be declared by the compiler.
+task5 f4() {
+co_return 43;
+}
+
+namespace std {
+  struct nothrow_t {};
+  constexpr nothrow_t nothrow = {};
+}
+
+struct task6 {
+  struct promise_type {
+auto initial_suspend() { return std::suspend_always{}; }
+auto final_suspend() noexcept { return std::suspend_always{}; }
+auto get_return_object() { return task6{}; }
+void unhandled_exception() {}
+void return_value(int) {}
+static task6 get_return_object_on_allocation_failure() { return task6{}; }
+  };
+};
+
+task6 f5() { // expected-error 1+{{unable to find '::operator new(size_t, align_val_t, nothrow_t)' for 'f5'}}
+co_return 43;
+}
+
+void *operator new(std::size_t, std::align_val_t, std::nothrow_t) noexcept; 
+
+task6 f6() {
+co_return 43;
+}
Index: clang/test/CodeGenCoroutines/coro-aligned-alloc.cpp
===
--- /dev/null
+++ clang/test/CodeGenCoroutines/coro-aligned-alloc.cpp
@@ -0,0 +1,190 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -std=c++20 \
+// RUN:   -fcoro-aligned-allocation -S -emit-llvm %s -o - -disable-llvm-passes \
+// RUN:   | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+namespace std {
+typedef __SIZE_TYPE__ size_t;
+enum class align_val_t : size_t

[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

[Chuanqi Xu via Phabricator via cfe-commits]

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



Comment at: clang/test/SemaCXX/coroutine-alloc-4.cpp:49
+void return_value(int) {}
+void *operator new(std::size_t, std::align_val_t) noexcept;
+void *operator new(std::size_t) noexcept;

ychen wrote:
> ChuanqiXu wrote:
> > ychen wrote:
> > > Like this test case, please add additional test cases to check the 
> > > expected look-up order, one test for each consecutive pair should be good.
> > > 
> > > ```
> > > void* T::operator new  ( std::size_t count, std::align_val_t al, 
> > > user-defined-args... );
> > > void* T::operator new  ( std::size_t count, std::align_val_t al);
> > > void* T::operator new  ( std::size_t count, user-defined-args... );
> > > void* T::operator new  ( std::size_t count);
> > > void* operator new  ( std::size_t count, std::align_val_t al );
> > > ```
> > > 
> > > 
> > Yeah, I'm testing this in CodeGenCoroutines. (It is hard to test the 
> > selection in Sema Test)
> Thanks for adding the overload. I think the `noexcept` on operator new is not 
> necessary. Strictly speaking, it is not a conforming API.
The noexcept here is necessary. The specs says the allocation function should 
have a noexcept specifier if get_return_object_on_allocation_failure presents.


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

https://reviews.llvm.org/D133341

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

[PATCH] D133920: [X86][fastcall] Move capability check before free register update

[Phoebe Wang via Phabricator via cfe-commits]

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

Fixes: #57737


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133920

Files:
  clang/lib/CodeGen/TargetInfo.cpp
  clang/test/CodeGen/mangle-windows.c


Index: clang/test/CodeGen/mangle-windows.c
===
--- clang/test/CodeGen/mangle-windows.c
+++ clang/test/CodeGen/mangle-windows.c
@@ -47,7 +47,7 @@
 // X64: define dso_local void @f8(
 
 void __fastcall f9(long long a, char b, char c, short d) {}
-// CHECK: define dso_local x86_fastcallcc void @"\01@f9@20"(i64 noundef %a, i8 
noundef signext %b, i8 noundef signext %c, i16 noundef signext %d)
+// CHECK: define dso_local x86_fastcallcc void @"\01@f9@20"(i64 noundef %a, i8 
inreg noundef signext %b, i8 inreg noundef signext %c, i16 noundef signext %d)
 // X64: define dso_local void @f9(
 
 void f12(void) {}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1771,21 +1771,25 @@
 }
 
 bool X86_32ABIInfo::shouldPrimitiveUseInReg(QualType Ty, CCState &State) const 
{
-  if (!updateFreeRegs(Ty, State))
+  auto CanUseInReg = [this](QualType Ty, CCState &State) {
+if (State.CC == llvm::CallingConv::X86_FastCall ||
+State.CC == llvm::CallingConv::X86_VectorCall ||
+State.CC == llvm::CallingConv::X86_RegCall) {
+  if (getContext().getTypeSize(Ty) > 32)
+return false;
+
+  return (Ty->isIntegralOrEnumerationType() || Ty->isPointerType() ||
+  Ty->isReferenceType());
+}
+return true;
+  };
+
+  if (!CanUseInReg(Ty, State) || !updateFreeRegs(Ty, State))
 return false;
 
   if (IsMCUABI)
 return false;
 
-  if (State.CC == llvm::CallingConv::X86_FastCall ||
-  State.CC == llvm::CallingConv::X86_VectorCall ||
-  State.CC == llvm::CallingConv::X86_RegCall) {
-if (getContext().getTypeSize(Ty) > 32)
-  return false;
-
-return (Ty->isIntegralOrEnumerationType() || Ty->isPointerType() ||
-Ty->isReferenceType());
-  }
 
   return true;
 }


Index: clang/test/CodeGen/mangle-windows.c
===
--- clang/test/CodeGen/mangle-windows.c
+++ clang/test/CodeGen/mangle-windows.c
@@ -47,7 +47,7 @@
 // X64: define dso_local void @f8(
 
 void __fastcall f9(long long a, char b, char c, short d) {}
-// CHECK: define dso_local x86_fastcallcc void @"\01@f9@20"(i64 noundef %a, i8 noundef signext %b, i8 noundef signext %c, i16 noundef signext %d)
+// CHECK: define dso_local x86_fastcallcc void @"\01@f9@20"(i64 noundef %a, i8 inreg noundef signext %b, i8 inreg noundef signext %c, i16 noundef signext %d)
 // X64: define dso_local void @f9(
 
 void f12(void) {}
Index: clang/lib/CodeGen/TargetInfo.cpp
===
--- clang/lib/CodeGen/TargetInfo.cpp
+++ clang/lib/CodeGen/TargetInfo.cpp
@@ -1771,21 +1771,25 @@
 }
 
 bool X86_32ABIInfo::shouldPrimitiveUseInReg(QualType Ty, CCState &State) const {
-  if (!updateFreeRegs(Ty, State))
+  auto CanUseInReg = [this](QualType Ty, CCState &State) {
+if (State.CC == llvm::CallingConv::X86_FastCall ||
+State.CC == llvm::CallingConv::X86_VectorCall ||
+State.CC == llvm::CallingConv::X86_RegCall) {
+  if (getContext().getTypeSize(Ty) > 32)
+return false;
+
+  return (Ty->isIntegralOrEnumerationType() || Ty->isPointerType() ||
+  Ty->isReferenceType());
+}
+return true;
+  };
+
+  if (!CanUseInReg(Ty, State) || !updateFreeRegs(Ty, State))
 return false;
 
   if (IsMCUABI)
 return false;
 
-  if (State.CC == llvm::CallingConv::X86_FastCall ||
-  State.CC == llvm::CallingConv::X86_VectorCall ||
-  State.CC == llvm::CallingConv::X86_RegCall) {
-if (getContext().getTypeSize(Ty) > 32)
-  return false;
-
-return (Ty->isIntegralOrEnumerationType() || Ty->isPointerType() ||
-Ty->isReferenceType());
-  }
 
   return true;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133875: [clang] fix generation of .debug_aranges with LTO (resubmit)

[Azat Khuzhin via Phabricator via cfe-commits]

azat updated this revision to Diff 460321.
azat added a comment.

Fix clang-format and tiny test refactoring


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133875

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/debug-options-aranges.c


Index: clang/test/Driver/debug-options-aranges.c
===
--- /dev/null
+++ clang/test/Driver/debug-options-aranges.c
@@ -0,0 +1,19 @@
+// REQUIRES: lld
+
+// Check that the linker plugin will get -generate-arange-section
+//
+// RUN: %clang -### -g -target x86_64-linux -flto  -fuse-ld=lld 
-gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=PLUGIN_GARANGE %s
+// RUN: %clang -### -g -target x86_64-linux -flto=thin -fuse-ld=lld 
-gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=PLUGIN_GARANGE %s
+// GARANGE: -generate-arange-section
+// PLUGIN_GARANGE: --plugin-opt=-generate-arange-section
+
+// Check that .debug_aranges will be emitted
+//
+// RUN: %clang -g -target x86_64-linux-fuse-ld=lld -gdwarf-aranges 
%s -o - | llvm-dwarfdump --debug-aranges - | FileCheck -check-prefix=ARANGES %s
+// RUN: %clang -g -target x86_64-linux -flto  -fuse-ld=lld -gdwarf-aranges 
%s -o - | llvm-dwarfdump --debug-aranges - | FileCheck -check-prefix=ARANGES %s
+// RUN: %clang -g -target x86_64-linux -flto=thin -fuse-ld=lld -gdwarf-aranges 
%s -o - | llvm-dwarfdump --debug-aranges - | FileCheck -check-prefix=ARANGES %s
+// ARANGES: Address Range Header: length = 0x002c, format = DWARF32, 
version = 0x0002, cu_offset = 0x, addr_size = 0x08, seg_size = 0x00
+
+int main()
+{
+}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -508,6 +508,14 @@
 CmdArgs.push_back(Args.MakeArgString(Plugin));
   }
 
+  // Note, this solution is far from perfect, better to encode it into IR
+  // metadata, but this may not be worth it, since it looks like aranges is on
+  // the way out.
+  if (Args.hasArg(options::OPT_gdwarf_aranges)) {
+CmdArgs.push_back(
+Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+  }
+
   // Try to pass driver level flags relevant to LTO code generation down to
   // the plugin.
 


Index: clang/test/Driver/debug-options-aranges.c
===
--- /dev/null
+++ clang/test/Driver/debug-options-aranges.c
@@ -0,0 +1,19 @@
+// REQUIRES: lld
+
+// Check that the linker plugin will get -generate-arange-section
+//
+// RUN: %clang -### -g -target x86_64-linux -flto  -fuse-ld=lld -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=PLUGIN_GARANGE %s
+// RUN: %clang -### -g -target x86_64-linux -flto=thin -fuse-ld=lld -gdwarf-aranges %s 2>&1 | FileCheck -check-prefix=PLUGIN_GARANGE %s
+// GARANGE: -generate-arange-section
+// PLUGIN_GARANGE: --plugin-opt=-generate-arange-section
+
+// Check that .debug_aranges will be emitted
+//
+// RUN: %clang -g -target x86_64-linux-fuse-ld=lld -gdwarf-aranges %s -o - | llvm-dwarfdump --debug-aranges - | FileCheck -check-prefix=ARANGES %s
+// RUN: %clang -g -target x86_64-linux -flto  -fuse-ld=lld -gdwarf-aranges %s -o - | llvm-dwarfdump --debug-aranges - | FileCheck -check-prefix=ARANGES %s
+// RUN: %clang -g -target x86_64-linux -flto=thin -fuse-ld=lld -gdwarf-aranges %s -o - | llvm-dwarfdump --debug-aranges - | FileCheck -check-prefix=ARANGES %s
+// ARANGES: Address Range Header: length = 0x002c, format = DWARF32, version = 0x0002, cu_offset = 0x, addr_size = 0x08, seg_size = 0x00
+
+int main()
+{
+}
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -508,6 +508,14 @@
 CmdArgs.push_back(Args.MakeArgString(Plugin));
   }
 
+  // Note, this solution is far from perfect, better to encode it into IR
+  // metadata, but this may not be worth it, since it looks like aranges is on
+  // the way out.
+  if (Args.hasArg(options::OPT_gdwarf_aranges)) {
+CmdArgs.push_back(
+Args.MakeArgString("--plugin-opt=-generate-arange-section"));
+  }
+
   // Try to pass driver level flags relevant to LTO code generation down to
   // the plugin.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

[Tom Praschan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG220d85082349: [clangd] Fix hover on symbol introduced by 
using declaration (authored by tom-anders).

Changed prior to commit:
  https://reviews.llvm.org/D133664?vs=460058&id=460328#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133664

Files:
  clang-tools-extra/clangd/Hover.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
@@ -1630,6 +1630,38 @@
 HI.Type = "int";
 HI.Definition = "int foo";
   }},
+  {
+  R"cpp(// Function definition via using declaration
+namespace ns { 
+  void foo(); 
+}
+int main() {
+  using ns::foo;
+  ^[[foo]]();
+}
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Function;
+HI.NamespaceScope = "ns::";
+HI.Type = "void ()";
+HI.Definition = "void foo()";
+HI.Documentation = "";
+HI.ReturnType = "void";
+HI.Parameters = std::vector{};
+  }},
+  {
+  R"cpp( // using declaration and two possible function declarations
+namespace ns { void foo(int); void foo(char); }
+using ns::foo;
+template  void bar() { [[f^oo]](T{}); }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Using;
+HI.NamespaceScope = "";
+HI.Definition = "using ns::foo";
+  }},
   {
   R"cpp(// Macro
 #define MACRO 0
@@ -1734,6 +1766,25 @@
 HI.Definition = "ONE";
 HI.Value = "0";
   }},
+  {
+  R"cpp(// C++20's using enum
+enum class Hello {
+  ONE, TWO, THREE,
+};
+void foo() {
+  using enum Hello;
+  Hello hello = [[O^NE]];
+}
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "ONE";
+HI.Kind = index::SymbolKind::EnumConstant;
+HI.NamespaceScope = "";
+HI.LocalScope = "Hello::";
+HI.Type = "enum Hello";
+HI.Definition = "ONE";
+HI.Value = "0";
+  }},
   {
   R"cpp(// Enumerator in anonymous enum
 enum {
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -1006,6 +1006,37 @@
   HI.CallPassType.emplace(PassType);
 }
 
+const NamedDecl *pickDeclToUse(llvm::ArrayRef Candidates) {
+  if (Candidates.empty())
+return nullptr;
+
+  // This is e.g the case for
+  // namespace ns { void foo(); }
+  // void bar() { using ns::foo; f^oo(); }
+  // One declaration in Candidates will refer to the using declaration,
+  // which isn't really useful for Hover. So use the other one,
+  // which in this example would be the actual declaration of foo.
+  if (Candidates.size() <= 2) {
+if (llvm::isa(Candidates.front()))
+  return Candidates.back();
+return Candidates.front();
+  }
+
+  // For something like
+  // namespace ns { void foo(int); void foo(char); }
+  // using ns::foo;
+  // template  void bar() { fo^o(T{}); }
+  // we actually want to show the using declaration,
+  // it's not clear which declaration to pick otherwise.
+  auto BaseDecls = llvm::make_filter_range(Candidates, [](const NamedDecl *D) {
+return llvm::isa(D);
+  });
+  if (std::distance(BaseDecls.begin(), BaseDecls.end()) == 1)
+return *BaseDecls.begin();
+
+  return Candidates.front();
+}
+
 } // namespace
 
 llvm::Optional getHover(ParsedAST &AST, Position Pos,
@@ -1081,11 +1112,11 @@
   // FIXME: Fill in HighlightRange with range coming from N->ASTNode.
   auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias,
 AST.getHeuristicResolver());
-  if (!Decls.empty()) {
-HI = getHoverContents(Decls.front(), PP, Index, TB);
+  if (const auto *DeclToUse = pickDeclToUse(Decls)) {
+HI = getHoverContents(DeclToUse, PP, Index, TB);
 // Layout info only shown when hovering on the field/class itself.
-if (Decls.front() == N->ASTNode.get())
-  addLayoutInfo(*Decls.front(), *HI);
+if (DeclToUse == N->ASTNode.get())
+  addLayoutInfo(*DeclToUse, *HI);
 // Look for a close enclosing expression to show the value of.
 if (!HI->Value)
   HI->

[clang-tools-extra] 220d850 - [clangd] Fix hover on symbol introduced by using declaration

[Tom Praschan via cfe-commits]

Author: Tom Praschan
Date: 2022-09-15T13:02:58+02:00
New Revision: 220d850823494aad48d984395512e2ac74c666de

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

LOG: [clangd] Fix hover on symbol introduced by using declaration

This fixes https://github.com/clangd/clangd/issues/1284. The example
there was C++20's "using enum", but I noticed that we had the same issue
for other using-declarations.

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

Added: 


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

Removed: 




diff  --git a/clang-tools-extra/clangd/Hover.cpp 
b/clang-tools-extra/clangd/Hover.cpp
index 2ef6104e3a43..b587d9c5f1e7 100644
--- a/clang-tools-extra/clangd/Hover.cpp
+++ b/clang-tools-extra/clangd/Hover.cpp
@@ -1006,6 +1006,37 @@ void maybeAddCalleeArgInfo(const SelectionTree::Node *N, 
HoverInfo &HI,
   HI.CallPassType.emplace(PassType);
 }
 
+const NamedDecl *pickDeclToUse(llvm::ArrayRef Candidates) {
+  if (Candidates.empty())
+return nullptr;
+
+  // This is e.g the case for
+  // namespace ns { void foo(); }
+  // void bar() { using ns::foo; f^oo(); }
+  // One declaration in Candidates will refer to the using declaration,
+  // which isn't really useful for Hover. So use the other one,
+  // which in this example would be the actual declaration of foo.
+  if (Candidates.size() <= 2) {
+if (llvm::isa(Candidates.front()))
+  return Candidates.back();
+return Candidates.front();
+  }
+
+  // For something like
+  // namespace ns { void foo(int); void foo(char); }
+  // using ns::foo;
+  // template  void bar() { fo^o(T{}); }
+  // we actually want to show the using declaration,
+  // it's not clear which declaration to pick otherwise.
+  auto BaseDecls = llvm::make_filter_range(Candidates, [](const NamedDecl *D) {
+return llvm::isa(D);
+  });
+  if (std::distance(BaseDecls.begin(), BaseDecls.end()) == 1)
+return *BaseDecls.begin();
+
+  return Candidates.front();
+}
+
 } // namespace
 
 llvm::Optional getHover(ParsedAST &AST, Position Pos,
@@ -1081,11 +1112,11 @@ llvm::Optional getHover(ParsedAST &AST, 
Position Pos,
   // FIXME: Fill in HighlightRange with range coming from N->ASTNode.
   auto Decls = explicitReferenceTargets(N->ASTNode, DeclRelation::Alias,
 AST.getHeuristicResolver());
-  if (!Decls.empty()) {
-HI = getHoverContents(Decls.front(), PP, Index, TB);
+  if (const auto *DeclToUse = pickDeclToUse(Decls)) {
+HI = getHoverContents(DeclToUse, PP, Index, TB);
 // Layout info only shown when hovering on the field/class itself.
-if (Decls.front() == N->ASTNode.get())
-  addLayoutInfo(*Decls.front(), *HI);
+if (DeclToUse == N->ASTNode.get())
+  addLayoutInfo(*DeclToUse, *HI);
 // Look for a close enclosing expression to show the value of.
 if (!HI->Value)
   HI->Value = printExprValue(N, AST.getASTContext());

diff  --git a/clang-tools-extra/clangd/unittests/HoverTests.cpp 
b/clang-tools-extra/clangd/unittests/HoverTests.cpp
index d4000f31a5f7..d2ad2c69c4e7 100644
--- a/clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ b/clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -1630,6 +1630,38 @@ TEST(Hover, All) {
 HI.Type = "int";
 HI.Definition = "int foo";
   }},
+  {
+  R"cpp(// Function definition via using declaration
+namespace ns { 
+  void foo(); 
+}
+int main() {
+  using ns::foo;
+  ^[[foo]]();
+}
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Function;
+HI.NamespaceScope = "ns::";
+HI.Type = "void ()";
+HI.Definition = "void foo()";
+HI.Documentation = "";
+HI.ReturnType = "void";
+HI.Parameters = std::vector{};
+  }},
+  {
+  R"cpp( // using declaration and two possible function declarations
+namespace ns { void foo(int); void foo(char); }
+using ns::foo;
+template  void bar() { [[f^oo]](T{}); }
+  )cpp",
+  [](HoverInfo &HI) {
+HI.Name = "foo";
+HI.Kind = index::SymbolKind::Using;
+HI.NamespaceScope = "";
+HI.Definition = "using ns::foo";
+  }},
   {
   R"cpp(// Macro
 #define MACRO 0
@@ -1734,6 +1766,25 @@ TEST(Hover, All) {
 HI.Definition = "ONE";
 HI.Value = "0";
   }},
+  {
+  R"cpp(// C++20's using enum
+enum class Hello {
+

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

[Tom Praschan via Phabricator via cfe-commits]

tom-anders added a comment.

Hmm, Github noticed that I referenced the issue with this commit, but didn't 
close it. 
According to https://github.blog/2013-03-18-closing-issues-across-repositories/ 
closing issues across repos should work, but only if you have push permissions 
in the repo that has the issue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133664

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

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:5779
 QualType ParamTy = Proto->getParamType(ArgIdx);
+if (ParamTy->containsErrors())
+  continue;

ArcsinX wrote:
> hokein wrote:
> > It looks like for the failure case the `ParamTy` for the parameter is a 
> > dependent array type, and it violates the "non-dependent" assumption of 
> > `clang::ASTContext::getTypeInfoImpl` which is called by  
> > `getTypeAlignInChars` in `CheckArgAlignment`.
> > 
> > so I'd suggest moving the fix to `CheckArgAlignment` line 5685 (adding a 
> > `ParamTy->isDependentType()` to the `if` condition).
> When I found this problem I fixed it like you are suggesting. But after that 
> I replaced it with this check, because it seems there is no reason to try to 
> check something on code with errors.
> 
> I mean that even if we are not crashing, results from `CheckArgAlignment()` 
> can't be trusted if we are passing  something with errors to it.
> because it seems there is no reason to try to check something on code with 
> errors.

I think this is case-by-case (for this case, it may be true) -- in some cases 
(see test7 and test8 for example in `recovery-expr-type.cpp`), we do want to 
check something even on the code with errors to emit useful secondary 
diagnostics. In general we want to emit a full list of diagnostics and at the 
same time avoid any suspicious diagnostics, however achieving both goals is 
hard.

Depending on how fatal the error is

- if the error is fatal, RecoveryError is just a dependent-type wrapper, we 
should not call check* to emit diagnostics (we're less certain about the 
quality of these diagnostics), this is mostly done by leveraging on the 
existing template dependent mechanism;
- if the error is minor, RecoveryError preserves a concrete type, we might want 
call check* to emit diagnostics as we're more confident; 

The current solution makes sense to fix the crash, but I think the main reason 
is that `CheckArgAlignment` now can be invoked with a dependent-type parameter 
in non-template context (after we extended the "dependent" concept from 
depending on template parameters to depending on template parameters and 
errors), so we should fix `CheckArgAlignment`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133886

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

[PATCH] D133664: [clangd] Fix hover on symbol introduced by using declaration

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D133664#3791694 , @tom-anders 
wrote:

> Hmm, Github noticed that I referenced the issue with this commit, but didn't 
> close it. 
> According to 
> https://github.blog/2013-03-18-closing-issues-across-repositories/ closing 
> issues across repos should work, but only if you have push permissions in the 
> repo that has the issue

thanks for the heads up, closed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133664

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

[PATCH] D133924: add clang_CXXMethod_isDeleted function

[Anders Langlands via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133924

Files:
  clang/bindings/python/clang/cindex.py
  clang/include/clang-c/Index.h
  clang/tools/c-index-test/c-index-test.c
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/libclang.map


Index: clang/tools/libclang/libclang.map
===
--- clang/tools/libclang/libclang.map
+++ clang/tools/libclang/libclang.map
@@ -409,6 +409,7 @@
   global:
 clang_getUnqualifiedType;
 clang_getNonReferenceType;
+clang_CXXMethod_isDeleted;
 };
 
 # Example of how to add a new symbol version entry.  If you do add a new symbol
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -8861,6 +8861,16 @@
   return (Method && Method->isDefaulted()) ? 1 : 0;
 }
 
+unsigned clang_CXXMethod_isDeleted(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXMethodDecl *Method =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Method && Method->isDeleted()) ? 1 : 0;
+}
+
 unsigned clang_CXXMethod_isStatic(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return 0;
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -900,6 +900,8 @@
   printf(" (mutable)");
 if (clang_CXXMethod_isDefaulted(Cursor))
   printf(" (defaulted)");
+if (clang_CXXMethod_isDeleted(Cursor))
+  printf(" (deleted)");
 if (clang_CXXMethod_isStatic(Cursor))
   printf(" (static)");
 if (clang_CXXMethod_isVirtual(Cursor))
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -4924,6 +4924,11 @@
  */
 CINDEX_LINKAGE unsigned clang_CXXMethod_isDefaulted(CXCursor C);
 
+/**
+ * Determine if a C++ method is declared '= default'.
+ */
+CINDEX_LINKAGE unsigned clang_CXXMethod_isDeleted(CXCursor C);
+
 /**
  * Determine if a C++ member function or member function template is
  * pure virtual.
Index: clang/bindings/python/clang/cindex.py
===
--- clang/bindings/python/clang/cindex.py
+++ clang/bindings/python/clang/cindex.py
@@ -1473,6 +1473,12 @@
 """
 return conf.lib.clang_CXXMethod_isDefaulted(self)
 
+def is_deleted_method(self):
+"""Returns True if the cursor refers to a C++ member function or member
+function template that is declared '= delete'.
+"""
+return conf.lib.clang_CXXMethod_isDeleted(self)
+
 def is_mutable_field(self):
 """Returns True if the cursor refers to a C++ field that is declared
 'mutable'.
@@ -3426,6 +3432,10 @@
[Cursor],
bool),
 
+  ("clang_CXXMethod_isDeleted",
+   [Cursor],
+   bool),
+
   ("clang_CXXMethod_isPureVirtual",
[Cursor],
bool),


Index: clang/tools/libclang/libclang.map
===
--- clang/tools/libclang/libclang.map
+++ clang/tools/libclang/libclang.map
@@ -409,6 +409,7 @@
   global:
 clang_getUnqualifiedType;
 clang_getNonReferenceType;
+clang_CXXMethod_isDeleted;
 };
 
 # Example of how to add a new symbol version entry.  If you do add a new symbol
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -8861,6 +8861,16 @@
   return (Method && Method->isDefaulted()) ? 1 : 0;
 }
 
+unsigned clang_CXXMethod_isDeleted(CXCursor C) {
+  if (!clang_isDeclaration(C.kind))
+return 0;
+
+  const Decl *D = cxcursor::getCursorDecl(C);
+  const CXXMethodDecl *Method =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Method && Method->isDeleted()) ? 1 : 0;
+}
+
 unsigned clang_CXXMethod_isStatic(CXCursor C) {
   if (!clang_isDeclaration(C.kind))
 return 0;
Index: clang/tools/c-index-test/c-index-test.c
===
--- clang/tools/c-index-test/c-index-test.c
+++ clang/tools/c-index-test/c-index-test.c
@@ -900,6 +900,8 @@
   printf(" (mutable)");
 if (clang_CXXMethod_isDefaulted(Cursor))
   printf(" (defaulted)");
+if (clang_CXXMethod_isDeleted(Cursor))
+  printf(" (deleted)");
 if (clang_CXXMethod_isStatic(Cursor))
   printf(" (static)");
 if (clang_CXXMethod_isVirtual(Cursor))
Index: clang/include

[PATCH] D128462: [HLSL] add -I option for dxc mode.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang/include/clang/Driver/Options.td:6362
 
-def cl_Group : OptionGroup<"">, Flags<[CLOption]>,
+def cl_Group : OptionGroup<"">, Flags<[CLDXCOption]>,
   HelpText<"CL.EXE COMPATIBILITY OPTIONS">;

i am failing to understand the semantics of this new option group, and it's 
causing regressions in clangd. see https://github.com/clangd/clangd/issues/1292.

is this new group suppose to serve flags that are available both in 
`--driver-mode=cl` and `--driver-mode=dxc`? i.e. they are for the flags in the 
intersection?
are there any flags that are applicable to CL driver mode but not to the DXC 
mode (and vice versa)?

why didn't we go for just adding DXCOption mode into here, if every option in 
CL mode is actually applicable to DXC option as well, rather than introducing a 
new option?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128462

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

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

[Gabor Marton via Phabricator via cfe-commits]

martong marked 7 inline comments as done.
martong added inline comments.
Herald added a project: All.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:156-157
 // the bug is reported.
-virtual std::string describe(ProgramStateRef State,
+virtual std::string describe(DescritptionKind DK, ProgramStateRef State,
  const Summary &Summary) const {
   // There are some descendant classes that are not used as argument

Szelethus wrote:
> How about we turn this into a print-like function and instead of returning 
> with a string, we take an `llvm::raw_ostream` object as argument? 
> `SmallString` + `raw_svector_stream` is how we construct most of our checker 
> message strings.
I don't see how that change would be relevant. Would we have a better run-time, 
or code that is easier to understand? please elaborate.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:854
+  NewState, NewNode,
+  C.getNoteTag([Msg, ArgSVal](PathSensitiveBugReport &BR,
+  llvm::raw_ostream &OS) {

steakhal wrote:
> 
Thanks, changed it.



Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:857
+if (BR.isInteresting(ArgSVal))
+  OS << Msg;
+  }));

NoQ wrote:
> Szelethus wrote:
> > steakhal wrote:
> > > Ah, there is a slight issue.
> > > You should mark some stuff interesting here, to make this interestingness 
> > > propagate back transitively.
> > > 
> > > Let's say `ArgSVal` is `x + y` which is considered to be out of range 
> > > `[42,52]`. We should mark both `x` and `y` interesting because they 
> > > themselves could have been constrained by the StdLibChecker previously. 
> > > So, they must be interesting as well.
> > > 
> > > On the same token, IMO `PathSensitiveBugReport::markInteresting(symbol)` 
> > > should be //transitive//. So that all `SymbolData` in that symbolic 
> > > expression tree are considered interesting. What do you think @NoQ?
> > > If we were doing this, @martong  - you could simply acquire the 
> > > assumption you constructed for the given `ValueConstraint`, and mark that 
> > > interesting. Then all `SymbolData`s on both sides of the logic operator 
> > > would become implicitly interesting.
> > >On the same token, IMO PathSensitiveBugReport::markInteresting(symbol) 
> > >should be transitive. So that all SymbolData in that symbolic expression 
> > >tree are considered interesting. What do you think @NoQ?
> > 
> > Thats how I'd expect this to work. This shouldn't be a burden on the 
> > checker developer (certainly not this kind of a checker), but rather be 
> > handled by `PathSensitiveBugReport`.
> > 
> > So I think this is fine as it is.
> Interestingness isn't a thing on its own; its meaning is entirely tied to the 
> nature of the specific bug report. An interesting pointer in the null 
> dereference checker is absolutely not the same thing as an interesting mutex 
> pointer in `PthreadLockChecker` or a (de)allocated pointer in `MallocChecker`.
> 
> I currently treat interestingness as a "GDM for visitors". It's their own way 
> of communicating with themselves and with each other, a state they keep track 
> of and update as they visit the bug report (initially populated during 
> construction of the bug report). But the meaning of this state is entirely 
> specific to the visitors. It is visitors who give interestingness a meaning 
> (and the visitors are, naturally, also hand-picked during construction of the 
> bug report).
> 
> So I think the right question to ask is "what do you want interestingness to 
> mean in your checker?" and build your visitors accordingly.
> 
> Your visitors should provide enough information for the user to be able to 
> understand the bug report. When the report says "$x + $y is in range [42, 52] 
> and no values in that range are a valid input to that function", the user 
> asks "why do you think $x + $y is in range [42, 52]?" and we'll have to 
> answer that.
> 
> For example, in
> ```lang=c++
> if (x + y >= 42 && x + y <= 52)
>   foo(x + y);
> ```
> there's no need to track ranges for $x and $y separately; it is sufficient to 
> point the user to the constraint over $x + $y obtained from the if-statement. 
> On the other hand, in
> ```lang=c++
> if (x >= 44 && x <= 50)
>   if (y >= -2 && y <= 2)
> foo(x + y);
> ```
> you'll have to explain both $x and $y in order for the user to understand 
> that $x + $y is indeed in range [42, 52].
> 
> There are also other funny edge cases depending on the nature of the 
> arithmetic, such as
> ```
> int z = x * y;
> if (x == 0)
>   return 1 / z;
> ```
> where in order to explain the division by the zero value $x * $y it is 
> sufficient to explain $x which makes $y redundant. And if they both are zero 
> then we should p

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 460345.
martong marked 2 inline comments as done.
martong added a comment.

- Rebase
- move Msg into the lambda


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101526

Files:
  clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
  clang/test/Analysis/std-c-library-functions-arg-constraints.c

Index: clang/test/Analysis/std-c-library-functions-arg-constraints.c
===
--- clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -239,6 +239,7 @@
   // State split should not happen here. I.e. x == 1 should not be evaluated
   // FALSE.
   __two_constrained_args(x, y);
+  //NOTE! Because of the second `clang_analyzer_eval` call we have two bug
   clang_analyzer_eval(x == 1); // \
   // report-warning{{TRUE}} \
   // bugpath-warning{{TRUE}} \
@@ -252,7 +253,6 @@
 int __arg_constrained_twice(int);
 void test_multiple_constraints_on_same_arg(int x) {
   __arg_constrained_twice(x);
-  // Check that both constraints are applied and only one branch is there.
   clang_analyzer_eval(x < 1 || x > 2); // \
   // report-warning{{TRUE}} \
   // bugpath-warning{{TRUE}} \
Index: clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
===
--- /dev/null
+++ clang/test/Analysis/std-c-library-functions-arg-constraints-note-tags.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions \
+// RUN:   -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
+// RUN:   -analyzer-checker=debug.StdCLibraryFunctionsTester \
+// RUN:   -analyzer-config apiModeling.StdCLibraryFunctions:DisplayLoadedSummaries=true \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -triple i686-unknown-linux \
+// RUN:   -analyzer-output=text \
+// RUN:   -verify
+
+int __single_val_0(int);  // [0, 0]
+
+int test_note(int x, int y) {
+__single_val_0(x);  // expected-note{{Assuming the 1st arg is within the range [0, 0]}}
+return y / x;   // expected-warning{{Division by zero}} \
+// expected-note{{Division by zero}}
+}
Index: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp
@@ -134,9 +134,18 @@
 
 virtual StringRef getName() const = 0;
 
+// Represents that in which context do we require a description of the
+// constraint.
+enum class DescritptionKind {
+  // The constraint is violated.
+  Violation,
+  // We assume that the constraint is satisfied.
+  Assumption
+};
+
 // Give a description that explains the constraint to the user. Used when
 // the bug is reported.
-virtual std::string describe(ProgramStateRef State,
+virtual std::string describe(DescritptionKind DK, ProgramStateRef State,
  const Summary &Summary) const {
   // There are some descendant classes that are not used as argument
   // constraints, e.g. ComparisonConstraint. In that case we can safely
@@ -174,7 +183,7 @@
 RangeConstraint(ArgNo ArgN, RangeKind Kind, const IntRangeVector &Ranges)
 : ValueConstraint(ArgN), Kind(Kind), Ranges(Ranges) {}
 
-std::string describe(ProgramStateRef State,
+std::string describe(DescritptionKind DK, ProgramStateRef State,
  const Summary &Summary) const override;
 
 const IntRangeVector &getRanges() const { return Ranges; }
@@ -244,7 +253,7 @@
 bool CannotBeNull = true;
 
   public:
-std::string describe(ProgramStateRef State,
+std::string describe(DescritptionKind DK, ProgramStateRef State,
  const Summary &Summary) const override;
 StringRef getName() const override { return "NonNull"; }
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
@@ -316,7 +325,7 @@
   return Result;
 }
 
-std::string describe(ProgramStateRef State,
+std::string describe(DescritptionKind DK, ProgramStateRef State,
  const Summary &Summary) const override;
 
 ProgramStateRef apply(ProgramStateRef State, const CallEvent &Call,
@@ -704,9 +713,12 @@
 // Highlight the range of the argument that was violated.
 R->addRange(Call.getArgSourceRange(VC->getArgNo()));
 
-// Describe the argument constraint in a note.
-R->addNote(VC->describe(C.getState(), Summary), R->getLocation(),
-   

[PATCH] D101526: [analyzer][StdLibraryFunctionsChecker] Add NoteTags for applied arg constraints

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Gentle ping @steakhal @NoQ 
Trying to revive this after a year :) I am sorry it took so long to get back to 
this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101526

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

[PATCH] D133757: [clangd] Turn QueryDriverDatabase into a CompileCommandsAdjuster

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D133757#3791485 , @nridge wrote:

> I think the sticking point for just having `QueryDriverDatabase` run after 
> the entirety of `CommandMangler` is this check 
> 
>  in `resolveDriver()`, which replaces e.g. `gcc` with 
> `/path/to/clang/bin/gcc`, which likely does not actually exist (i.e. 
> `QueryDriverDatabase` will not be able to query it, when it might have been 
> able to look up `gcc` in `PATH`).
>
> Do you know what the purpose of this logic in `resolveDriver()` is, and in 
> particular:
>
> - why is `gcc` treated differently than say `gcc-12`?
> - why is turning `gcc` into `/path/to/clang/bin/gcc` performed **before** 
> trying to resolve `gcc` against `PATH`?

We thought generic driver names would actually imply "not caring" about the 
compiler, hence it would be better to just use installation that came with 
clang, if we were able to detect any. this extremely helpful on macs, i think 
without this we won't have a working setup by default. since the clang coming 
from $PATH doesn't have stdlib installed, whereas clang we discover through 
xcode has it.

> If we can change the order of the checks in `resolveDriver()` such that we 
> try to resolve `gcc` against `PATH` first, and only turn it into 
> `/path/to/clang/bin/gcc` if it was not found in `PATH`, I think that would 
> not interfere with `QueryDriverDatabase`.

Right, that would be one way to go. But as explained above, I think this is 
actually making most of the "don't care" cases work by default and it wouldn't 
be great to give them up, without understanding the tradeoff.
Hence I was trying to understand what exactly are we supporting by not doing 
this resolution eagerly. i.e.:

- what are the cases in which we have a generic driver name in the compile 
commands, and user indeed has all the necessary system headers installed 
relative to resolved path of that generic compiler name in `$PATH`
- do we lose anything by not resolving a driver name provided by 
CompileFlags.Compiler.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757

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

[PATCH] D133924: add clang_CXXMethod_isDeleted function

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

tbaeder added inline comments.



Comment at: clang/tools/libclang/CIndex.cpp:8870
+  const CXXMethodDecl *Method =
+  D ? dyn_cast_or_null(D->getAsFunction()) : nullptr;
+  return (Method && Method->isDeleted()) ? 1 : 0;




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133924

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

[PATCH] D133930: [clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`.

[weiyi via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133930

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

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -14,10 +14,8 @@
 #include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Tooling.h"
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -1248,13 +1246,9 @@
  Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
 ASTContext &Ctx, const CFGElement &Elt,
 const TypeErasedDataflowAnalysisState &State) mutable {
-  auto Stmt = Elt.getAs();
-  if (!Stmt) {
-return;
-  }
-  auto StmtDiagnostics =
-  Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env);
-  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
+  auto EltDiagnostics =
+  Diagnoser.diagnose(Ctx, &Elt, State.Env);
+  llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
 })
 .withASTBuildArgs(
 {"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"})
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -18,8 +18,9 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
-#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/NoopLattice.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/SourceLocation.h"
@@ -559,41 +560,42 @@
   // lot of duplicated work (e.g. string comparisons), consider providing APIs
   // that avoid it through memoization.
   auto IgnorableOptional = ignorableOptional(Options);
-  return MatchSwitchBuilder()
+  return CFGMatchSwitchBuilder()
   // Attach a symbolic "has_value" state to optional values that we see for
   // the first time.
-  .CaseOf(
+  .CaseOfCFGStmt(
   expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()),
   initializeOptionalReference)
 
   // make_optional
-  .CaseOf(isMakeOptionalCall(), transferMakeOptionalCall)
+  .CaseOfCFGStmt(isMakeOptionalCall(), transferMakeOptionalCall)
 
   // optional::optional
-  .CaseOf(
+  .CaseOfCFGStmt(
   isOptionalInPlaceConstructor(),
   [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,
  LatticeTransferState &State) {
 assignOptionalValue(*E, State, State.Env.getBoolLiteralValue(true));
   })
-  .CaseOf(
+  .CaseOfCFGStmt(
   isOptionalNulloptConstructor(),
   [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,
  LatticeTransferState &State) {
 assignOptionalValue(*E, State,
 State.Env.getBoolLiteralValue(false));
   })
-  .CaseOf(isOptionalValueOrConversionConstructor(),
-transferValueOrConversionConstructor)
+  .CaseOfCFGStmt(isOptionalValueOrConversionConstructor(),
+   transferValueOrConversionConstructor)
 
   // optional::operator=
-  .CaseOf(isOptionalValueOrConversionAssignment(),
-   transferValueOrConversionAssignment)
-  .CaseOf(isOptionalNulloptAssignment(),
-   transferNulloptAssignment)
+  .CaseOfCFGStmt(
+  isOptionalValueOrConversionAssignment(),
+  transferValueOrConversionAssignment)
+  .CaseOfCFGStmt(isOptionalNulloptAssignment(),
+

[PATCH] D133931: [clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `clang/Analysis/FlowSensitive`.

[weiyi via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133931

Files:
  clang/include/clang/Analysis/FlowSensitive/NoopAnalysis.h
  clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
  clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -111,7 +111,7 @@
 
   static NonConvergingLattice initialElement() { return {0}; }
 
-  void transfer(const Stmt *S, NonConvergingLattice &E, Environment &Env) {
+  void transfer(const CFGElement *, NonConvergingLattice &E, Environment &) {
 ++E.State;
   }
 };
@@ -162,7 +162,11 @@
 
   static FunctionCallLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, FunctionCallLattice &E, Environment &Env) {
+  void transfer(const CFGElement *Elt, FunctionCallLattice &E, Environment &) {
+auto CS = Elt->getAs();
+if (!CS)
+  return;
+auto S = CS->getStmt();
 if (auto *C = dyn_cast(S)) {
   if (auto *F = dyn_cast(C->getCalleeDecl())) {
 E.CalledFunctions.insert(F->getNameInfo().getAsString());
@@ -314,7 +318,11 @@
 
   static NoopLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+  void transfer(const CFGElement *Elt, NoopLattice &, Environment &Env) {
+auto CS = Elt->getAs();
+if (!CS)
+  return;
+auto S = CS->getStmt();
 auto SpecialBoolRecordDecl = recordDecl(hasName("SpecialBool"));
 auto HasSpecialBoolType = hasType(SpecialBoolRecordDecl);
 
@@ -466,7 +474,11 @@
 
   static NoopLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+  void transfer(const CFGElement *Elt, NoopLattice &, Environment &Env) {
+auto CS = Elt->getAs();
+if (!CS)
+  return;
+auto S = CS->getStmt();
 auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
 auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
 
Index: clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp
@@ -19,16 +19,15 @@
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
-#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Testing/Support/Annotations.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -122,8 +121,12 @@
 return ConstantPropagationLattice::bottom();
   }
 
-  void transfer(const Stmt *S, ConstantPropagationLattice &Element,
+  void transfer(const CFGElement *E, ConstantPropagationLattice &Element,
 Environment &Env) {
+auto CS = E->getAs();
+if (!CS)
+  return;
+auto S = CS->getStmt();
 auto matcher = stmt(
 anyOf(declStmt(hasSingleDecl(varDecl(hasType(isInteger()),
  hasInitializer(expr().bind(kInit)))
Index: clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp
@@ -19,17 +19,16 @@
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "clang/Analysis/FlowSensitive/MapLattice.h"
-#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #includ

[PATCH] D133933: [clang][dataflow] Modify `transfer` in `DataflowModel` to take `CFGElement` as input instead of `Stmt`.

[weiyi via Phabricator via cfe-commits]

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

To keep API of transfer functions consistent.

The single use of this transfer function in `ChromiumCheckModel` is also 
updated.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133933

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
  clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
  clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
@@ -120,9 +120,7 @@
   static NoopLattice initialElement() { return NoopLattice(); }
 
   void transfer(const CFGElement *E, NoopLattice &, Environment &Env) {
-if (auto S = E->getAs()) {
-  M.transfer(S->getStmt(), Env);
-}
+M.transfer(E, Env);
   }
 
 private:
Index: clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
@@ -50,7 +50,11 @@
   return CheckDecls.contains(&D);
 }
 
-bool ChromiumCheckModel::transfer(const Stmt *Stmt, Environment &Env) {
+bool ChromiumCheckModel::transfer(const CFGElement *Elt, Environment &Env) {
+  auto CS = Elt->getAs();
+  if (!CS)
+return false;
+  auto Stmt = CS->getStmt();
   if (const auto *Call = dyn_cast(Stmt)) {
 if (const auto *M = dyn_cast(Call->getDirectCallee())) {
   if (isCheckLikeMethod(CheckDecls, *M)) {
Index: clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
===
--- clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
+++ clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
@@ -26,7 +26,7 @@
 class ChromiumCheckModel : public DataflowModel {
 public:
   ChromiumCheckModel() = default;
-  bool transfer(const Stmt *Stmt, Environment &Env) override;
+  bool transfer(const CFGElement *Elt, Environment &Env) override;
 
 private:
   /// Declarations for `::logging::CheckError::.*Check`, lazily initialized.
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -231,8 +231,8 @@
 /// example, a model may capture a type and its related functions.
 class DataflowModel : public Environment::ValueModel {
 public:
-  /// Return value indicates whether the model processed the `Stmt`.
-  virtual bool transfer(const Stmt *Stmt, Environment &Env) = 0;
+  /// Return value indicates whether the model processed the `Element`.
+  virtual bool transfer(const CFGElement *Elt, Environment &Env) = 0;
 };
 
 } // namespace dataflow


Index: clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
@@ -120,9 +120,7 @@
   static NoopLattice initialElement() { return NoopLattice(); }
 
   void transfer(const CFGElement *E, NoopLattice &, Environment &Env) {
-if (auto S = E->getAs()) {
-  M.transfer(S->getStmt(), Env);
-}
+M.transfer(E, Env);
   }
 
 private:
Index: clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
@@ -50,7 +50,11 @@
   return CheckDecls.contains(&D);
 }
 
-bool ChromiumCheckModel::transfer(const Stmt *Stmt, Environment &Env) {
+bool ChromiumCheckModel::transfer(const CFGElement *Elt, Environment &Env) {
+  auto CS = Elt->getAs();
+  if (!CS)
+return false;
+  auto Stmt = CS->getStmt();
   if (const auto *Call = dyn_cast(Stmt)) {
 if (const auto *M = dyn_cast(Call->getDirectCallee())) {
   if (isCheckLikeMethod(CheckDecls, *M)) {
Index: clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
===
--- clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
+++ clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
@@ -26,7 +26,7 @@
 class ChromiumCheckModel

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

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

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

Handle sizeof expressions.

I was a bit unsure here about alignment, but the rest is pretty 
self-explanatory I think.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133934

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

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -72,10 +72,30 @@
 constexpr const int* getIntPointer() {
   return &m;
 }
-//static_assert(getIntPointer() == &m, ""); TODO
-//static_assert(*getIntPointer() == 10, ""); TODO
+static_assert(getIntPointer() == &m, "");
+static_assert(*getIntPointer() == 10, "");
 
 constexpr int gimme(int k) {
   return k;
 }
-// static_assert(gimme(5) == 5, ""); TODO
+static_assert(gimme(5) == 5, "");
+
+namespace SizeOf {
+  constexpr int soint = sizeof(int);
+  constexpr int souint = sizeof(unsigned int);
+  static_assert(soint == souint, "");
+
+  static_assert(sizeof(&soint) == sizeof(void*), "");
+  static_assert(sizeof(&soint) == sizeof(nullptr), "");
+
+  static_assert(sizeof(long) == sizeof(unsigned long), "");
+  static_assert(sizeof(char) == sizeof(unsigned char), "");
+
+  constexpr int N = 4;
+  constexpr int arr[N] = {1,2,3,4};
+  static_assert(sizeof(arr) == N * sizeof(int), "");
+  static_assert(sizeof(arr) == N * sizeof(arr[0]), "");
+
+  constexpr bool arrB[N] = {true, true, true, true};
+  static_assert(sizeof(arrB) == N * sizeof(bool), "");
+};
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -427,11 +427,11 @@
 // TODO: Expand this to handle casts between more types.
 
 def FromCastTypeClass : TypeClass {
-  let Types = [Uint32, Sint32, Bool];
+  let Types = [Uint8, Sint8, Uint16, Sint16, Uint32, Sint32, Uint64, Sint64, Bool];
 }
 
 def ToCastTypeClass : TypeClass {
-  let Types = [Uint32, Sint32, Bool];
+  let Types = [Uint8, Sint8, Uint16, Sint16, Uint32, Sint32, Uint64, Sint64, Bool];
 }
 
 def Cast: Opcode {
Index: clang/lib/AST/Interp/Context.h
===
--- clang/lib/AST/Interp/Context.h
+++ clang/lib/AST/Interp/Context.h
@@ -62,6 +62,9 @@
   /// Classifies an expression.
   llvm::Optional classify(QualType T) const;
 
+  /// Calculate size of \T.
+  llvm::Optional sizeofType(QualType T) const;
+
 private:
   /// Runs a function.
   bool Run(State &Parent, Function *Func, APValue &Result);
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -130,6 +130,25 @@
   return {};
 }
 
+llvm::Optional Context::sizeofType(QualType T) const {
+  if (llvm::Optional ArgT = classify(T))
+return primSize(*ArgT);
+
+  if (const auto *AT = T->getAsArrayTypeUnsafe()) {
+if (const auto *CAT = dyn_cast(T)) {
+  size_t NumElems = CAT->getSize().getZExtValue();
+  QualType ElemType = CAT->getElementType();
+
+  if (llvm::Optional ElemSize = sizeofType(ElemType))
+return NumElems * (*ElemSize);
+}
+  }
+
+  // TODO: Handle record types
+
+  return {};
+}
+
 unsigned Context::getCharBit() const {
   return Ctx.getTargetInfo().getCharWidth();
 }
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -80,6 +80,7 @@
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
   bool VisitConstantExpr(const ConstantExpr *E);
+  bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -281,7 +281,20 @@
 }
 
 template 
-bool ByteCodeExprGen::discard(const Expr *E) {
+bool ByteCodeExprGen::VisitUnaryExprOrTypeTraitExpr(
+const UnaryExprOrTypeTraitExpr *E) {
+
+  if (E->getKind() == UETT_SizeOf) {
+QualType ArgType = E->getTypeOfArgument();
+
+if (Optional ArgSize = Ctx.sizeofType(ArgType))
+  return this->emitConst(E, *ArgSize);
+  }
+
+  return false;
+}
+
+template  bool ByteCodeExprGen::discard(const Expr *E) 

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

[Aleksandr Platonov via Phabricator via cfe-commits]

ArcsinX updated this revision to Diff 460358.
ArcsinX added a comment.

- Check for dependent type inside CheckArgAlignment()
- Simplify test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133886

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -162,3 +162,12 @@
   b = __builtin_object_size(c, 0); // crash2
 }
 }
+
+namespace test15 {
+void f() {
+  struct {
+void m(int (&)[undefined()]) {} // expected-error {{undeclared identifier}}
+  } S;
+  S.m(1); // no crash
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -5690,9 +5690,9 @@
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ArgTy.isNull() || ParamTy->isIncompleteType() ||
-  ArgTy->isIncompleteType() || ParamTy->isUndeducedType() ||
-  ArgTy->isUndeducedType())
+  if (ArgTy.isNull() || ParamTy->isDependentType() ||
+  ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -162,3 +162,12 @@
   b = __builtin_object_size(c, 0); // crash2
 }
 }
+
+namespace test15 {
+void f() {
+  struct {
+void m(int (&)[undefined()]) {} // expected-error {{undeclared identifier}}
+  } S;
+  S.m(1); // no crash
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -5690,9 +5690,9 @@
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ArgTy.isNull() || ParamTy->isIncompleteType() ||
-  ArgTy->isIncompleteType() || ParamTy->isUndeducedType() ||
-  ArgTy->isUndeducedType())
+  if (ArgTy.isNull() || ParamTy->isDependentType() ||
+  ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133935: [clang][dataflow] Refactor `clang/Analysis/FlowSensitive/MatchSwitchTest.cpp`.

[weiyi via Phabricator via cfe-commits]

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

- Remove use of `runDataflowAnalysis` to keep test isolated.
- Add test for `ASTMatchSwitch`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133935

Files:
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
@@ -7,26 +7,15 @@
 //===--===//
 
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
-#include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
-#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
-#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
-#include "clang/Analysis/FlowSensitive/MapLattice.h"
 #include "clang/Tooling/Tooling.h"
-#include "llvm/ADT/None.h"
-#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/Twine.h"
-#include "llvm/Support/Error.h"
-#include "llvm/Testing/Support/Annotations.h"
-#include "llvm/Testing/Support/Error.h"
-#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 #include 
@@ -38,162 +27,112 @@
 using namespace dataflow;
 
 namespace {
-using ::testing::Pair;
-using ::testing::UnorderedElementsAre;
 
-class BooleanLattice {
-public:
-  BooleanLattice() : Value(false) {}
-  explicit BooleanLattice(bool B) : Value(B) {}
-
-  static BooleanLattice bottom() { return BooleanLattice(false); }
-
-  static BooleanLattice top() { return BooleanLattice(true); }
-
-  LatticeJoinEffect join(BooleanLattice Other) {
-auto Prev = Value;
-Value = Value || Other.Value;
-return Prev == Value ? LatticeJoinEffect::Unchanged
- : LatticeJoinEffect::Changed;
-  }
-
-  friend bool operator==(BooleanLattice LHS, BooleanLattice RHS) {
-return LHS.Value == RHS.Value;
-  }
-
-  friend std::ostream &operator<<(std::ostream &Os, const BooleanLattice &B) {
-Os << B.Value;
-return Os;
-  }
-
-  bool value() const { return Value; }
-
-private:
-  bool Value;
-};
-} // namespace
-
-MATCHER_P(Holds, m,
-  ((negation ? "doesn't hold" : "holds") +
-   llvm::StringRef(" a lattice element that ") +
-   ::testing::DescribeMatcher(m, negation))
-  .str()) {
-  return ExplainMatchResult(m, arg.Lattice, result_listener);
-}
-
-void TransferSetTrue(const DeclRefExpr *,
- const ast_matchers::MatchFinder::MatchResult &,
- TransferState &State) {
-  State.Lattice = BooleanLattice(true);
-}
-
-void TransferSetFalse(const Stmt *,
-  const ast_matchers::MatchFinder::MatchResult &,
-  TransferState &State) {
-  State.Lattice = BooleanLattice(false);
-}
-
-class TestAnalysis : public DataflowAnalysis {
-  MatchSwitch> TransferSwitch;
-
-public:
-  explicit TestAnalysis(ASTContext &Context)
-  : DataflowAnalysis(Context) {
-using namespace ast_matchers;
-TransferSwitch =
-MatchSwitchBuilder>()
-.CaseOf(declRefExpr(to(varDecl(hasName("X",
- TransferSetTrue)
-.CaseOf(callExpr(callee(functionDecl(hasName("Foo",
-  TransferSetFalse)
-.Build();
-  }
-
-  static BooleanLattice initialElement() { return BooleanLattice::bottom(); }
-
-  void transfer(const Stmt *S, BooleanLattice &L, Environment &Env) {
-TransferState State(L, Env);
-TransferSwitch(*S, getASTContext(), State);
-  }
-};
-
-template 
-void RunDataflow(llvm::StringRef Code, Matcher Expectations) {
-  ASSERT_THAT_ERROR(
-  test::checkDataflow(
-  Code, "fun",
-  [](ASTContext &C, Environment &) { return TestAnalysis(C); },
-  [&Expectations](
-  llvm::ArrayRef>>
-  Results,
-  ASTContext &) { EXPECT_THAT(Results, Expectations); },
-  {"-fsyntax-only", "-std=c++17"}),
-  llvm::Succeeded());
-}
-
-TEST(MatchSwitchTest, JustX) {
-  std::string Code = R"(
-void fun() {
-  int X = 1;
-  (void)X;
-  // [[p]]
-}
-  )";
-  RunDataflow(Code,
-  UnorderedElementsAre(Pair("p", Holds(BooleanLattice(true);
-}
-
-TEST(MatchSwitchTest, JustFoo) {
-  std::string Code = R"(
-void Foo();
-void fun() {
-  Foo();
-  // [[p]]
-}
-  )";
-  RunDataflow(Code,
-  Unor

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

[Aleksandr Platonov via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaChecking.cpp:5779
 QualType ParamTy = Proto->getParamType(ArgIdx);
+if (ParamTy->containsErrors())
+  continue;

hokein wrote:
> ArcsinX wrote:
> > hokein wrote:
> > > It looks like for the failure case the `ParamTy` for the parameter is a 
> > > dependent array type, and it violates the "non-dependent" assumption of 
> > > `clang::ASTContext::getTypeInfoImpl` which is called by  
> > > `getTypeAlignInChars` in `CheckArgAlignment`.
> > > 
> > > so I'd suggest moving the fix to `CheckArgAlignment` line 5685 (adding a 
> > > `ParamTy->isDependentType()` to the `if` condition).
> > When I found this problem I fixed it like you are suggesting. But after 
> > that I replaced it with this check, because it seems there is no reason to 
> > try to check something on code with errors.
> > 
> > I mean that even if we are not crashing, results from `CheckArgAlignment()` 
> > can't be trusted if we are passing  something with errors to it.
> > because it seems there is no reason to try to check something on code with 
> > errors.
> 
> I think this is case-by-case (for this case, it may be true) -- in some cases 
> (see test7 and test8 for example in `recovery-expr-type.cpp`), we do want to 
> check something even on the code with errors to emit useful secondary 
> diagnostics. In general we want to emit a full list of diagnostics and at the 
> same time avoid any suspicious diagnostics, however achieving both goals is 
> hard.
> 
> Depending on how fatal the error is
> 
> - if the error is fatal, RecoveryError is just a dependent-type wrapper, we 
> should not call check* to emit diagnostics (we're less certain about the 
> quality of these diagnostics), this is mostly done by leveraging on the 
> existing template dependent mechanism;
> - if the error is minor, RecoveryError preserves a concrete type, we might 
> want call check* to emit diagnostics as we're more confident; 
> 
> The current solution makes sense to fix the crash, but I think the main 
> reason is that `CheckArgAlignment` now can be invoked with a dependent-type 
> parameter in non-template context (after we extended the "dependent" concept 
> from depending on template parameters to depending on template parameters and 
> errors), so we should fix `CheckArgAlignment`.
> 
Thanks for clarification. Moved check inside CheckArgAlignment() as you 
suggested.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133886

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

[PATCH] D133930: [clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`.

[Stanislav Gatev via Phabricator via cfe-commits]

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



Comment at: 
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h:75
+  std::vector
+  diagnose(ASTContext &Context, const CFGElement *Elt, const Environment &Env);
 

Why not `Ctx` like above?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133930

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

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type contains errors

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133886

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

[PATCH] D133933: [clang][dataflow] Modify `transfer` in `DataflowModel` to take `CFGElement` as input instead of `Stmt`.

[Stanislav Gatev via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:235
+  /// Return value indicates whether the model processed the `Element`.
+  virtual bool transfer(const CFGElement *Elt, Environment &Env) = 0;
 };

In `DataflowAnalysis` we use the name `Element`. Can we use the same here too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133933

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

[PATCH] D133935: [clang][dataflow] Refactor `clang/Analysis/FlowSensitive/MatchSwitchTest.cpp`.

[Stanislav Gatev via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp:139
 TEST(MatchSwitchTest, ReturnNonVoid) {
   using namespace ast_matchers;
 

Let's move this below line 27 and remove it from the other tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133935

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

[PATCH] D133800: [Clang 15.0.1] Downgrade implicit int and implicit function declaration to warning only

[Aaron Ballman via Phabricator via cfe-commits]

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

In D133800#3789377 , @aaron.ballman 
wrote:

> In D133800#3787378 , @thieta wrote:
>
>> I think the easiest way to handle this is that when you have it approved 
>> here - push it to fork on GitHub based on the release branch and create a 
>> GitHub issue and write /branch aballman/llvm-project/my_branch in a comment 
>> and it will queue up the cherry pick.
>
> I've filed https://github.com/llvm/llvm-project/issues/57739 and assigned it 
> to you @thieta -- please let me know if I've mucked something up!

And that was landed in 
https://github.com/llvm/llvm-project-release-prs/commit/c0141f3c300fbc002cf79404fa0b82b4cb1191df
 so this review can now be closed. Thank you for the quick turnaround on 
getting me review feedback!


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

https://reviews.llvm.org/D133800

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

[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 3 inline comments as done.
aaron.ballman added inline comments.



Comment at: clang/docs/ReleaseNotes.rst:52
+  into an error-only diagnostic in the next Clang release. Fixes
+  `Issue 50055: `_.
+- ``-Wincompatible-function-pointer-types`` now defaults to an error in all C

thesamesam wrote:
> Reflecting on this a bit, do wonder about explicitly calling out configure 
> scripts, as I suspect it's something a lot of people may not even think of.
> 
> "The LLVM team recommends that projects using configure scripts verify the 
> results do not change before/after setting 
> `-Werror=implicit-function-declarations` (repeat for others) to avoid 
> incompatibility with Clang 16."
Good idea!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133771

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

[clang] e076680 - Add a "Potentially Breaking Changes" section to the Clang release notes

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-09-15T07:29:49-04:00
New Revision: e076680bd59cec5e98fe05aeb91891a1e5745e5b

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

LOG: Add a "Potentially Breaking Changes" section to the Clang release notes

Sometimes we make changes to the compiler that we expect may cause
disruption for users. For example, we may strengthen a warning to
default to be an error, or fix an accepts-invalid bug that's been
around for a long time, etc which may cause previously accepted code to
now be rejected. Rather than hope users discover that information by
reading all of the release notes, it's better that we call these out in
one location at the top of the release notes.

Based on feedback collected in the discussion at:
https://discourse.llvm.org/t/configure-script-breakage-with-the-new-werror-implicit-function-declaration/65213/

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dcc2eaf4b202f..0680aa56db0b7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -36,6 +36,30 @@ main Clang web page, this document applies to the *next* 
release, not
 the current one. To see the release notes for a specific release, please
 see the `releases page `_.
 
+Potentially Breaking Changes
+
+These changes are ones which we think may surprise users when upgrading to
+Clang |release| because of the opportunity they pose for disruption to existing
+code bases.
+
+- Clang will now correctly diagnose as ill-formed a constant expression where 
an
+  enum without a fixed underlying type is set to a value outside the range of
+  the enumeration's values. Due to the extended period of time this bug was
+  present in major C++ implementations (including Clang), this error has the
+  ability to be downgraded into a warning (via:
+  ``-Wno-error=enum-constexpr-conversion``) to provide a transition period for
+  users. This diagnostic is expected to turn into an error-only diagnostic in
+  the next Clang release. Fixes
+  `Issue 50055: `_.
+- ``-Wincompatible-function-pointer-types`` now defaults to an error in all C
+  language modes. It may be downgraded to a warning with
+  ``-Wno-error=incompatible-function-pointer-types`` or disabled entirely with
+  ``-Wno-implicit-function-pointer-types``. *NOTE* We recommend that projects
+  using configure scripts verify the results do not change before/after setting
+  ``-Werror=incompatible-function-pointer-types`` to avoid incompatibility with
+  Clang 16.
+
+
 What's New in Clang |release|?
 ==
 
@@ -99,14 +123,6 @@ Bug Fixes
 
 Improvements to Clang's diagnostics
 ^^^
-- Clang will now correctly diagnose as ill-formed a constant expression where 
an
-  enum without a fixed underlying type is set to a value outside the range of
-  the enumeration's values. Due to the extended period of time this bug was
-  present in major C++ implementations (including Clang), this error has the
-  ability to be downgraded into a warning (via: 
-Wno-error=enum-constexpr-conversion)
-  to provide a transition period for users. This diagnostic is expected to turn
-  into an error-only diagnostic in the next Clang release. Fixes
-  `Issue 50055: `_.
 - Clang will now check compile-time determinable string literals as format 
strings.
   Fixes `Issue 55805: `_.
 - ``-Wformat`` now recognizes ``%b`` for the ``printf``/``scanf`` family of
@@ -119,10 +135,6 @@ Improvements to Clang's diagnostics
   potential false positives, this diagnostic will not diagnose use of the
   ``true`` macro (from ``>`) in C language mode despite the macro
   being defined to expand to ``1``.
-- ``-Wincompatible-function-pointer-types`` now defaults to an error in all C
-  language modes. It may be downgraded to a warning with
-  ``-Wno-error=incompatible-function-pointer-types`` or disabled entirely with
-  ``-Wno-implicit-function-pointer-types``.
 - Clang will now print more information about failed static assertions. In
   particular, simple static assertion expressions are evaluated to their
   compile-time value and printed out if the assertion fails.



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

[PATCH] D133771: Add a "Potentially Breaking Changes" section to the Clang release notes

[Aaron Ballman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
aaron.ballman marked an inline comment as done.
Closed by commit rGe076680bd59c: Add a "Potentially Breaking Changes" 
section to the Clang release notes (authored by aaron.ballman).

Changed prior to commit:
  https://reviews.llvm.org/D133771?vs=459718&id=460365#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133771

Files:
  clang/docs/ReleaseNotes.rst


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -36,6 +36,30 @@
 the current one. To see the release notes for a specific release, please
 see the `releases page `_.
 
+Potentially Breaking Changes
+
+These changes are ones which we think may surprise users when upgrading to
+Clang |release| because of the opportunity they pose for disruption to existing
+code bases.
+
+- Clang will now correctly diagnose as ill-formed a constant expression where 
an
+  enum without a fixed underlying type is set to a value outside the range of
+  the enumeration's values. Due to the extended period of time this bug was
+  present in major C++ implementations (including Clang), this error has the
+  ability to be downgraded into a warning (via:
+  ``-Wno-error=enum-constexpr-conversion``) to provide a transition period for
+  users. This diagnostic is expected to turn into an error-only diagnostic in
+  the next Clang release. Fixes
+  `Issue 50055: `_.
+- ``-Wincompatible-function-pointer-types`` now defaults to an error in all C
+  language modes. It may be downgraded to a warning with
+  ``-Wno-error=incompatible-function-pointer-types`` or disabled entirely with
+  ``-Wno-implicit-function-pointer-types``. *NOTE* We recommend that projects
+  using configure scripts verify the results do not change before/after setting
+  ``-Werror=incompatible-function-pointer-types`` to avoid incompatibility with
+  Clang 16.
+
+
 What's New in Clang |release|?
 ==
 
@@ -99,14 +123,6 @@
 
 Improvements to Clang's diagnostics
 ^^^
-- Clang will now correctly diagnose as ill-formed a constant expression where 
an
-  enum without a fixed underlying type is set to a value outside the range of
-  the enumeration's values. Due to the extended period of time this bug was
-  present in major C++ implementations (including Clang), this error has the
-  ability to be downgraded into a warning (via: 
-Wno-error=enum-constexpr-conversion)
-  to provide a transition period for users. This diagnostic is expected to turn
-  into an error-only diagnostic in the next Clang release. Fixes
-  `Issue 50055: `_.
 - Clang will now check compile-time determinable string literals as format 
strings.
   Fixes `Issue 55805: `_.
 - ``-Wformat`` now recognizes ``%b`` for the ``printf``/``scanf`` family of
@@ -119,10 +135,6 @@
   potential false positives, this diagnostic will not diagnose use of the
   ``true`` macro (from ``>`) in C language mode despite the macro
   being defined to expand to ``1``.
-- ``-Wincompatible-function-pointer-types`` now defaults to an error in all C
-  language modes. It may be downgraded to a warning with
-  ``-Wno-error=incompatible-function-pointer-types`` or disabled entirely with
-  ``-Wno-implicit-function-pointer-types``.
 - Clang will now print more information about failed static assertions. In
   particular, simple static assertion expressions are evaluated to their
   compile-time value and printed out if the assertion fails.


Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -36,6 +36,30 @@
 the current one. To see the release notes for a specific release, please
 see the `releases page `_.
 
+Potentially Breaking Changes
+
+These changes are ones which we think may surprise users when upgrading to
+Clang |release| because of the opportunity they pose for disruption to existing
+code bases.
+
+- Clang will now correctly diagnose as ill-formed a constant expression where an
+  enum without a fixed underlying type is set to a value outside the range of
+  the enumeration's values. Due to the extended period of time this bug was
+  present in major C++ implementations (including Clang), this error has the
+  ability to be downgraded into a warning (via:
+  ``-Wno-error=enum-constexpr-conversion``) to provide a transition period for
+  users. This diagnostic is expected to turn into an error-only diagnostic in
+  the next Clang release. Fixes
+  `Issue 50

[PATCH] D133930: [clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel`.

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 460366.
wyt marked an inline comment as done.
wyt added a comment.

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133930

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

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -14,10 +14,8 @@
 #include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Tooling.h"
-#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -1248,13 +1246,9 @@
  Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
 ASTContext &Ctx, const CFGElement &Elt,
 const TypeErasedDataflowAnalysisState &State) mutable {
-  auto Stmt = Elt.getAs();
-  if (!Stmt) {
-return;
-  }
-  auto StmtDiagnostics =
-  Diagnoser.diagnose(Ctx, Stmt->getStmt(), State.Env);
-  llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
+  auto EltDiagnostics =
+  Diagnoser.diagnose(Ctx, &Elt, State.Env);
+  llvm::move(EltDiagnostics, std::back_inserter(Diagnostics));
 })
 .withASTBuildArgs(
 {"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"})
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -18,8 +18,9 @@
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
-#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "clang/Analysis/FlowSensitive/NoopLattice.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/SourceLocation.h"
@@ -559,41 +560,42 @@
   // lot of duplicated work (e.g. string comparisons), consider providing APIs
   // that avoid it through memoization.
   auto IgnorableOptional = ignorableOptional(Options);
-  return MatchSwitchBuilder()
+  return CFGMatchSwitchBuilder()
   // Attach a symbolic "has_value" state to optional values that we see for
   // the first time.
-  .CaseOf(
+  .CaseOfCFGStmt(
   expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()),
   initializeOptionalReference)
 
   // make_optional
-  .CaseOf(isMakeOptionalCall(), transferMakeOptionalCall)
+  .CaseOfCFGStmt(isMakeOptionalCall(), transferMakeOptionalCall)
 
   // optional::optional
-  .CaseOf(
+  .CaseOfCFGStmt(
   isOptionalInPlaceConstructor(),
   [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,
  LatticeTransferState &State) {
 assignOptionalValue(*E, State, State.Env.getBoolLiteralValue(true));
   })
-  .CaseOf(
+  .CaseOfCFGStmt(
   isOptionalNulloptConstructor(),
   [](const CXXConstructExpr *E, const MatchFinder::MatchResult &,
  LatticeTransferState &State) {
 assignOptionalValue(*E, State,
 State.Env.getBoolLiteralValue(false));
   })
-  .CaseOf(isOptionalValueOrConversionConstructor(),
-transferValueOrConversionConstructor)
+  .CaseOfCFGStmt(isOptionalValueOrConversionConstructor(),
+   transferValueOrConversionConstructor)
 
   // optional::operator=
-  .CaseOf(isOptionalValueOrConversionAssignment(),
-   transferValueOrConversionAssignment)
-  .CaseOf(isOptionalNulloptAssignment(),
-   transferNulloptAssignment)
+  .CaseOfCFGStmt(
+  isOptionalValueOrConversionAssignment(),
+  transferValueOrConversionAssignment)
+  .CaseOfCFGStmt(isOptionalNulloptAssignment(),
+  transferNulloptAssignment)
 
   // option

[PATCH] D133933: [clang][dataflow] Modify `transfer` in `DataflowModel` to take `CFGElement` as input instead of `Stmt`.

[weiyi via Phabricator via cfe-commits]

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

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133933

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
  clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
  clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
@@ -120,9 +120,7 @@
   static NoopLattice initialElement() { return NoopLattice(); }
 
   void transfer(const CFGElement *E, NoopLattice &, Environment &Env) {
-if (auto S = E->getAs()) {
-  M.transfer(S->getStmt(), Env);
-}
+M.transfer(E, Env);
   }
 
 private:
Index: clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
@@ -50,7 +50,11 @@
   return CheckDecls.contains(&D);
 }
 
-bool ChromiumCheckModel::transfer(const Stmt *Stmt, Environment &Env) {
+bool ChromiumCheckModel::transfer(const CFGElement *Element, Environment &Env) 
{
+  auto CS = Element->getAs();
+  if (!CS)
+return false;
+  auto Stmt = CS->getStmt();
   if (const auto *Call = dyn_cast(Stmt)) {
 if (const auto *M = dyn_cast(Call->getDirectCallee())) {
   if (isCheckLikeMethod(CheckDecls, *M)) {
Index: clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
===
--- clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
+++ clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
@@ -26,7 +26,7 @@
 class ChromiumCheckModel : public DataflowModel {
 public:
   ChromiumCheckModel() = default;
-  bool transfer(const Stmt *Stmt, Environment &Env) override;
+  bool transfer(const CFGElement *Element, Environment &Env) override;
 
 private:
   /// Declarations for `::logging::CheckError::.*Check`, lazily initialized.
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -231,8 +231,8 @@
 /// example, a model may capture a type and its related functions.
 class DataflowModel : public Environment::ValueModel {
 public:
-  /// Return value indicates whether the model processed the `Stmt`.
-  virtual bool transfer(const Stmt *Stmt, Environment &Env) = 0;
+  /// Return value indicates whether the model processed the `Element`.
+  virtual bool transfer(const CFGElement *Element, Environment &Env) = 0;
 };
 
 } // namespace dataflow


Index: clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp
@@ -120,9 +120,7 @@
   static NoopLattice initialElement() { return NoopLattice(); }
 
   void transfer(const CFGElement *E, NoopLattice &, Environment &Env) {
-if (auto S = E->getAs()) {
-  M.transfer(S->getStmt(), Env);
-}
+M.transfer(E, Env);
   }
 
 private:
Index: clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/ChromiumCheckModel.cpp
@@ -50,7 +50,11 @@
   return CheckDecls.contains(&D);
 }
 
-bool ChromiumCheckModel::transfer(const Stmt *Stmt, Environment &Env) {
+bool ChromiumCheckModel::transfer(const CFGElement *Element, Environment &Env) {
+  auto CS = Element->getAs();
+  if (!CS)
+return false;
+  auto Stmt = CS->getStmt();
   if (const auto *Call = dyn_cast(Stmt)) {
 if (const auto *M = dyn_cast(Call->getDirectCallee())) {
   if (isCheckLikeMethod(CheckDecls, *M)) {
Index: clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
===
--- clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
+++ clang/include/clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h
@@ -26,7 +26,7 @@
 class ChromiumCheckModel : public DataflowModel {
 public:
   ChromiumCheckModel() = default;
-  bool transfer(const Stmt *Stmt, Environment &Env) override;
+  bool transfer(const CFGElement *Element, Environment &Env) overr

[PATCH] D133935: [clang][dataflow] Refactor `clang/Analysis/FlowSensitive/MatchSwitchTest.cpp`.

[weiyi via Phabricator via cfe-commits]

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

Address comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133935

Files:
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
@@ -7,26 +7,15 @@
 //===--===//
 
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
-#include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
-#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
-#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
-#include "clang/Analysis/FlowSensitive/MapLattice.h"
 #include "clang/Tooling/Tooling.h"
-#include "llvm/ADT/None.h"
-#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/Twine.h"
-#include "llvm/Support/Error.h"
-#include "llvm/Testing/Support/Annotations.h"
-#include "llvm/Testing/Support/Error.h"
-#include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 #include 
@@ -36,169 +25,114 @@
 
 using namespace clang;
 using namespace dataflow;
+using namespace ast_matchers;
 
 namespace {
-using ::testing::Pair;
-using ::testing::UnorderedElementsAre;
 
-class BooleanLattice {
-public:
-  BooleanLattice() : Value(false) {}
-  explicit BooleanLattice(bool B) : Value(B) {}
-
-  static BooleanLattice bottom() { return BooleanLattice(false); }
-
-  static BooleanLattice top() { return BooleanLattice(true); }
-
-  LatticeJoinEffect join(BooleanLattice Other) {
-auto Prev = Value;
-Value = Value || Other.Value;
-return Prev == Value ? LatticeJoinEffect::Unchanged
- : LatticeJoinEffect::Changed;
-  }
-
-  friend bool operator==(BooleanLattice LHS, BooleanLattice RHS) {
-return LHS.Value == RHS.Value;
-  }
-
-  friend std::ostream &operator<<(std::ostream &Os, const BooleanLattice &B) {
-Os << B.Value;
-return Os;
-  }
-
-  bool value() const { return Value; }
-
-private:
-  bool Value;
-};
-} // namespace
-
-MATCHER_P(Holds, m,
-  ((negation ? "doesn't hold" : "holds") +
-   llvm::StringRef(" a lattice element that ") +
-   ::testing::DescribeMatcher(m, negation))
-  .str()) {
-  return ExplainMatchResult(m, arg.Lattice, result_listener);
-}
-
-void TransferSetTrue(const DeclRefExpr *,
- const ast_matchers::MatchFinder::MatchResult &,
- TransferState &State) {
-  State.Lattice = BooleanLattice(true);
-}
-
-void TransferSetFalse(const Stmt *,
-  const ast_matchers::MatchFinder::MatchResult &,
-  TransferState &State) {
-  State.Lattice = BooleanLattice(false);
-}
-
-class TestAnalysis : public DataflowAnalysis {
-  MatchSwitch> TransferSwitch;
-
-public:
-  explicit TestAnalysis(ASTContext &Context)
-  : DataflowAnalysis(Context) {
-using namespace ast_matchers;
-TransferSwitch =
-MatchSwitchBuilder>()
-.CaseOf(declRefExpr(to(varDecl(hasName("X",
- TransferSetTrue)
-.CaseOf(callExpr(callee(functionDecl(hasName("Foo",
-  TransferSetFalse)
-.Build();
-  }
-
-  static BooleanLattice initialElement() { return BooleanLattice::bottom(); }
-
-  void transfer(const Stmt *S, BooleanLattice &L, Environment &Env) {
-TransferState State(L, Env);
-TransferSwitch(*S, getASTContext(), State);
-  }
-};
-
-template 
-void RunDataflow(llvm::StringRef Code, Matcher Expectations) {
-  ASSERT_THAT_ERROR(
-  test::checkDataflow(
-  Code, "fun",
-  [](ASTContext &C, Environment &) { return TestAnalysis(C); },
-  [&Expectations](
-  llvm::ArrayRef>>
-  Results,
-  ASTContext &) { EXPECT_THAT(Results, Expectations); },
-  {"-fsyntax-only", "-std=c++17"}),
-  llvm::Succeeded());
-}
-
-TEST(MatchSwitchTest, JustX) {
-  std::string Code = R"(
-void fun() {
-  int X = 1;
-  (void)X;
-  // [[p]]
-}
-  )";
-  RunDataflow(Code,
-  UnorderedElementsAre(Pair("p", Holds(BooleanLattice(true);
-}
-
-TEST(MatchSwitchTest, JustFoo) {
-  std::string Code = R"(
-void Foo();
-void fun() {
-  Foo();
-  // [[p]]
-}
-  )";
-  RunDataflow(Code,
-  UnorderedElementsAre(Pair("p", Holds(BooleanLattice(false);
-}
-
-TEST(MatchSwitchTest, XThenFoo) {
+T

[PATCH] D133937: [mlir] Remove the unused source file.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: Groverkss.
Herald added subscribers: anlunx, bzcheeseman, arjunp, sdasgup3, wenzhicui, 
wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, 
grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, 
shauheen, rriddle, mehdi_amini.
Herald added a reviewer: aartbik.
Herald added a project: All.
hokein requested review of this revision.
Herald added a reviewer: nicolasvasilache.
Herald added subscribers: stephenneuendorffer, nicolasvasilache.
Herald added a reviewer: dcaballe.
Herald added a project: MLIR.

It seems to be a missing file in 84d07d021333f7b5716f0444d5c09105557272e0 



Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133937

Files:
  mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp


Index: mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp
===
--- mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-//===- AffineStructuresParser.cpp - Parser for AffineStructures -*- 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 "./AffineStructuresParser.h"
-#include "mlir/AsmParser/AsmParser.h"
-#include "mlir/IR/IntegerSet.h"
-
-using namespace mlir;
-using namespace presburger;
-
-FailureOr
-mlir::parseIntegerSetToFAC(llvm::StringRef str, MLIRContext *context,
-   bool printDiagnosticInfo) {
-  IntegerSet set = parseIntegerSet(str, context, printDiagnosticInfo);
-
-  if (!set)
-return failure();
-
-  return FlatAffineValueConstraints(set);
-}


Index: mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp
===
--- mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-//===- AffineStructuresParser.cpp - Parser for AffineStructures -*- 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 "./AffineStructuresParser.h"
-#include "mlir/AsmParser/AsmParser.h"
-#include "mlir/IR/IntegerSet.h"
-
-using namespace mlir;
-using namespace presburger;
-
-FailureOr
-mlir::parseIntegerSetToFAC(llvm::StringRef str, MLIRContext *context,
-   bool printDiagnosticInfo) {
-  IntegerSet set = parseIntegerSet(str, context, printDiagnosticInfo);
-
-  if (!set)
-return failure();
-
-  return FlatAffineValueConstraints(set);
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133711: [Sema] Reject array element types whose sizes aren't a multiple of their alignments

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133711#3790933 , @rnk wrote:

> This seems like a new error that will be pretty easy to run into, especially 
> in the funky MSVC virtual base case. @aaron.ballman proposed some kind of new 
> breaking change announcement mechanism. Can you add a release note and send 
> an announcement following that process? I think it's just posting to 
> Discourse announcements.

I just landed the change to the release notes so there's now a potentially 
breaking changes section to put the note under. We're not quite ready to post 
to announcements yet (that channel is locked so we need to remove the 
gatekeeping mechanism there), but when I'm able to post to announcements, I 
plan to announce everything already in the release notes anyway. So I'd say 
adding `clang-vendors` like @efriedma did and putting in the release notes is 
great for now.

Note, precommit CI found a relevant failure:

   TEST 'Clang :: 
CodeGenCXX/override-layout-packed-base.cpp' FAILED 
  
  Script:
  
  --
  
  : 'RUN: at line 1';   
/var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 
-internal-isystem 
/var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/16.0.0/include 
-nostdsysteminc -triple i686-windows-msvc -w -fdump-record-layouts-simple 
-foverride-record-layout=/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/Inputs/override-layout-packed-base.layout
 
/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/override-layout-packed-base.cpp
 | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck 
/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/override-layout-packed-base.cpp
  
  --
  
  Exit Code: 1
  
  
  
  Command Output (stderr):
  
  --
  
  
/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/override-layout-packed-base.cpp:38:7:
 error: size of array element of type 'C' (11 bytes) isn't a multiple of its 
alignment (4 bytes)
  
C cs[sizeof(C)];
  
^
  
  
/var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGenCXX/override-layout-packed-base.cpp:39:7:
 error: size of array element of type 'D' (15 bytes) isn't a multiple of its 
alignment (4 bytes)
  
D ds[sizeof(D)];
  
^
  
  2 errors generated.
  
  
  
  --
  
  
  
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133711

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

[PATCH] D133436: Ground work for cuda-related checks in clang-tidy

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460374.
barcisz marked 2 inline comments as done.
barcisz added a comment.

Changes suggested by njames93


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133436

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  clang-tools-extra/test/clang-tidy/check_clang_tidy.py
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h

Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
@@ -0,0 +1,26 @@
+/* Minimal declarations for CUDA support.  Testing purposes only. */
+
+#define __constant__ __attribute__((constant))
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+#define __host__ __attribute__((host))
+#define __shared__ __attribute__((shared))
+
+struct dim3 {
+  unsigned x, y, z;
+  __host__ __device__ dim3(unsigned x, unsigned y = 1, unsigned z = 1) : x(x), y(y), z(z) {}
+};
+
+typedef struct cudaStream *cudaStream_t;
+typedef enum cudaError {} cudaError_t;
+extern "C" int cudaConfigureCall(dim3 gridSize, dim3 blockSize,
+ size_t sharedSize = 0,
+ cudaStream_t stream = 0);
+extern "C" int __cudaPushCallConfiguration(dim3 gridSize, dim3 blockSize,
+   size_t sharedSize = 0,
+   cudaStream_t stream = 0);
+extern "C" cudaError_t cudaLaunchKernel(const void *func, dim3 gridDim,
+dim3 blockDim, void **args,
+size_t sharedMem, cudaStream_t stream);
+
+extern "C" __device__ int printf(const char*, ...);
Index: clang-tools-extra/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -93,7 +93,7 @@
 
 file_name_with_extension = self.assume_file_name or self.input_file_name
 _, extension = os.path.splitext(file_name_with_extension)
-if extension not in ['.c', '.hpp', '.m', '.mm']:
+if extension not in ['.c', '.cu', '.hpp', '.m', '.mm']:
   extension = '.cpp'
 self.temp_file_name = self.temp_file_name + extension
 
@@ -115,9 +115,14 @@
   self.clang_extra_args = ['-fobjc-abi-version=2', '-fobjc-arc', '-fblocks'] + \
   self.clang_extra_args
 
-if extension in ['.cpp', '.hpp', '.mm']:
+if extension in ['.cpp', '.cu', '.hpp', '.mm']:
   self.clang_extra_args.append('-std=' + self.std)
 
+# Tests should not rely on a certain cuda headers and library version
+# being available on the machine
+if extension == '.cu':
+  self.clang_extra_args.extend(["--no-cuda-version-check", "-nocudalib", "-nocudainc"])
+
 # Tests should not rely on STL being available, and instead provide mock
 # implementations of relevant APIs.
 self.clang_extra_args.append('-nostdinc++')
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -67,6 +67,7 @@
 ``concurrency-``   Checks related to concurrent programming (including
threads, fibers, coroutines, etc.).
 ``cppcoreguidelines-`` Checks related to C++ Core Guidelines.
+``cuda-``  Checks related to CUDA best practices.
 ``darwin-``Checks related to Darwin coding conventions.
 ``fuchsia-``   Checks related to Fuchsia coding conventions.
 ``google-``Checks related to Google coding conventions.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -16,6 +16,7 @@
clang-analyzer/*
concurrency/*
cppcoreguidelines/*
+   cuda/*
darwin/*
fuchsia/*
google/*
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -96,6 +96,8 @@
 Improvements to clang-tidy
 --
 
+- Introduce the cuda module for checks specific to CUDA code.
+
 New checks
 ^^
 
Index: clang-tools-extra/clang-tidy/cuda/CudaTid

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460376.
barcisz added a comment.

Rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-api-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'cudaHandler'}, \
+// RUN:  {key: cuda-unsafe-api-call.AcceptedHandlers, \
+// RUN:   value: 'CUDA_HANDLER, DUMMY_C

[PATCH] D133937: [mlir] Remove the unused source file.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe488ce29ec5e: [mlir] Remove the unused source file. 
(authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133937

Files:
  mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp


Index: mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp
===
--- mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-//===- AffineStructuresParser.cpp - Parser for AffineStructures -*- 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 "./AffineStructuresParser.h"
-#include "mlir/AsmParser/AsmParser.h"
-#include "mlir/IR/IntegerSet.h"
-
-using namespace mlir;
-using namespace presburger;
-
-FailureOr
-mlir::parseIntegerSetToFAC(llvm::StringRef str, MLIRContext *context,
-   bool printDiagnosticInfo) {
-  IntegerSet set = parseIntegerSet(str, context, printDiagnosticInfo);
-
-  if (!set)
-return failure();
-
-  return FlatAffineValueConstraints(set);
-}


Index: mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp
===
--- mlir/unittests/Dialect/Affine/Analysis/AffineStructuresParser.cpp
+++ /dev/null
@@ -1,25 +0,0 @@
-//===- AffineStructuresParser.cpp - Parser for AffineStructures -*- 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 "./AffineStructuresParser.h"
-#include "mlir/AsmParser/AsmParser.h"
-#include "mlir/IR/IntegerSet.h"
-
-using namespace mlir;
-using namespace presburger;
-
-FailureOr
-mlir::parseIntegerSetToFAC(llvm::StringRef str, MLIRContext *context,
-   bool printDiagnosticInfo) {
-  IntegerSet set = parseIntegerSet(str, context, printDiagnosticInfo);
-
-  if (!set)
-return failure();
-
-  return FlatAffineValueConstraints(set);
-}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133659: [Clang] P1169R4: static operator()

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a subscriber: erichkeane.
cor3ntin added a comment.

Beside a missing test, this LGTM
I'd like someone else (@erichkeane maybe) to review the codegen tests.




Comment at: clang/test/Parser/cxx2b-lambdas.cpp:60
+  auto SC5 = [&y = x]() static {}; // expected-error {{a static lambda cannot 
have any captures}} // expected-note {{captures declared here}}
+  auto SC6 = [=]() static {}; // expected-error {{a static lambda cannot have 
any captures}} // expected-note {{captures declared here}}
+

For completeness, can you add a test for `*this`/`this`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133659

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

[PATCH] D128440: [WebAssembly] Initial support for reference type funcref in clang

[Paulo Matos via Phabricator via cfe-commits]

pmatos added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:954
 10, // wasm_externref,
+20, // wasm_funcref
 };

aaron.ballman wrote:
> Where did this value come from?
Unsure what you mean here. This is the address space number we also use on the 
LLVM side.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128440

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

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

It seems wrong to change semantics of initialization when instantiating concept 
requirements. It implies that semantic checking may behave differently inside 
requires expressions, this is a red flag.
Clang has a mechanism 

 to prevent recursion when considering user-defined conversion for copy 
initialization.

Currently the intention of the Clang code path that handles this case is to:

1. Deduce `TO` to be `Deferred`
2. Try to check `Numeric`,
3. Check conversions during overload resolution for `foo(a)`. Go to step 1, 

4.  Check 

 that conversion operator converts the type to itself and mark the candidate as 
failed.

If move the check in step 4 to happen before step 3 (even if we need to 
duplicate the check), we will get the desired semantics.

Does that look reasonable?

I also agree there is a more general issue with recursive concept 
instantiations at play here, e.g. for code like

  template  concept A = requires (T a) { foo(a); };
  template  concept B = requires (T a) { bar(a); };
  
  struct X {};
  template  void bar(T);
  template  void foo(T);
  
  void test() {
  foo(X());
  }

Clang will currently cut this out because the template instantiation depth is 
too high, whereas GCC will provide a useful diagnostics that says concept 
satisfaction computation is recursive.
We probably want a more informative error message too. Probably worth 
investigating separately from that particular change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133052

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

[clang] 3ce7d25 - [clang][RecoveryExpr] Don't perform alignment check if parameter type is dependent

[Aleksandr Platonov via cfe-commits]

Author: Aleksandr Platonov
Date: 2022-09-15T15:51:43+03:00
New Revision: 3ce7d256f2d7f64d57ccbfd7935d55eafc639314

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

LOG: [clang][RecoveryExpr] Don't perform alignment check if parameter type is 
dependent

This patch fixes a crash which appears because of getTypeAlignInChars() call 
with depentent type.

Reviewed By: hokein

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

Added: 


Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/recovery-expr-type.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index bc9642d17d852..e419287014893 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -5690,9 +5690,9 @@ void Sema::CheckArgAlignment(SourceLocation Loc, 
NamedDecl *FDecl,
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ArgTy.isNull() || ParamTy->isIncompleteType() ||
-  ArgTy->isIncompleteType() || ParamTy->isUndeducedType() ||
-  ArgTy->isUndeducedType())
+  if (ArgTy.isNull() || ParamTy->isDependentType() ||
+  ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);

diff  --git a/clang/test/SemaCXX/recovery-expr-type.cpp 
b/clang/test/SemaCXX/recovery-expr-type.cpp
index df6f4b69763c7..a5ba1ae2b8222 100644
--- a/clang/test/SemaCXX/recovery-expr-type.cpp
+++ b/clang/test/SemaCXX/recovery-expr-type.cpp
@@ -162,3 +162,12 @@ extern "C" void *memset(void *, int b, unsigned long) {
   b = __builtin_object_size(c, 0); // crash2
 }
 }
+
+namespace test15 {
+void f() {
+  struct {
+void m(int (&)[undefined()]) {} // expected-error {{undeclared identifier}}
+  } S;
+  S.m(1); // no crash
+}
+}



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

[PATCH] D133886: [clang][RecoveryExpr] Don't perform alignment check if parameter type is dependent

[Aleksandr Platonov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ArcsinX marked an inline comment as done.
Closed by commit rG3ce7d256f2d7: [clang][RecoveryExpr] Don't perform 
alignment check if parameter type is… (authored by ArcsinX).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133886

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -162,3 +162,12 @@
   b = __builtin_object_size(c, 0); // crash2
 }
 }
+
+namespace test15 {
+void f() {
+  struct {
+void m(int (&)[undefined()]) {} // expected-error {{undeclared identifier}}
+  } S;
+  S.m(1); // no crash
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -5690,9 +5690,9 @@
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ArgTy.isNull() || ParamTy->isIncompleteType() ||
-  ArgTy->isIncompleteType() || ParamTy->isUndeducedType() ||
-  ArgTy->isUndeducedType())
+  if (ArgTy.isNull() || ParamTy->isDependentType() ||
+  ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);


Index: clang/test/SemaCXX/recovery-expr-type.cpp
===
--- clang/test/SemaCXX/recovery-expr-type.cpp
+++ clang/test/SemaCXX/recovery-expr-type.cpp
@@ -162,3 +162,12 @@
   b = __builtin_object_size(c, 0); // crash2
 }
 }
+
+namespace test15 {
+void f() {
+  struct {
+void m(int (&)[undefined()]) {} // expected-error {{undeclared identifier}}
+  } S;
+  S.m(1); // no crash
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -5690,9 +5690,9 @@
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
-  if (ArgTy.isNull() || ParamTy->isIncompleteType() ||
-  ArgTy->isIncompleteType() || ParamTy->isUndeducedType() ||
-  ArgTy->isUndeducedType())
+  if (ArgTy.isNull() || ParamTy->isDependentType() ||
+  ParamTy->isIncompleteType() || ArgTy->isIncompleteType() ||
+  ParamTy->isUndeducedType() || ArgTy->isUndeducedType())
 return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 460389.

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

https://reviews.llvm.org/D123630

Files:
  clang/docs/UsersManual.rst
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/ffp-contract-option.c
  clang/test/Driver/fp-contract.c

Index: clang/test/Driver/fp-contract.c
===
--- /dev/null
+++ clang/test/Driver/fp-contract.c
@@ -0,0 +1,114 @@
+// Test that -ffp-contract is set to the right value when combined with
+// the options -ffast-math and -fno-fast-math.
+
+// RUN: %clang -### -ffast-math -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// CHECK-FPC-FAST: "-ffp-contract=fast"
+
+// RUN: %clang -### -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+// CHECK-FPC-ON:   "-ffp-contract=on"
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+// CHECK-FPC-OFF:  "-ffp-contract=off"
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+// RUN: %clang -### -ffp-contract=on -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -ffp-contract=fast \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=on -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=off -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffast-math -ffp-contract=fast -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -fno-fast-math -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=on -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=off -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -fno-fast-math -ffp-contract=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=on \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=fast -fno-fast-math -ffp-contract=off \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=fast \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=off -fno-fast-math -ffp-contract=on \
+// RUN: -c %s 2>&1 | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=on -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=off -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-FAST %s
+
+// RUN: %clang -### -ffp-contract=on -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-ON %s
+
+// RUN: %clang -### -ffp-contract=off -ffast-math -fno-fast-math -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FPC-OFF %s
+
+// RUN: %clang -### -ffp-contract=fast -ffast-math -fno-

[PATCH] D133641: [Clang] [Sema] Ignore invalid multiversion function redeclarations

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

makes sense to me, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133641

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

[PATCH] D125142: [clang][auto-init] Remove -enable flag for "zero" mode

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D125142#3782296 , @xbolva00 wrote:

> In D125142#3782244 , 
> @nickdesaulniers wrote:
>
>> @rsmith can we get some guidance here?  Has your opinion changed in the time 
>> since GCC has been shipping this?
>
> Maybe we should now ask @aaron.ballman as he is now main code owner.

I agree with @rsmith that this is creating a language dialect, and it's 
creating one that at least WG21 has shown divided opinions on. So I think it is 
worth exploring the idea he had in https://reviews.llvm.org/D79249 in so much 
as that comes closer to meeting our bar for language extensions. However, I 
still think GCC compatibility is important in how we expose the functionality. 
If we spell the option `-ftrivial-auto-var-init=zero`, it should work the same 
as GCC (modulo bugs); the alternative would cause too much confusion in the 
field, I think.

That said, there's significant deployment experience with the zeroing flag, and 
some committees (like WG14) honor prior art when considering standardization. 
That there's now one very popular C implementation exposing this functionality 
for users makes it slightly more likely WG14 would be interested in 
standardizing something in this realm. Having a second very popular C 
implementation further strengthens that case. So while WG21 has shown divided 
opinions, WG14 hasn't been consulted. Putting a paper in front of them that 
shows some signs of life (which we could do already today without any changes 
here) would remove the concerns regarding meeting our bar for an extension if 
we wanted the same semantics as GCC. However, I'm still skeptical of WG14 
adopting such a proposal, so it's by no means a slam dunk. Despite that and 
given the success of the feature in GCC, I'm less concerned about the creation 
of a language dialect in this particular case. We've done that before (many of 
our floating-point math flags like `-ffast-math` and `-fhonor-nans` come to 
mind) and we have sufficient evidence that users are making use of 
`-enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang`: 
https://sourcegraph.com/search?q=context:global+enable-trivial-auto-var-init-zero-knowing-it-will-be-removed-from-clang+-file:.*test.*+-file:Options.td&patternType=standard

tl;dr: I think it's defensible to move forward with this patch as-is, 
especially given that we've been threatening to remove the -cc1 flag (we're 
nearing a point where we either need to support this feature or drop it and I 
think people would strongly prefer we support it).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125142

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

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

[David Rector via Phabricator via cfe-commits]

davrec added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:12367
+  case Type::Class:
\
+llvm_unreachable("Unexpected " Kind ": " #Class);
+

aaronpuchert wrote:
> mizvekov wrote:
> > aaronpuchert wrote:
> > > mizvekov wrote:
> > > > davrec wrote:
> > > > > mizvekov wrote:
> > > > > > davrec wrote:
> > > > > > > mizvekov wrote:
> > > > > > > > davrec wrote:
> > > > > > > > > Could we just return `X` here? Would that just default to the 
> > > > > > > > > old behavior instead of crashing whenever unforeseen cases 
> > > > > > > > > arise?  
> > > > > > > > No, I think we should enforce the invariants and make sure we 
> > > > > > > > are handling everything that can be handled.
> > > > > > > > 
> > > > > > > > Classing `TemplateTypeParm`  as sugar-free was what was wrong 
> > > > > > > > and we missed this on the review.
> > > > > > > There might always going to be a few rare corner cases vulnerable 
> > > > > > > to this though, particularly as more types are added and the 
> > > > > > > people adding them don't pay strict attention to how to 
> > > > > > > incorporate them here, and don't write the requisite tests (which 
> > > > > > > seem very difficult to foresee and produce).  When those cases 
> > > > > > > arise we will be crashing even though we could produce a 
> > > > > > > perfectly good program with the intended semantics; the only 
> > > > > > > thing that would suffer for most users is slightly less clear 
> > > > > > > diagnostic messages for those rare cases.  I think it would be 
> > > > > > > better to let those cases gradually percolate to our attention 
> > > > > > > via bug reports concerning those diagnostics, rather than 
> > > > > > > inconveniencing the user and demanding immediate attention via 
> > > > > > > crashes.  Or am I missing something?
> > > > > > I could on the same argument remove all asserts here and just let 
> > > > > > the program not crash on unforeseen circumstances.
> > > > > > 
> > > > > > On the other hand, having these asserts here helps us catch bugs 
> > > > > > not only here, but in other parts of the code. For example uniquing 
> > > > > > / canonicalization bugs.
> > > > > > 
> > > > > > If someone changes the properties of a type so that the assumptions 
> > > > > > here are not valid anymore, it's helpful to have that pointed out 
> > > > > > soon.
> > > > > > 
> > > > > > Going for as an example this specific bug, if there werent those 
> > > > > > asserts / unreachables in place and we had weakened the validation 
> > > > > > here, it would take a very long time for us to figure out we were 
> > > > > > making the wrong assumption with regards to TemplateTypeParmType.
> > > > > > 
> > > > > > I'd rather figure that out sooner on CI / internal testing rather 
> > > > > > than have a bug created by a user two years from now.
> > > > > Yes its nicer to developers to get stack traces and reproductions 
> > > > > whenever something goes wrong, and crashing is a good way to get 
> > > > > those, but users probably won't be so thrilled about it.  Especially 
> > > > > given that the main selling point of this patch is that it makes 
> > > > > diagnostics nicer for users: isn't it a bit absurd to crash whenever 
> > > > > we can't guarantee our diagnostics will be perfect?
> > > > > 
> > > > > And again the real problem is future types not being properly 
> > > > > incorporated here and properly tested: i.e. the worry is that this 
> > > > > will be a continuing source of crashes, even if we handle all the 
> > > > > present types properly.
> > > > > 
> > > > > How about we leave it as an unreachable for now, to help ensure all 
> > > > > the present types are handled, but if months or years from now there 
> > > > > continue to be crashes due to this, then just return X (while maybe 
> > > > > write something to llvm::errs() to encourage users to report it), so 
> > > > > we don't make the perfect the enemy of the good.
> > > > It's not about crashing when it won't be perfect. We already do these 
> > > > kinds of things, see the FIXMEs around the TemplateArgument and 
> > > > NestedNameSpecifier, where we just return something worse than we wish 
> > > > to, just because we don't have time to implement it now.
> > > > 
> > > > These unreachables and asserts here are about testing / documenting our 
> > > > knowledge of the system and making it easier to find problems. These 
> > > > should be impossible to happen in correct code, and if they do happen 
> > > > because of mistakes, fixing them sooner is just going to save us 
> > > > resources.
> > > > 
> > > > `llvm::errs` suggestion I perceive as out of line with current practice 
> > > > in LLVM, we don't and have a system for producing diagnostics for 
> > > > results possibly affected by FIXME and TODO situations and the like, as 
> > > > far as I know, and I am not aware of any discussions in

[PATCH] D133941: [clang][Interp] Record item types in InterpStack

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

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

Since I've run into this a few times now...

When `push()`ing something on the stack, we need to later `pop()` the correct 
type of value, otherwise we run into problems.

This s just an idea and I've disabled it if `NDEBUG` is defined. The current 
tests all still work though, which is good.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133941

Files:
  clang/lib/AST/Interp/InterpStack.h


Index: clang/lib/AST/Interp/InterpStack.h
===
--- clang/lib/AST/Interp/InterpStack.h
+++ clang/lib/AST/Interp/InterpStack.h
@@ -13,7 +13,9 @@
 #ifndef LLVM_CLANG_AST_INTERP_INTERPSTACK_H
 #define LLVM_CLANG_AST_INTERP_INTERPSTACK_H
 
+#include "PrimType.h"
 #include 
+#include 
 
 namespace clang {
 namespace interp {
@@ -29,10 +31,17 @@
   /// Constructs a value in place on the top of the stack.
   template  void push(Tys &&... Args) {
 new (grow(aligned_size())) T(std::forward(Args)...);
+#ifndef NDEBUG
+ItemTypes.push_back(toPrimType());
+#endif
   }
 
   /// Returns the value from the top of the stack and removes it.
   template  T pop() {
+#ifndef NDEBUG
+assert(ItemTypes.back() == toPrimType());
+ItemTypes.pop_back();
+#endif
 auto *Ptr = &peek();
 auto Value = std::move(*Ptr);
 Ptr->~T();
@@ -42,6 +51,10 @@
 
   /// Discards the top value from the stack.
   template  void discard() {
+#ifndef NDEBUG
+assert(ItemTypes.back() == toPrimType());
+ItemTypes.pop_back();
+#endif
 auto *Ptr = &peek();
 Ptr->~T();
 shrink(aligned_size());
@@ -111,6 +124,43 @@
   StackChunk *Chunk = nullptr;
   /// Total size of the stack.
   size_t StackSize = 0;
+
+#ifndef NDEBUG
+  /// vector recording the type of data we pushed into the stack.
+  std::vector ItemTypes;
+
+  template  static constexpr PrimType toPrimType() {
+if constexpr (std::is_same::value)
+  return PT_Ptr;
+else if (std::is_same::value || std::is_same::value)
+  return PT_Bool;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Sint8;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Uint8;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Sint16;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Uint16;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Sint32;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Uint32;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Sint64;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Uint64;
+assert(false);
+  }
+#endif
 };
 
 } // namespace interp


Index: clang/lib/AST/Interp/InterpStack.h
===
--- clang/lib/AST/Interp/InterpStack.h
+++ clang/lib/AST/Interp/InterpStack.h
@@ -13,7 +13,9 @@
 #ifndef LLVM_CLANG_AST_INTERP_INTERPSTACK_H
 #define LLVM_CLANG_AST_INTERP_INTERPSTACK_H
 
+#include "PrimType.h"
 #include 
+#include 
 
 namespace clang {
 namespace interp {
@@ -29,10 +31,17 @@
   /// Constructs a value in place on the top of the stack.
   template  void push(Tys &&... Args) {
 new (grow(aligned_size())) T(std::forward(Args)...);
+#ifndef NDEBUG
+ItemTypes.push_back(toPrimType());
+#endif
   }
 
   /// Returns the value from the top of the stack and removes it.
   template  T pop() {
+#ifndef NDEBUG
+assert(ItemTypes.back() == toPrimType());
+ItemTypes.pop_back();
+#endif
 auto *Ptr = &peek();
 auto Value = std::move(*Ptr);
 Ptr->~T();
@@ -42,6 +51,10 @@
 
   /// Discards the top value from the stack.
   template  void discard() {
+#ifndef NDEBUG
+assert(ItemTypes.back() == toPrimType());
+ItemTypes.pop_back();
+#endif
 auto *Ptr = &peek();
 Ptr->~T();
 shrink(aligned_size());
@@ -111,6 +124,43 @@
   StackChunk *Chunk = nullptr;
   /// Total size of the stack.
   size_t StackSize = 0;
+
+#ifndef NDEBUG
+  /// vector recording the type of data we pushed into the stack.
+  std::vector ItemTypes;
+
+  template  static constexpr PrimType toPrimType() {
+if constexpr (std::is_same::value)
+  return PT_Ptr;
+else if (std::is_same::value || std::is_same::value)
+  return PT_Bool;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Sint8;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Uint8;
+else if (std::is_same::value ||
+ std::is_same>::value)
+  return PT_Sint16;
+else if (std::is_same::

[PATCH] D133931: [clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `clang/Analysis/FlowSensitive`.

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: 
clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp:137-140
+auto CS = E->getAs();
+if (!CS)
+  return;
+auto S = CS->getStmt();

This is exactly the same code that you have in 
`SingleVarConstantPropagationTest.cpp` and in the rest of the files. I think 
this could be hoisted into a common function declared in 
`FlowSensitive/DataflowAnalysis.h`.

By the way, wouldn't it be more convenient for the client to provide a 
`transferStmt` function if the they are interested only in `Stmt`s? (Defining 
both `transferStmt` and `transfer` should be a compile time error)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133931

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

[PATCH] D128462: [HLSL] add -I option for dxc mode.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/include/clang/Driver/Options.td:6362
 
-def cl_Group : OptionGroup<"">, Flags<[CLOption]>,
+def cl_Group : OptionGroup<"">, Flags<[CLDXCOption]>,
   HelpText<"CL.EXE COMPATIBILITY OPTIONS">;

kadircet wrote:
> i am failing to understand the semantics of this new option group, and it's 
> causing regressions in clangd. see 
> https://github.com/clangd/clangd/issues/1292.
> 
> is this new group suppose to serve flags that are available both in 
> `--driver-mode=cl` and `--driver-mode=dxc`? i.e. they are for the flags in 
> the intersection?
> are there any flags that are applicable to CL driver mode but not to the DXC 
> mode (and vice versa)?
> 
> why didn't we go for just adding DXCOption mode into here, if every option in 
> CL mode is actually applicable to DXC option as well, rather than introducing 
> a new option?
DXC does not support all the CL options, and vice versa. There are some shared 
options, which is what this group tries to capture.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128462

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

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

[David Spickett via Phabricator via cfe-commits]

DavidSpickett added subscribers: nickdesaulniers, psmith.
DavidSpickett added a comment.

> 1:38 AM  might be good to take them out here as well

So I looked into this. Here are the Arm architectures that clang has that
gcc doesn't:
"armv5tej" // Not in GCC, j = jazelle
"armv7k" // Apple Watch S1
"armv7s" // iPhone 5
"xscale"

(minus some very new ones that are just because I used trunk clang)

These are the gcc architectures that clang doesn't have:
"armv6j"
"armv6s-m"
"armv6z"
"armv6zk"
"armv7"

So a random grab bag of v6 and 7 on both sides. Not worth disturbing that
now.

Arnd picked out jazelle. GCC doesn't list armv5tej but it does accept it
https://godbolt.org/z/cazcbfjGY. For armv6j clang doesn't list it but it
also does accept it https://godbolt.org/z/aEKfnojTY.

llvm's support for jazelle is a single instruction "bxj" which is a branch
exchange, but for jazelle (
https://developer.arm.com/documentation/dui0473/j/arm-and-thumb-instructions/bxj
).
Which makes some sense. If you had that one instruction you could compile
99% of the code with public toolchains, then the rest with some tool from
Arm, I guess. GCC appears to have done the same.

So targets wise I think things are fine as is unless a v6 enthusiast tries
to use clang sometime.

For jazelle the bxj instruction appears to be present on non jazelle
processors, and can be used as a normal bx in ThumbEE. The code to
implement it and the armv5tej target is minimal and compact so I think we
keep it for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133109

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

[PATCH] D133941: [clang][Interp] Record item types in InterpStack

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Can you clarify what the intent of this patch is?  Perhaps I'm just being slow 
today, but I don't really get the intent here.




Comment at: clang/lib/AST/Interp/InterpStack.h:135
+  return PT_Ptr;
+else if (std::is_same::value || std::is_same::value)
+  return PT_Bool;

Each of these ALSO need to be 'if constexpr' I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133941

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

[PATCH] D133941: [clang][Interp] Record item types in InterpStack

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/Interp/InterpStack.h:161
+  return PT_Uint64;
+assert(false);
+  }

llvm_unreachable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133941

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

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/Interp/Context.cpp:134
+llvm::Optional Context::sizeofType(QualType T) const {
+  if (llvm::Optional ArgT = classify(T))
+return primSize(*ArgT);

This function seems pretty large for something you should be able to pick up 
via `ASTContext::getTypeSize` or `ASTContext::getTypeSizeInChars` or 
`ASTContext::getTypeSizeInCharsIfKnown`.



Comment at: clang/lib/AST/Interp/Context.cpp:149
+
+  return {};
+}

Typically, I prefer we use llvm::None here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133934

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

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
barcisz requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133942

Files:
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.h

Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
@@ -46,6 +46,17 @@
   DeclSpec::TQ Qualifier,
   QualifierTarget CT = QualifierTarget::Pointee,
   QualifierPolicy CP = QualifierPolicy::Left);
+
+/// \brief Adds a statement to be executed right after this statement .
+/// Is designed for taking potential comments or statements in the same line
+/// into account. The statement should not be an expression that's part of
+/// another statement. The statement range should include the terminator
+/// (semicolon).
+llvm::SmallVector
+addSubsequentStatement(SourceRange stmtRangeWithTerminator,
+   const Stmt &parentStmt, llvm::StringRef nextStmt,
+   ASTContext &context);
+
 } // namespace fixit
 } // namespace utils
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -223,6 +223,147 @@
 
   return None;
 }
+
+static std::string getIndent(SourceLocation sLoc, ASTContext& context) {
+  auto& SM = context.getSourceManager();
+
+  const auto sLocLineNo = getLineNumber(sLoc, SM);
+
+  auto indentation_template = tooling::fixit::internal::getText(
+  CharSourceRange::getCharRange(SourceRange(
+  SM.translateLineCol(SM.getFileID(sLoc), sLocLineNo, 1), sLoc)),
+  context);
+
+  std::string indentation;
+  indentation.reserve(indentation_template.size());
+  std::transform(
+  indentation_template.begin(),
+  indentation_template.end(),
+  std::back_inserter(indentation),
+  [](char c) { return isspace(c) ? c : ' '; });
+  return indentation;
+}
+
+static llvm::SmallVector addSubsequentStatement(
+SourceRange stmtRangeWithTerminator,
+const Stmt& parentStmt,
+llvm::StringRef nextStmt,
+ASTContext& context) {
+  auto& SM = context.getSourceManager();
+  auto langOpts = context.getLangOpts();
+
+  const auto stmtEndLineNo =
+  getLineNumber(stmtRangeWithTerminator.getEnd(), SM);
+
+  // Find the first token's data for which the next token is
+  // either a line apart or is not a comment
+  SourceLocation lastTokenEndLoc =
+  stmtRangeWithTerminator.getEnd().getLocWithOffset(1);
+  auto lastTokenLine = stmtEndLineNo;
+  bool insertNewLine = true;
+  while (true) {
+llvm::Optional tokenOption = findNextTokenIncludingComments(
+lastTokenEndLoc.getLocWithOffset(-1), SM, langOpts);
+if (!tokenOption) {
+  return llvm::SmallVector();
+}
+if (tokenOption->is(tok::eof)) {
+  insertNewLine = false;
+  break;
+}
+const auto tokenBeginLineNo = getLineNumber(tokenOption->getLocation(), SM);
+
+if (tokenOption->isNot(tok::comment)) {
+  insertNewLine = tokenBeginLineNo != stmtEndLineNo;
+  break;
+}
+if (tokenBeginLineNo > lastTokenLine) {
+  break;
+}
+
+lastTokenEndLoc = tokenOption->getEndLoc();
+lastTokenLine = getLineNumber(tokenOption->getEndLoc(), SM);
+  }
+
+  bool isEnclosedWithBrackets =
+  parentStmt.getStmtClass() == Stmt::CompoundStmtClass;
+
+  // Generating the FixItHint
+  // There are 5 scenarios that we have to take into account:
+  // 1. The statement is enclosed in brackets but the next statement is
+  //in the same line - insert the new statement right after the previous one
+  // 2. The statement is not enclosed in brackets and the next statement is
+  //in the same line - same as 1. and enclose both statements in brackets
+  //on the same line
+  // 3. The statement is enclosed in brackets and the next statement is
+  //on subsequent lines - skip all the comments in this line
+  // 4. The statement is not enclosed in brackets but the next statement is on
+  //subsequent lines - same as 3. and enclose the statements with
+  //google-style multiline brackets (opening bracket right after the parent
+  //statement and closing bracket on a new line after the new statement).
+  // 5. The statement is not enclosed in brackets but the next statement is on
+  //subsequent lines and the main statement is before an else token - same
+  //as 4. but the closing bracket is put on the same line as the else
+  //statement
+  if (!insertNewLin

[PATCH] D133942: Clang tidy utility to generate a fix hint for a subsequent expression to the existing one

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460405.
barcisz added a comment.

Use the new Lexer option to include comments while finding next token


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133942

Files:
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
  clang-tools-extra/clang-tidy/utils/FixItHintUtils.h

Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.h
@@ -46,6 +46,17 @@
   DeclSpec::TQ Qualifier,
   QualifierTarget CT = QualifierTarget::Pointee,
   QualifierPolicy CP = QualifierPolicy::Left);
+
+/// \brief Adds a statement to be executed right after this statement .
+/// Is designed for taking potential comments or statements in the same line
+/// into account. The statement should not be an expression that's part of
+/// another statement. The statement range should include the terminator
+/// (semicolon).
+llvm::SmallVector
+addSubsequentStatement(SourceRange stmtRangeWithTerminator,
+   const Stmt &parentStmt, llvm::StringRef nextStmt,
+   ASTContext &context);
+
 } // namespace fixit
 } // namespace utils
 } // namespace tidy
Index: clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/FixItHintUtils.cpp
@@ -223,6 +223,147 @@
 
   return None;
 }
+
+static std::string getIndent(SourceLocation sLoc, ASTContext& context) {
+  auto& SM = context.getSourceManager();
+
+  const auto sLocLineNo = getLineNumber(sLoc, SM);
+
+  auto indentation_template = tooling::fixit::internal::getText(
+  CharSourceRange::getCharRange(SourceRange(
+  SM.translateLineCol(SM.getFileID(sLoc), sLocLineNo, 1), sLoc)),
+  context);
+
+  std::string indentation;
+  indentation.reserve(indentation_template.size());
+  std::transform(
+  indentation_template.begin(),
+  indentation_template.end(),
+  std::back_inserter(indentation),
+  [](char c) { return isspace(c) ? c : ' '; });
+  return indentation;
+}
+
+static llvm::SmallVector addSubsequentStatement(
+SourceRange stmtRangeWithTerminator,
+const Stmt& parentStmt,
+llvm::StringRef nextStmt,
+ASTContext& context) {
+  auto& SM = context.getSourceManager();
+  auto langOpts = context.getLangOpts();
+
+  const auto stmtEndLineNo =
+  getLineNumber(stmtRangeWithTerminator.getEnd(), SM);
+
+  // Find the first token's data for which the next token is
+  // either a line apart or is not a comment
+  SourceLocation lastTokenEndLoc =
+  stmtRangeWithTerminator.getEnd().getLocWithOffset(1);
+  auto lastTokenLine = stmtEndLineNo;
+  bool insertNewLine = true;
+  while (true) {
+llvm::Optional tokenOption = Lexer::findNextToken(
+lastTokenEndLoc.getLocWithOffset(-1), SM, langOpts, true);
+if (!tokenOption) {
+  return llvm::SmallVector();
+}
+if (tokenOption->is(tok::eof)) {
+  insertNewLine = false;
+  break;
+}
+const auto tokenBeginLineNo = getLineNumber(tokenOption->getLocation(), SM);
+
+if (tokenOption->isNot(tok::comment)) {
+  insertNewLine = tokenBeginLineNo != stmtEndLineNo;
+  break;
+}
+if (tokenBeginLineNo > lastTokenLine) {
+  break;
+}
+
+lastTokenEndLoc = tokenOption->getEndLoc();
+lastTokenLine = getLineNumber(tokenOption->getEndLoc(), SM);
+  }
+
+  bool isEnclosedWithBrackets =
+  parentStmt.getStmtClass() == Stmt::CompoundStmtClass;
+
+  // Generating the FixItHint
+  // There are 5 scenarios that we have to take into account:
+  // 1. The statement is enclosed in brackets but the next statement is
+  //in the same line - insert the new statement right after the previous one
+  // 2. The statement is not enclosed in brackets and the next statement is
+  //in the same line - same as 1. and enclose both statements in brackets
+  //on the same line
+  // 3. The statement is enclosed in brackets and the next statement is
+  //on subsequent lines - skip all the comments in this line
+  // 4. The statement is not enclosed in brackets but the next statement is on
+  //subsequent lines - same as 3. and enclose the statements with
+  //google-style multiline brackets (opening bracket right after the parent
+  //statement and closing bracket on a new line after the new statement).
+  // 5. The statement is not enclosed in brackets but the next statement is on
+  //subsequent lines and the main statement is before an else token - same
+  //as 4. but the closing bracket is put on the same line as the else
+  //statement
+  if (!insertNewLine) {
+if (isEnclos

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki updated this revision to Diff 460407.
yusuke-kadowaki marked 3 inline comments as done.
yusuke-kadowaki added a comment.

Implementation done except for the `Leave` option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/WhitespaceManager.cpp
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/FormatTestComments.cpp

Index: clang/unittests/Format/FormatTestComments.cpp
===
--- clang/unittests/Format/FormatTestComments.cpp
+++ clang/unittests/Format/FormatTestComments.cpp
@@ -2858,6 +2858,187 @@
"int a; //\n");
 }
 
+TEST_F(FormatTestComments, AlignTrailingCommentsAcrossEmptyLines) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignTrailingComments.Kind = FormatStyle::TCAS_Always;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  verifyFormat("#include \"a.h\"  // simple\n"
+   "\n"
+   "#include \"aa.h\" // example case\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "\n"
+   "#include \"aa.h\"  // two empty lines\n"
+   "\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // one\n"
+   "#include \"aa.h\" // empty line\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align trailing comments\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\" // across a line without comment\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"   // align across\n"
+   "#include \"a.h\"\n"
+   "#include \"aa.h\"  // two lines without comment\n"
+   "#include \"a.h\"\n"
+   "#include \"aaa.h\" // in a row\n",
+   Style);
+
+  verifyFormat("#include \"a.h\"  // align\n"
+   "#include \"aa.h\" // comment\n"
+   "#include \"aaa.h\"// blocks\n"
+   "#include \"a.h\"\n"
+   "#include \".h\"   // across\n"
+   "#include \"a.h\"  // a line without\n"
+   "#include \"aa.h\" // comment\n",
+   Style);
+
+  // Start of testing OverEmptyLines
+  Style.MaxEmptyLinesToKeep = 3;
+  Style.AlignTrailingComments.OverEmptyLines = 2;
+  // Cannot use verifyFormat here
+  // test::messUp removes all new lines which changes the logic
+  EXPECT_EQ("#include \"a.h\" // comment\n"
+"\n"
+"\n"
+"\n"
+"#include \"ab.h\"  // comment\n"
+"\n"
+"\n"
+"#include \"abcdefg.h\" // comment\n",
+format("#include \"a.h\" // comment\n"
+   "\n"
+   "\n"
+   "\n"
+   "#include \"ab.h\" // comment\n"
+   "\n"
+   "\n"
+   "#include \"abcdefg.h\" // comment\n",
+   Style));
+
+  Style.MaxEmptyLinesToKeep = 1;
+  Style.AlignTrailingComments.OverEmptyLines = 1;
+  // End of testing OverEmptyLines
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"int a;  // long\n"
+"// long\n"
+"\n"
+"// long",
+format("int ab; // line\n"
+   "int a; // long long\n"
+   "\n"
+   "// long",
+   Style));
+
+  Style.ColumnLimit = 15;
+  EXPECT_EQ("int ab; // line\n"
+"\n"
+"int a;  // long\n"
+"// long\n",
+format("int ab; // line\n"
+   "\n"
+   "int a; // long long\n",
+   Style));
+
+  Style.ColumnLimit = 30;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+"int bar =\n"
+"1234;  // This is a very\n"
+"   // long comment\n"
+"   // which is wrapped\n"
+"   // arround.\n"
+"\n"
+"int x = 2; // Is this still\n"
+"   // aligned?\n",
+format("int foo = 12345; // comment\n"
+   "int bar = 1234; // This is a very long comment\n"
+   "// which is wrapped arround.\n"
+   "\n"
+   "int x = 2; // Is this still aligned?\n",
+   Style));
+
+  Style.ColumnLimit = 35;
+  EXPECT_EQ("int foo = 12345; // comment\n"
+

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki added inline comments.



Comment at: clang/include/clang/Format/Format.h:428
+
+bool operator==(const TrailingCommentsAlignmentStyle &R) const {
+  return Kind == R.Kind && OverEmptyLines == R.OverEmptyLines;

MyDeveloperDay wrote:
> yusuke-kadowaki wrote:
> > MyDeveloperDay wrote:
> > > yusuke-kadowaki wrote:
> > > > > I don't understand the need for state as a struct could have multiple 
> > > > > options (as enums) each enum should have a state that means "Leave"
> > > > 
> > > > @MyDeveloperDay 
> > > > Without having state, how can this be implemented?
> > > bool Enabled  =  (Kind != FormatStyle::TCAS_Leave)
> > Oh ok so I think we are on the same page.
> do you need this?
Yes. Without this it does not compile.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D133109: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M

[Renato Golin via Phabricator via cfe-commits]

rengolin added a comment.

I think we can safely say that we care less about jazelle than we care about 
armv1/2/3, so feel free to ignore that, too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133109

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

[PATCH] D133634: [clang] Allow vector of BitInt

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

There was no motivation for disabling this other than not wanting/caring enough 
to define the ABI and writing sufficient tests.  While I don't think there is 
sufficient testing here around ABI/etc, I have no problem with this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133634

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

[PATCH] D133941: [clang][Interp] Record item types in InterpStack

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

tbaeder added a comment.

In D133941#3792330 , @erichkeane 
wrote:

> Can you clarify what the intent of this patch is?  Perhaps I'm just being 
> slow today, but I don't really get the intent here.

Consider:

  push(...);
  (lots of stuff)
  pop();

currently this would just work and give you an integer, but they value wouldn't 
make any sense. This patch would assert here since the value on the stack is 
not an integer, it's a pointer.
It basically adds a bit of type-safety back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133941

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

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

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

tbaeder updated this revision to Diff 460409.

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

https://reviews.llvm.org/D133934

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

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -72,10 +72,30 @@
 constexpr const int* getIntPointer() {
   return &m;
 }
-//static_assert(getIntPointer() == &m, ""); TODO
-//static_assert(*getIntPointer() == 10, ""); TODO
+static_assert(getIntPointer() == &m, "");
+static_assert(*getIntPointer() == 10, "");
 
 constexpr int gimme(int k) {
   return k;
 }
-// static_assert(gimme(5) == 5, ""); TODO
+static_assert(gimme(5) == 5, "");
+
+namespace SizeOf {
+  constexpr int soint = sizeof(int);
+  constexpr int souint = sizeof(unsigned int);
+  static_assert(soint == souint, "");
+
+  static_assert(sizeof(&soint) == sizeof(void*), "");
+  static_assert(sizeof(&soint) == sizeof(nullptr), "");
+
+  static_assert(sizeof(long) == sizeof(unsigned long), "");
+  static_assert(sizeof(char) == sizeof(unsigned char), "");
+
+  constexpr int N = 4;
+  constexpr int arr[N] = {1,2,3,4};
+  static_assert(sizeof(arr) == N * sizeof(int), "");
+  static_assert(sizeof(arr) == N * sizeof(arr[0]), "");
+
+  constexpr bool arrB[N] = {true, true, true, true};
+  static_assert(sizeof(arrB) == N * sizeof(bool), "");
+};
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -427,11 +427,11 @@
 // TODO: Expand this to handle casts between more types.
 
 def FromCastTypeClass : TypeClass {
-  let Types = [Uint32, Sint32, Bool];
+  let Types = [Uint8, Sint8, Uint16, Sint16, Uint32, Sint32, Uint64, Sint64, Bool];
 }
 
 def ToCastTypeClass : TypeClass {
-  let Types = [Uint32, Sint32, Bool];
+  let Types = [Uint8, Sint8, Uint16, Sint16, Uint32, Sint32, Uint64, Sint64, Bool];
 }
 
 def Cast: Opcode {
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -80,6 +80,7 @@
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
   bool VisitConstantExpr(const ConstantExpr *E);
+  bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -281,7 +281,18 @@
 }
 
 template 
-bool ByteCodeExprGen::discard(const Expr *E) {
+bool ByteCodeExprGen::VisitUnaryExprOrTypeTraitExpr(
+const UnaryExprOrTypeTraitExpr *E) {
+
+  if (E->getKind() == UETT_SizeOf) {
+QualType ArgType = E->getTypeOfArgument();
+return this->emitConst(E, Ctx.getASTContext().getTypeSize(ArgType));
+  }
+
+  return false;
+}
+
+template  bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
 }
Index: clang/lib/AST/Interp/Boolean.h
===
--- clang/lib/AST/Interp/Boolean.h
+++ clang/lib/AST/Interp/Boolean.h
@@ -48,6 +48,10 @@
   Boolean operator~() const { return Boolean(true); }
 
   explicit operator unsigned() const { return V; }
+  explicit operator int8_t() const { return V; }
+  explicit operator uint8_t() const { return V; }
+  explicit operator int16_t() const { return V; }
+  explicit operator uint16_t() const { return V; }
   explicit operator int64_t() const { return V; }
   explicit operator uint64_t() const { return V; }
   explicit operator int() const { return V; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

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

tbaeder added a comment.

Didn't now I could just get that information from the ASTContext, nice. Thanks.


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

https://reviews.llvm.org/D133934

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

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:289
+QualType ArgType = E->getTypeOfArgument();
+return this->emitConst(E, Ctx.getASTContext().getTypeSize(ArgType));
+  }

You probably want `getTypeSizeInChars`. `getTypeSize` returns it in bits, so 
this would give the wrong answer.  You should probably add some tests to make 
sure the values you are getting back are sane (like `char` is 1, etc).


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

https://reviews.llvm.org/D133934

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

[PATCH] D133804: Cuda Check for ignored return errors from api calls to cuda

[Bartłomiej Cieślar via Phabricator via cfe-commits]

barcisz updated this revision to Diff 460410.
barcisz added a comment.

Use header guards instead of pragma


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133804

Files:
  clang-tools-extra/-
  clang-tools-extra/clang-tidy/cuda/CMakeLists.txt
  clang-tools-extra/clang-tidy/cuda/CudaTidyModule.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.cpp
  clang-tools-extra/clang-tidy/cuda/UnsafeApiCallCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cuda/unsafe-api-call.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda.h
  clang-tools-extra/test/clang-tidy/checkers/Inputs/Headers/cuda/cuda_runtime.h
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
  
clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu

Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-macro-handler.cu
@@ -0,0 +1,96 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'CUDA_HANDLER'}] \
+// RUN: }" \
+// RUN:   -- -isystem %clang_tidy_headers -std=c++14
+#include 
+
+class DummyContainer {
+ public:
+  int* begin();
+  int* end();
+};
+
+#define DUMMY_CUDA_HANDLER(stmt) stmt
+#define CUDA_HANDLER(stmt) do {auto err = stmt;} while(0)
+#define API_CALL() do {cudaDeviceReset();} while(0)
+
+void errorCheck();
+void errorCheck(cudaError_t error);
+
+void bad() {
+  API_CALL();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // There isn't supposed to be a fix here since it's a macro call
+
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  errorCheck();
+
+  if (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  while (true)
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  do
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+  while(false);
+
+  switch (0) {
+case 0:
+  cudaDeviceReset();
+  // CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+  // CHECK-FIXES:  {{^}}  CUDA_HANDLER(cudaDeviceReset());{{$}}
+  }
+
+  for(
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+;
+cudaDeviceReset()
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset()){{$}}
+  ) cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}  ) CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  for(int i : DummyContainer())
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+
+  auto x = ({
+cudaDeviceReset();
+// CHECK-MESSAGES: :[[@LINE-1]]:{{[0-9]+}}: warning: Unchecked CUDA API call.
+// CHECK-FIXES:  {{^}}CUDA_HANDLER(cudaDeviceReset());{{$}}
+true;
+  });
+}
+
+int good() {
+  DUMMY_CUDA_HANDLER(cudaDeviceReset());
+
+  if (cudaDeviceReset()) {
+return 0;
+  }
+
+  switch (cudaDeviceReset()) {
+case cudaErrorInvalidValue: return 1;
+case cudaErrorMemoryAllocation: return 2;
+default: return 3;
+  }
+
+  auto err = ({cudaDeviceReset();});
+  // NOTE: We don't check that `errorCheck()` actually handles the error; we just assume it does.
+  errorCheck(cudaDeviceReset());
+}
Index: clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cuda/unsafe-api-call-function-handler.cu
@@ -0,0 +1,71 @@
+// RUN: %check_clang_tidy %s cuda-unsafe-api-call %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: cuda-unsafe-api-call.HandlerName, \
+// RUN:   value: 'cudaHandler'}, \
+// RUN:  {key: cuda-unsafe-api-call.AcceptedHandlers, \
+// RUN:   

[PATCH] D132131: [clang-format] Adds a formatter for aligning trailing comments over empty lines

[Yusuke Kadowaki via Phabricator via cfe-commits]

yusuke-kadowaki added inline comments.



Comment at: clang/docs/ClangFormatStyleOptions.rst:865
+
+  * ``TCAS_DontAlign`` (in configuration: ``DontAlign``)
+Don't align trailing comments.

HazardyKnusperkeks wrote:
> yusuke-kadowaki wrote:
> > HazardyKnusperkeks wrote:
> > > MyDeveloperDay wrote:
> > > > yusuke-kadowaki wrote:
> > > > > MyDeveloperDay wrote:
> > > > > > Is Don'tAlign the same as Leave that other options support  (for 
> > > > > > options it best if we try and use the same terminiology
> > > > > > 
> > > > > > Always,Never,Leave (if that fits)
> > > > > IMHO, `Leave` probably fits but `DontAlign`  is more straightforward. 
> > > > > Also `DontAlign` is used for other alignment styles like 
> > > > > `BracketAlignmentStyle` so it would be more natural.
> > > > Leave should mean do nothing.. I'm personally not a fan of DontAlign 
> > > > because obvious there should be a ' between the n and the t but I see 
> > > > we use it elsewhere
> > > > 
> > > > But actually now I see your DontAlign is effectively the as before 
> > > > (i.e. it will not add extra spaces) 
> > > > 
> > > > The point of Leave is what people have been asking for when we 
> > > > introduce a new option, that is we if possible add an option which 
> > > > means "Don't touch it at all"  i.e.  if I want to have
> > > > 
> > > > ```
> > > > int a;   //  abc
> > > > int b;   /// bcd
> > > > int c;// cde
> > > > ```
> > > > 
> > > > Then so be it
> > > > 
> > > > 
> > > Leave is a nice option, yes.
> > > I think it is complementary to `Dont`.
> > > 
> > > But maybe if the option is called `AlignTrailingComments` one may 
> > > interpret `Leave` not as "don't touch the position of a comment at all" 
> > > (what do we do, if the comment is outside of the column limit?), but as 
> > > "just don't touch them, when they are somewhat aligned". Just a thought.
> > > Leave should mean do nothing
> > 
> > Ok now I see what `Leave` means. 
> > But that should be another piece of work no? 
> > 
> > Of course me or someone can add the feature later (I don't really know how 
> > to implement that though at least for now) 
> > 
> > 
> Fine by me.
@HazardyKnusperkeks 
Is this complicated? If it's not I might as well do this.

Also it would be helpful if you could provide some implementation guidance. 
Sorry to ask this even though I haven't tried it myself yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132131

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

[PATCH] D83015: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.
Herald added a subscriber: StephenFan.
Herald added a project: All.

`LinkerIsLLD` isn't getting set when `--ld-path` is used. (It also isn't 
getting set when using `-fuse-ld=` with an absolute path.)

How do you imagine using an absolute path and telling clang "the linker is lld, 
you can assume it's built at the same rev as you" in this new world should look 
like? Pass both `-fuse-ld=lld` and `--ld-path=path/to/lld`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83015

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

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

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

tbaeder added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:289
+QualType ArgType = E->getTypeOfArgument();
+return this->emitConst(E, Ctx.getASTContext().getTypeSize(ArgType));
+  }

erichkeane wrote:
> You probably want `getTypeSizeInChars`. `getTypeSize` returns it in bits, so 
> this would give the wrong answer.  You should probably add some tests to make 
> sure the values you are getting back are sane (like `char` is 1, etc).
Yeah, right. I explicitly didn't compare with fixed values when writing the 
tests so I don't run into the usual problems, but they wouldn't caught this 
problem...


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

https://reviews.llvm.org/D133934

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

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

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

tbaeder updated this revision to Diff 460412.

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

https://reviews.llvm.org/D133934

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

Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -72,10 +72,33 @@
 constexpr const int* getIntPointer() {
   return &m;
 }
-//static_assert(getIntPointer() == &m, ""); TODO
-//static_assert(*getIntPointer() == 10, ""); TODO
+static_assert(getIntPointer() == &m, "");
+static_assert(*getIntPointer() == 10, "");
 
 constexpr int gimme(int k) {
   return k;
 }
-// static_assert(gimme(5) == 5, ""); TODO
+static_assert(gimme(5) == 5, "");
+
+namespace SizeOf {
+  constexpr int soint = sizeof(int);
+  constexpr int souint = sizeof(unsigned int);
+  static_assert(soint == souint, "");
+
+  static_assert(sizeof(&soint) == sizeof(void*), "");
+  static_assert(sizeof(&soint) == sizeof(nullptr), "");
+
+  static_assert(sizeof(long) == sizeof(unsigned long), "");
+  static_assert(sizeof(char) == sizeof(unsigned char), "");
+
+  constexpr int N = 4;
+  constexpr int arr[N] = {1,2,3,4};
+  static_assert(sizeof(arr) == N * sizeof(int), "");
+  static_assert(sizeof(arr) == N * sizeof(arr[0]), "");
+
+  constexpr bool arrB[N] = {true, true, true, true};
+  static_assert(sizeof(arrB) == N * sizeof(bool), "");
+
+  static_assert(sizeof(bool) == 1, "");
+  static_assert(sizeof(char) == 1, "");
+};
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -427,11 +427,11 @@
 // TODO: Expand this to handle casts between more types.
 
 def FromCastTypeClass : TypeClass {
-  let Types = [Uint32, Sint32, Bool];
+  let Types = [Uint8, Sint8, Uint16, Sint16, Uint32, Sint32, Uint64, Sint64, Bool];
 }
 
 def ToCastTypeClass : TypeClass {
-  let Types = [Uint32, Sint32, Bool];
+  let Types = [Uint8, Sint8, Uint16, Sint16, Uint32, Sint32, Uint64, Sint64, Bool];
 }
 
 def Cast: Opcode {
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -80,6 +80,7 @@
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
   bool VisitConstantExpr(const ConstantExpr *E);
+  bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
 
 protected:
   bool visitExpr(const Expr *E) override;
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -281,7 +281,19 @@
 }
 
 template 
-bool ByteCodeExprGen::discard(const Expr *E) {
+bool ByteCodeExprGen::VisitUnaryExprOrTypeTraitExpr(
+const UnaryExprOrTypeTraitExpr *E) {
+
+  if (E->getKind() == UETT_SizeOf) {
+QualType ArgType = E->getTypeOfArgument();
+return this->emitConst(
+E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity());
+  }
+
+  return false;
+}
+
+template  bool ByteCodeExprGen::discard(const Expr *E) {
   OptionScope Scope(this, /*NewDiscardResult=*/true);
   return this->Visit(E);
 }
Index: clang/lib/AST/Interp/Boolean.h
===
--- clang/lib/AST/Interp/Boolean.h
+++ clang/lib/AST/Interp/Boolean.h
@@ -48,6 +48,10 @@
   Boolean operator~() const { return Boolean(true); }
 
   explicit operator unsigned() const { return V; }
+  explicit operator int8_t() const { return V; }
+  explicit operator uint8_t() const { return V; }
+  explicit operator int16_t() const { return V; }
+  explicit operator uint16_t() const { return V; }
   explicit operator int64_t() const { return V; }
   explicit operator uint64_t() const { return V; }
   explicit operator int() const { return V; }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133945: [clang][ASTImporter] Continue with slow lookup in DeclContext::localUncachedLookup when regular lookup fails

[Michael Buch via Phabricator via cfe-commits]

Michael137 created this revision.
Michael137 added reviewers: aprantl, martong.
Herald added a subscriber: rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a reviewer: shafik.
Herald added a project: All.
Michael137 requested review of this revision.
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

The uncached lookup is mainly used in the ASTImporter/LLDB code-path
where we're not allowed to load from external storage. When importing
a FieldDecl with a DeclContext that had no external visible storage
(but came from a Clang module or PCH) the above call to `lookup(Name)`
the regular lookup fails because:

1. `DeclContext::buildLookup` doesn't set `LookupPtr` for decls that came from 
a module
2. LLDB doesn't use the `SharedImporterState`

In such a case but we would never go on to the "slow" path of iterating
through the decls in the DeclContext. In some cases this means that
`ASTNodeImporter::VisitFieldDecl` ends up importing a decl into the
`DeclContext` a second time.

The patch removes the short-circuit in the case where we don't find
any decls via the regular lookup.

**Tests**

- Un-skip the failing LLDB API tests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133945

Files:
  clang/lib/AST/DeclBase.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py


Index: 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.


Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   //

[PATCH] D133945: [clang][ASTImporter] Continue with slow lookup in DeclContext::localUncachedLookup when regular lookup fails

[Michael Buch via Phabricator via cfe-commits]

Michael137 added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: clang/lib/AST/DeclBase.cpp:1781
   if (Name && !hasLazyLocalLexicalLookups() &&
   !hasLazyExternalLexicalLookups()) {
 if (StoredDeclsMap *Map = LookupPtr) {

Could merge the two if-blocks now I suppose


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

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

[PATCH] D132461: [clang-tidy] Add cppcoreguidelines-avoid-do-while check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 460419.
carlosgalvezp added a comment.

Adjust warning message for consistency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132461

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidDoWhileCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-do-while.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-do-while.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-do-while.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-do-while.cpp
@@ -0,0 +1,88 @@
+// RUN: %check_clang_tidy -check-suffixes=DEFAULT   %s cppcoreguidelines-avoid-do-while %t
+// RUN: %check_clang_tidy -check-suffixes=IGNORE-MACROS %s cppcoreguidelines-avoid-do-while %t -- -config='{CheckOptions: [{key: cppcoreguidelines-avoid-do-while.IgnoreMacros, value: true}]}'
+
+#define FOO(x) \
+  do { \
+  } while (x != 0)
+
+#define BAR_0(x) \
+  do { \
+bar(x); \
+  } while (0)
+
+#define BAR_FALSE(x) \
+  do { \
+bar(x); \
+  } while (false)
+
+void bar(int);
+int baz();
+
+void foo()
+{
+// CHECK-MESSAGES-IGNORE-MACROS: :[[@LINE+2]]:5: warning: avoid do-while loops [cppcoreguidelines-avoid-do-while]
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: avoid do-while loops [cppcoreguidelines-avoid-do-while]
+do {
+
+} while(0);
+
+// CHECK-MESSAGES-IGNORE-MACROS: :[[@LINE+2]]:5: warning: avoid do-while loops
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: avoid do-while loops
+do {
+
+} while(1);
+
+// CHECK-MESSAGES-IGNORE-MACROS: :[[@LINE+2]]:5: warning: avoid do-while loops
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: avoid do-while loops
+do {
+
+} while(-1);
+
+// CHECK-MESSAGES-IGNORE-MACROS: :[[@LINE+2]]:5: warning: avoid do-while loops
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: avoid do-while loops
+do {
+
+} while(false);
+
+// CHECK-MESSAGES-IGNORE-MACROS: :[[@LINE+2]]:5: warning: avoid do-while loops
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: avoid do-while loops
+do {
+
+} while(true);
+
+// CHECK-MESSAGES-IGNORE-MACROS: :[[@LINE+3]]:5: warning: avoid do-while loops
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+2]]:5: warning: avoid do-while loops
+int x1{0};
+do {
+  x1 = baz();
+} while (x1 > 0);
+
+// CHECK-MESSAGES-IGNORE-MACROS: :[[@LINE+2]]:5: warning: avoid do-while loops
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: avoid do-while loops
+do {
+
+} while (x1 != 0);
+
+// CHECK-MESSAGES-IGNORE-MACROS: :[[@LINE+3]]:5: warning: avoid do-while loops
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+2]]:5: warning: avoid do-while loops
+constexpr int x2{0};
+do {
+
+} while (x2);
+
+// CHECK-MESSAGES-IGNORE-MACROS: :[[@LINE+3]]:5: warning: avoid do-while loops
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+2]]:5: warning: avoid do-while loops
+constexpr bool x3{false};
+do {
+
+} while (x3);
+
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: avoid do-while loops
+FOO(x1);
+
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: avoid do-while loops
+BAR_0(x1);
+
+// CHECK-MESSAGES-DEFAULT: :[[@LINE+1]]:5: warning: avoid do-while loops
+BAR_FALSE(x1);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -177,6 +177,7 @@
`concurrency-mt-unsafe `_,
`concurrency-thread-canceltype-asynchronous `_,
`cppcoreguidelines-avoid-const-or-ref-data-members `_,
+   `cppcoreguidelines-avoid-do-while `_,
`cppcoreguidelines-avoid-goto `_,
`cppcoreguidelines-avoid-non-const-global-variables `_,
`cppcoreguidelines-init-variables `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-do-while.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-do-while.rst
@@ -0,0 +1,38 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-do-while
+
+cppcoreguidelines-avoid-do-while
+
+
+Warns when using ``do-while`` loops. They are less readable than plain ``while``
+loops, since the termination condition is at the end and the condition is not
+checked prior to the first iteration. Thi

[PATCH] D133945: [clang][ASTImporter] Continue with slow lookup in DeclContext::localUncachedLookup when regular lookup fails

[Michael Buch via Phabricator via cfe-commits]

Michael137 updated this revision to Diff 460420.
Michael137 added a comment.

- Merge if-blocks
- Reword commit


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

Files:
  clang/lib/AST/DeclBase.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py


Index: 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,13 +1771,11 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
-  }
+if (!Results.empty())
+  return;
 
-  // If we have a lookup table, check there first. Maybe we'll get lucky.
-  // FIXME: Should we be checking these flags on the primary context?
-  if (Name && !hasLazyLocalLexicalLookups() &&
-  !hasLazyExternalLexicalLookups()) {
+// If we have a lookup table, check there first. Maybe we'll get lucky.
+// FIXME: Should we be checking these flags on the primary context?
 if (StoredDeclsMap *Map = LookupPtr) {
   StoredDeclsMap::iterator Pos = Map->find(Name);
   if (Pos != Map->end()) {


Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,13 +1771,11 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
-  }
+if (!Results.empty())
+  return;
 
-  // If we have a lookup table, check there first. Maybe we'll get lucky.
-  // FIXME: Should we be checking these flags on the primary context?
-  if (Name && !hasLazyLocalLexicalLookups() &&
-  !hasLazyExternalLexicalLookups()) {
+// If we have a lookup table, check there first. Maybe we'll get lucky.
+// FIXME: Should we be checking these flags on the primary context?
 if (StoredDeclsMap *Map = LookupPtr) {
   StoredDeclsMap::iterator Pos = Map->find(Name);
   if (Pos != Map->end()) {
___
cf

[PATCH] D123630: Remove connection between 'ffast-math' and 'ffp-contract'.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a few nits.




Comment at: clang/docs/UsersManual.rst:1411
 
+   Note: ``-ffast-math`` causes ``crtfastmath.o`` to be linked with code.
+





Comment at: clang/docs/UsersManual.rst:1460
+ ``-ffp-contract`` setting is determined by the default value of
+ ``-ffp-contract`.
+





Comment at: clang/docs/UsersManual.rst:1694
+``-ffast-math`` and ``-funsafe-math-optimizations`` cause ``crtfastmath.o`` to 
be
+linked,  which adds a static constructor that sets the FTZ/DAZ bits in MXCSR,
+affecting not only the current compilation unit but all static and shared




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

https://reviews.llvm.org/D123630

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

[PATCH] D133853: [AST] Add msvc-specific C++11 attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for the patch! I'm wondering what the goal is for these changes. We 
don't typically add attributes to Clang that don't have any effect unless 
there's a very compelling reason to do so. Are you intending to add semantics 
for these attributes in follow-up patches?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133853

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

[PATCH] D133807: Update Unicode to 15.0

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

Structurally, these changes look like what I would expect. I didn't try to 
validate any of the code point ranges.

Are there useful tests that could be modified or added in order to validate 
(probably on a spot check basis) Unicode 15 support for regression purposes? 
For example, adding a test for `\N{}` for one of the newly added names or some 
case folding tests for new characters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133807

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

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290
+return this->emitConst(
+E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity());
+  }

I notice that `HandleSizeof` special cases `void` and function types. 



Comment at: clang/test/AST/Interp/literals.cpp:83
+
+namespace SizeOf {
+  constexpr int soint = sizeof(int);

Test for `void` and a function type too.


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

https://reviews.llvm.org/D133934

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

[PATCH] D133948: [clang][C++20] Fix clang/clangd assert/crash after compilation errors

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin created this revision.
DmitryPolukhin added reviewers: Tyker, rsmith, aaron.ballman.
DmitryPolukhin added a project: clang.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
DmitryPolukhin requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.

After compilation errors, expression a transformation result may not be usable.
It triggers an assert in RemoveNestedImmediateInvocation and SIGSEGV in case of
builds without asserts. This issue significantly affects clangd because source
may not be valid during typing. Tests cases that I attached was reduce from huge
C++ translation unit.

Test Plan: check-clang


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133948

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp


Index: clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: not %clang_cc1 -fsyntax-only -verify -std=gnu++20 -ferror-limit 19 %s
+// Creduced test case for the crash in RemoveNestedImmediateInvocation after 
compliation errros.
+
+a, class b {   template < typename c>  
   consteval b(c
+} template  using d = b;
+auto e(d<>) -> int:;
+}
+f
+}
+g() {
+auto h = "":(::i(e(h))
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17624,9 +17624,13 @@
 Transformer.AllowSkippingFirstCXXConstructExpr = false;
 
   ExprResult Res = Transformer.TransformExpr(It->getPointer()->getSubExpr());
-  assert(Res.isUsable());
-  Res = SemaRef.MaybeCreateExprWithCleanups(Res);
-  It->getPointer()->setSubExpr(Res.get());
+  // The result may not be usable in case of previous compilation errors.
+  // In this case evaluation of the expression may result in crash so just
+  // don't do anything further with the result.
+  if(Res.isUsable()) {
+Res = SemaRef.MaybeCreateExprWithCleanups(Res);
+It->getPointer()->setSubExpr(Res.get());
+  }
 }
 
 static void


Index: clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: not %clang_cc1 -fsyntax-only -verify -std=gnu++20 -ferror-limit 19 %s
+// Creduced test case for the crash in RemoveNestedImmediateInvocation after compliation errros.
+
+a, class b {   template < typename c> consteval b(c
+} template  using d = b;
+auto e(d<>) -> int:;
+}
+f
+}
+g() {
+auto h = "":(::i(e(h))
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17624,9 +17624,13 @@
 Transformer.AllowSkippingFirstCXXConstructExpr = false;
 
   ExprResult Res = Transformer.TransformExpr(It->getPointer()->getSubExpr());
-  assert(Res.isUsable());
-  Res = SemaRef.MaybeCreateExprWithCleanups(Res);
-  It->getPointer()->setSubExpr(Res.get());
+  // The result may not be usable in case of previous compilation errors.
+  // In this case evaluation of the expression may result in crash so just
+  // don't do anything further with the result.
+  if(Res.isUsable()) {
+Res = SemaRef.MaybeCreateExprWithCleanups(Res);
+It->getPointer()->setSubExpr(Res.get());
+  }
 }
 
 static void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133945: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

[Michael Buch via Phabricator via cfe-commits]

Michael137 updated this revision to Diff 460429.
Michael137 added a comment.

- Undo incorrect previous change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

Files:
  clang/lib/AST/DeclBase.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py


Index: 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ 
lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.


Index: lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
===
--- lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
+++ lldb/test/API/lang/cpp/gmodules/base-template-with-same-arg/TestBaseTemplateWithSameArg.py
@@ -30,7 +30,6 @@
 class TestBaseTemplateWithSameArg(TestBase):
 
 @add_test_categories(["gmodules"])
-@skipIf(bugnumber='rdar://96581048')
 def test_same_base_template_arg(self):
 self.build()
 
Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4924,9 +4924,9 @@
   FooDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
   EXPECT_EQ(FoundDecls.size(), 0u);
 
-  // Cannot find in the LookupTable of its LexicalDC (A).
+  // Finds via linear search of its LexicalDC (A).
   FooLexicalDC->getRedeclContext()->localUncachedLookup(FooName, FoundDecls);
-  EXPECT_EQ(FoundDecls.size(), 0u);
+  EXPECT_EQ(FoundDecls.size(), 1u);
 
   // Can't find in the list of Decls of the DC.
   EXPECT_EQ(findInDeclListOfDC(FooDC, FooName), nullptr);
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1771,7 +1771,8 @@
   if (!hasExternalVisibleStorage() && !hasExternalLexicalStorage() && Name) {
 lookup_result LookupResults = lookup(Name);
 Results.insert(Results.end(), LookupResults.begin(), LookupResults.end());
-return;
+if (!Results.empty())
+  return;
   }
 
   // If we have a lookup table, check there first. Maybe we'll get lucky.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133945: [clang][ASTImporter] DeclContext::localUncachedLookup: Continue lookup into decl chain when regular lookup fails

[Michael Buch via Phabricator via cfe-commits]

Michael137 added inline comments.



Comment at: clang/lib/AST/DeclBase.cpp:1781
   if (Name && !hasLazyLocalLexicalLookups() &&
   !hasLazyExternalLexicalLookups()) {
 if (StoredDeclsMap *Map = LookupPtr) {

Michael137 wrote:
> Could merge the two if-blocks now I suppose
Nevermind, not true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133945

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

[PATCH] D133249: [libc++] Documents details of the pre-commit CI.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/www/hacking.html:279
+
+  Some changes in Clang affect https://libcxx.llvm.org";>libc++,
+  for example:

ldionne wrote:
> This follows my previous comment, but we really don't want to encourage Clang 
> folks to start using a `.DELETE_ME` file for testing Clang changes. Indeed, 
> roughly 54/55 of our CI jobs don't use the just-built Clang. The only 
> relevant CI job for a Clang change is the Bootstrapping build. As a result, 
> using a `.DELETE_ME` file is extremely wasteful of  libc++ CI, since almost 
> all the CI jobs are completely irrelevant to the change being tested. 
> Furthermore, it will also provide a false sense of security for Clang folks, 
> who will think "hey but I ran the CI under all configurations", not 
> understanding the details.
> 
> So I think we need to set up libc++ testing for Clang changes, but we should 
> discourage folks from running the libc++ CI itself as a poor proxy for 
> testing Clang changes. Most of the jobs are just not useful at all from the 
> perspective of testing a Clang change.
> 
> So I'd rather not add this part of the documentation at all, to avoid 
> creating a problem for ourselves.
> So I'd rather not add this part of the documentation at all, to avoid 
> creating a problem for ourselves.

FWIW, I love the idea of setting up libc++ testing for Clang changes so we 
don't need a .DELETE_ME file. I think that's ultimately where we all would love 
to land. I'm fine either pulling these docs in anticipation of that work or 
leaving the docs here until that work is done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133249

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

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

[Erich Keane via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290
+return this->emitConst(
+E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity());
+  }

shafik wrote:
> I notice that `HandleSizeof` special cases `void` and function types. 
OOOH, DEFINITELY make sure you test function types here, and figure out how 
HandleSizeof does it.

BUT, I think 'void' should error/be an invalid before we get here?

ALSO, now that I think further, I'm sure @aaron.ballman has a bunch of 
runtime-evaluated sizeof examples to share around vlas.


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

https://reviews.llvm.org/D133934

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

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290
+return this->emitConst(
+E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity());
+  }

erichkeane wrote:
> shafik wrote:
> > I notice that `HandleSizeof` special cases `void` and function types. 
> OOOH, DEFINITELY make sure you test function types here, and figure out how 
> HandleSizeof does it.
> 
> BUT, I think 'void' should error/be an invalid before we get here?
> 
> ALSO, now that I think further, I'm sure @aaron.ballman has a bunch of 
> runtime-evaluated sizeof examples to share around vlas.
I was just about to comment about everyone's favorite horrible C++ extension. 
:-D We should make sure we properly reject code like:
```
void func() {
  int n = 12;
  constexpr int oofda = sizeof(int[n++]);
}
```
while accepting code like:
```
consteval int foo(int n) {
  return sizeof(int[n]);
}
constinit int var = foo(5);
```


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

https://reviews.llvm.org/D133934

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

[PATCH] D133948: [clang][C++20] Fix clang/clangd assert/crash after compilation errors

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 460437.
DmitryPolukhin added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133948

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp


Index: clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: not %clang_cc1 -fsyntax-only -verify -std=gnu++20 -ferror-limit 19 %s
+// Creduced test case for the crash in RemoveNestedImmediateInvocation after 
compliation errros.
+
+a, class b {   template < typename c>  
   consteval b(c
+} template  using d = b;
+auto e(d<>) -> int:;
+}
+f
+}
+g() {
+auto h = "":(::i(e(h))
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17624,9 +17624,13 @@
 Transformer.AllowSkippingFirstCXXConstructExpr = false;
 
   ExprResult Res = Transformer.TransformExpr(It->getPointer()->getSubExpr());
-  assert(Res.isUsable());
-  Res = SemaRef.MaybeCreateExprWithCleanups(Res);
-  It->getPointer()->setSubExpr(Res.get());
+  // The result may not be usable in case of previous compilation errors.
+  // In this case evaluation of the expression may result in crash so just
+  // don't do anything further with the result.
+  if (Res.isUsable()) {
+Res = SemaRef.MaybeCreateExprWithCleanups(Res);
+It->getPointer()->setSubExpr(Res.get());
+  }
 }
 
 static void


Index: clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/remove-nested-immediate-invocation-crash.cpp
@@ -0,0 +1,11 @@
+// RUN: not %clang_cc1 -fsyntax-only -verify -std=gnu++20 -ferror-limit 19 %s
+// Creduced test case for the crash in RemoveNestedImmediateInvocation after compliation errros.
+
+a, class b {   template < typename c> consteval b(c
+} template  using d = b;
+auto e(d<>) -> int:;
+}
+f
+}
+g() {
+auto h = "":(::i(e(h))
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -17624,9 +17624,13 @@
 Transformer.AllowSkippingFirstCXXConstructExpr = false;
 
   ExprResult Res = Transformer.TransformExpr(It->getPointer()->getSubExpr());
-  assert(Res.isUsable());
-  Res = SemaRef.MaybeCreateExprWithCleanups(Res);
-  It->getPointer()->setSubExpr(Res.get());
+  // The result may not be usable in case of previous compilation errors.
+  // In this case evaluation of the expression may result in crash so just
+  // don't do anything further with the result.
+  if (Res.isUsable()) {
+Res = SemaRef.MaybeCreateExprWithCleanups(Res);
+It->getPointer()->setSubExpr(Res.get());
+  }
 }
 
 static void
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D133583: [clang][ubsan] Fix __builtin_assume_aligned incorrect type descriptor and C++ object polymorphic address

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133583#3780460 , @yronglin wrote:

> Hi, follow D133202  , should I both fix 
> alignment in this patch or in another separate patch? (this seems have 
> different behavior with gcc https://godbolt.org/z/7dvM8zhnh )

I think that's a separate patch -- this one is fixing a mistake with the 
type-checking related crash fix and the other is about the behavior of the 
expression itself when it's valid.

The current patch looks reasonable to me, but I'd love a second opinion.




Comment at: clang/test/Sema/builtin-redecl.cpp:5-6
 
+#include 
+
 // Redeclaring library builtins is OK.

We can be tricky instead of including a header file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133583

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

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Alex Bradbury via Phabricator via cfe-commits]

asb added a comment.

I think the summary of our discussion on this was:

- The versioning confusion is unfortunate - ideally there would be discussion 
elsewhere at RVI on improving the situation (either ELF attributes to indicate 
extensions are experimental, or making that unnecessary via never using 1.0 
until something is ratified)
- But the above isn't a blocker to merging. As the extension is gated by the 
experimental flag, even if there are last minute changes the impact on users 
should be minimal / non-existent.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133443

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

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D133443#3792627 , @asb wrote:

> I think the summary of our discussion on this was:
>
> - The versioning confusion is unfortunate - ideally there would be discussion 
> elsewhere at RVI on improving the situation (either ELF attributes to 
> indicate extensions are experimental, or making that unnecessary via never 
> using 1.0 until something is ratified)
> - But the above isn't a blocker to merging. As the extension is gated by the 
> experimental flag, even if there are last minute changes the impact on users 
> should be minimal / non-existent.

Matches my takeaway.  I'm going to rebase this and add in a doc change which 
clearly notes the release candidate bit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133443

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

[PATCH] D133757: [clangd] Turn QueryDriverDatabase into a CompileCommandsAdjuster

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Kadir and I discussed this a bunch today. FWIW I agree with this patch: 
sticking the query-driver step in the middle of the pipeline is the right way 
to solve these bugs.
Putting it on the end and hoping to get away with it feels risky to me.




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:477
 std::make_unique(CDBOpts);
-BaseCDB = getQueryDriverDatabase(llvm::makeArrayRef(Opts.QueryDriverGlobs),
- std::move(BaseCDB));
+Extractor =
+getSystemIncludeExtractor(llvm::makeArrayRef(Opts.QueryDriverGlobs));

this is carefully preserving existing behavior: --query-driver has no effect 
when --compile_commands_from=lsp

I don't think that behavior is intended, we can remove it either here or in a 
followup patch



Comment at: clang-tools-extra/clangd/ClangdLSPServer.h:79
 
+  /// Methods exposed for testing.
+  llvm::Optional getCompileCommand(PathRef File);

(if the unit tests gets converted back to a lit test, we should probably remove 
this)



Comment at: clang-tools-extra/clangd/CompileCommands.cpp:310
+  //that needs to be invoked may come from the CompileFlags->Compiler key
+  //  - BEFORE resolveDriver() because that can mess up the driver path,
+  //e.g. changing gcc to /path/to/clang/bin/gcc

kadircet wrote:
> why is this necessary? i.e. are there cases where heuristics in clang-driver 
> today cannot identify the same set of includes we would extract from a system 
> installed `gcc` ?
> 
> because if we were to get rid of this requirement, we can just run this as an 
> extra arg adjuster after running command mangler, without changing the logic 
> in CommandMangler at all (and also rendering the underlying patch unnecessary 
> (i am still not sure if it's needed, but need to take a closer look there)).
I think Kadir is suggesting allowing a bug (handling of `gcc` + 
`--query-driver`) that we think is unimportant, in order to simplify the 
implementation.

After staring at this for a while, I think having the CommandMangler 
encapsulate all the ugly stuff we do here is the right *interface*, and that 
justifies the implementation being a bit ugly.



Comment at: clang-tools-extra/clangd/CompileCommands.h:56
 private:
+  std::unique_ptr SystemIncludeExtractor;
   Memoize> ResolvedDrivers;

caches notwithstanding, CommandMangler is a "simple struct" - I think this 
field should be public for consistency.
(And set manually rather than passed to detect() - there's no detection 
involved)



Comment at: clang-tools-extra/clangd/QueryDriverDatabase.cpp:318
 /// compilation database.
-class QueryDriverDatabase : public DelegatingCDB {
+class SystemIncludeExtractor : public CompileCommandsAdjuster {
 public:

this is renaming the class, but not the file, the flag, etc.
The actual name aside, such a rename should probably be more complete and a 
separate commit. If we keep the `--query-driver` flag we'll have at least two 
names, so we have to weigh that against the betterness of the name too.

(I agree that the current name is somewhat confusing. The new name is somewhat 
inaccurate: we're extracting from a compiler rather than the (operating?) 
system, and we extract more than includes. I prefer the new one on balance but 
maybe `ToolchainFlagsExtractor` or something would be even better...)



Comment at: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp:74
   FeatureModuleSet FeatureModules;
+  llvm::Optional Server;
 

The new destruction order looks much less obvious, even if we can get away with 
it at the moment.
Maybe add an accessor instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133757

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

[PATCH] D128440: [WebAssembly] Initial support for reference type funcref in clang

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ASTContext.cpp:954
 10, // wasm_externref,
+20, // wasm_funcref
 };

pmatos wrote:
> aaron.ballman wrote:
> > Where did this value come from?
> Unsure what you mean here. This is the address space number we also use on 
> the LLVM side.
> Unsure what you mean here. This is the address space number we also use on 
> the LLVM side.

That's exactly what I was looking for -- I wanted to make sure this agreed with 
the LLVM side of things and any public documentation about address spaces for 
WASM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128440

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

[PATCH] D83015: [Driver] Add --ld-path= and deprecate -fuse-ld=/abs/path and -fuse-ld=rel/path

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D83015#3792397 , @thakis wrote:

> `LinkerIsLLD` isn't getting set when `--ld-path` is used. (It also isn't 
> getting set when using `-fuse-ld=` with an absolute path.)
>
> How do you imagine using an absolute path and telling clang "the linker is 
> lld, you can assume it's built at the same rev as you" in this new world 
> should look like? Pass both `-fuse-ld=lld` and `--ld-path=path/to/lld`?

Yes, specify both options. -fuse-ld= specifies the flavor while --ld-path= 
specifies a path the Clang Driver does not want to detect.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83015

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

[PATCH] D133954: [clang-format] Fix template arguments in macros

[sstwcw via Phabricator via cfe-commits]

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

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

old

  #define FOO(typeName, realClass)   \
{\
  #typeName, foo < FooType>(new foo (#typeName))  \
}

new

  #define FOO(typeName, realClass)\
{ #typeName, foo(new foo(#typeName)) }

Previously, when an UnwrappedLine began with a hash in a macro
definition, the program incorrectly assumed the line was a preprocessor
directive.  It should be stringification.

The rule in spaceRequiredBefore was added in 8b5297117b.  Its purpose is
to add a space in an include directive.  It also added a space to a
template opener when the line began with a stringification hash.  So we
changed it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D133954

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/TokenAnnotator.h
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp
  clang/unittests/Format/TokenAnnotatorTest.cpp

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -284,6 +284,17 @@
   EXPECT_TOKEN(Tokens[13], tok::ampamp, TT_BinaryOperator);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsTemplatesInMacros) {
+  auto Tokens =
+  annotate("#define FOO(typeName) \\\n"
+   "  { #typeName, foo(new foo(#typeName)) }");
+  ASSERT_EQ(Tokens.size(), 27u) << Tokens;
+  EXPECT_TOKEN(Tokens[11], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[13], tok::greater, TT_TemplateCloser);
+  EXPECT_TOKEN(Tokens[17], tok::less, TT_TemplateOpener);
+  EXPECT_TOKEN(Tokens[19], tok::greater, TT_TemplateCloser);
+}
+
 TEST_F(TokenAnnotatorTest, UnderstandsWhitespaceSensitiveMacros) {
   FormatStyle Style = getLLVMStyle();
   Style.WhitespaceSensitiveMacros.push_back("FOO");
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -9790,6 +9790,9 @@
   verifyFormat("bool x = 5 < a::x;");
   verifyFormat("bool x = a < 4 ? a > 2 : false;");
   verifyFormat("bool x = f() ? a < 2 : a > 2;");
+  verifyFormat("#define FOO(typeName, realClass)   "
+   "\\\n"
+   "  { #typeName, foo(new foo(#typeName)) }");
 
   verifyGoogleFormat("A> a;");
   verifyGoogleFormat("A>> a;");
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -46,6 +46,8 @@
 
   /// Whether this \c UnwrappedLine is part of a preprocessor directive.
   bool InPPDirective;
+  /// Whether it is part of a macro body.
+  bool InMacroBody;
 
   bool MustBeDeclaration;
 
@@ -353,8 +355,8 @@
 };
 
 inline UnwrappedLine::UnwrappedLine()
-: Level(0), InPPDirective(false), MustBeDeclaration(false),
-  MatchingOpeningBlockLineIndex(kInvalidIndex) {}
+: Level(0), InPPDirective(false), InMacroBody(false),
+  MustBeDeclaration(false), MatchingOpeningBlockLineIndex(kInvalidIndex) {}
 
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -116,12 +116,14 @@
 TokenSource = this;
 Line.Level = 0;
 Line.InPPDirective = true;
+// InMacroBody gets set after the `#define x` part.
   }
 
   ~ScopedMacroState() override {
 TokenSource = PreviousTokenSource;
 ResetToken = Token;
 Line.InPPDirective = false;
+Line.InMacroBody = false;
 Line.Level = PreviousLineLevel;
   }
 
@@ -196,6 +198,7 @@
 Parser.Line = std::make_unique();
 Parser.Line->Level = PreBlockLine->Level;
 Parser.Line->InPPDirective = PreBlockLine->InPPDirective;
+Parser.Line->InMacroBody = PreBlockLine->InMacroBody;
   }
 
   ~ScopedLineState() {
@@ -1251,6 +1254,7 @@
 Line->Level += PPBranchLevel + 1;
   addUnwrappedLine();
   ++Line->Level;
+  Line->InMacroBody = true;
 
   // Errors during a preprocessor directive can only affect the layout of the
   // preprocessor directive, and thus we ignore them. An alternative approach
Index: clang/lib/Format/TokenAnnotator.h
===
--- clang/lib/Format/TokenAnnotator.h
+++ clang/lib/Format/Token

[PATCH] D133443: [RISCV][MC] Add support for experimental Zawrs extension

[Philip Reames via Phabricator via cfe-commits]

reames updated this revision to Diff 460447.
reames added a comment.

Add docs.


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

https://reviews.llvm.org/D133443

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s

Index: llvm/test/MC/RISCV/attribute-arch.s
===
--- llvm/test/MC/RISCV/attribute-arch.s
+++ llvm/test/MC/RISCV/attribute-arch.s
@@ -197,5 +197,8 @@
 .attribute arch, "rv32izca0p70"
 # CHECK: attribute  5, "rv32i2p0_zca0p70"
 
+.attribute arch, "rv32izawrs1p0"
+# CHECK: attribute  5, "rv32i2p0_zawrs1p0"
+
 .attribute arch, "rv32iztso0p1"
 # CHECK: attribute  5, "rv32i2p0_ztso0p1"
Index: llvm/test/CodeGen/RISCV/attributes.ll
===
--- llvm/test/CodeGen/RISCV/attributes.ll
+++ llvm/test/CodeGen/RISCV/attributes.ll
@@ -84,6 +84,7 @@
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbom %s -o - | FileCheck --check-prefix=RV64ZICBOM %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicboz %s -o - | FileCheck --check-prefix=RV64ZICBOZ %s
 ; RUN: llc -mtriple=riscv64 -mattr=+zicbop %s -o - | FileCheck --check-prefix=RV64ZICBOP %s
+; RUN: llc -mtriple=riscv64 -mattr=+experimental-zawrs %s -o - | FileCheck --check-prefix=RV64ZAWRS %s
 ; RUN: llc -mtriple=riscv64 -mattr=+experimental-ztso %s -o - | FileCheck --check-prefix=RV64ZTSO %s
 
 ; RV32M: .attribute 5, "rv32i2p0_m2p0"
@@ -170,6 +171,7 @@
 ; RV64COMBINEINTOZKS: .attribute 5, "rv64i2p0_zbkb1p0_zbkc1p0_zbkx1p0_zks1p0_zksed1p0_zksh1p0"
 ; RV64ZICBOM: .attribute 5, "rv64i2p0_zicbom1p0"
 ; RV64ZICBOZ: .attribute 5, "rv64i2p0_zicboz1p0"
+; RV64ZAWRS: .attribute 5, "rv64i2p0_zawrs1p0"
 ; RV64ZICBOP: .attribute 5, "rv64i2p0_zicbop1p0"
 ; RV64ZTSO: .attribute 5, "rv64i2p0_ztso0p1"
 
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -92,6 +92,7 @@
   bool HasStdExtZicboz = false;
   bool HasStdExtZicbop = false;
   bool HasStdExtZmmul = false;
+  bool HasStdExtZawrs = false;
   bool HasStdExtZtso = false;
   bool HasRV32 = false;
   bool HasRV64 = false;
@@ -192,6 +193,7 @@
   bool hasStdExtZicbom() const { return HasStdExtZicbom; }
   bool hasStdExtZicboz() const { return HasStdExtZicboz; }
   bool hasStdExtZicbop() const { return HasStdExtZicbop; }
+  bool hasStdExtZawrs() const { return HasStdExtZawrs; }
   bool hasStdExtZmmul() const { return HasStdExtZmmul; }
   bool hasStdExtZtso() const { return HasStdExtZtso; }
   bool is64Bit() const { return HasRV64; }
Index: llvm/lib/Target/RISCV/RISCVInstrInfo.td
===
--- llvm/lib/Target/RISCV/RISCVInstrInfo.td
+++ llvm/lib/Target/RISCV/RISCVInstrInfo.td
@@ -705,6 +705,23 @@
   let rd = 0;
   let imm12 = 0b1100;
 }
+
+let Predicates = [HasStdExtZawrs] in {
+def WRS_NTO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.nto", "">,
+  Sched<[]> {
+  let rs1 = 0;
+  let rd = 0;
+  let imm12 = 0b1101;
+}
+
+def WRS_STO : RVInstI<0b000, OPC_SYSTEM, (outs), (ins), "wrs.sto", "">,
+  Sched<[]> {
+  let rs1 = 0;
+  let rd = 0;
+  let imm12 = 0b00011101;
+}
+} // Predicates = [HasStdExtZawrs]
+
 } // hasSideEffects = 1, mayLoad = 0, mayStore = 0
 
 def CSRRW : CSR_ir<0b001, "csrrw">;
Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -461,6 +461,13 @@
AssemblerPredicate<(all_of FeatureStdExtZtso),
"'Ztso' (Memory Model - Total Store Order)">;
 
+def FeatureStdExtZawrs
+: SubtargetFeature<"experimental-zawrs", "HasStdExtZawrs", "true",
+   "'Zawrs' (Wait on Reservation Set)">;
+def HasStdExtZawrs : Predicate<"Subtarget->hasStdExtZawrs()">,
+   AssemblerPredicate<(all_of FeatureStdExtZawrs),
+   "'Zawrs' (Wait on Reservation Set)">;
+
 // Feature32Bit exists to mark CPUs that support RV32 to distinquish them from
 // tuning CPU names.
 def Feature32Bit
Index: llvm/lib/Support/RISCVISAInfo.cpp
===
--- llvm/lib/Support/RISCVISAInfo.cpp
+++ llvm/lib/Support/RISCVISAInfo.cpp
@@ -114,6 +114,7 @@
 {"zbt", RISCVExtensionVersion{0, 93}},
 {"zca", RISCVExtensionVersion{0, 70}},
 {"zvfh", RISCVExtensionVersion{0, 1}},
+{"zawrs", RISCVExtensionVersion{1, 0}},
 {"ztso", RISCVExtensionVersion{0, 1}},
 };
 
Index: llvm/docs/RISCVUsage.rst
=

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/Type.h:1972-1973
+  /// Check if this is a WebAssembly Reference Type.
+  bool isWebAssemblyReferenceType() const;
+  bool isWebAssemblyExternrefType() const;
   /// Determines if this is a sizeless type supported by the

pmatos wrote:
> aaron.ballman wrote:
> > It's unfortunate to name this with `ReferenceType` given that we already 
> > have a considerable number of APIs that assume "reference type" to mean `&` 
> > or `&&`. We run into similar problems with "pointer type" and objective-c 
> > pointers.
> > 
> > Basically, I worry we're setting ourselves up for another 
> > `isObjCObjectPointerType()`/`isPointerType()` situation.
> I understand this concern. However, unsure what else to call it given that's 
> what it is. It's a WebAssembly Reference Type, which indeed is different from 
> a C/C++ ReferenceType. Could call it OpaqueType but that would be even worse. 
> Not only it's not called an opaque type in Wasm, it's also not what LLVM 
> users will think of as Opaque Types.
Yeah, I think this is as good of a name as we're likely to find; I certainly 
haven't thought of something better. If we do think of something later, we can 
refactor then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122215

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