[clang] e190b7c - [Coroutines] Maintain the position of final suspend

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-08-12T13:05:08+08:00
New Revision: e190b7cc90ca5ee712ca3982bf476afa9e8acb3b

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

LOG: [Coroutines] Maintain the position of final suspend

Closing https://github.com/llvm/llvm-project/issues/56329

The problem happens when we try to simplify the suspend points. We might
break the assumption that the final suspend lives in the last slot of
Shape.CoroSuspends. This patch tries to main the assumption and fixes
the problem.

Added: 
clang/test/CodeGenCoroutines/pr56329.cpp
llvm/test/Transforms/Coroutines/coro-preserve-final.ll

Modified: 
llvm/lib/Transforms/Coroutines/CoroSplit.cpp

Removed: 




diff  --git a/clang/test/CodeGenCoroutines/pr56329.cpp 
b/clang/test/CodeGenCoroutines/pr56329.cpp
new file mode 100644
index 0..3918acae0f08f
--- /dev/null
+++ b/clang/test/CodeGenCoroutines/pr56329.cpp
@@ -0,0 +1,117 @@
+// Test for PR56919. Tests the we won't contain the resumption of final 
suspend point.
+//
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 %s -O3 -S -emit-llvm 
-o - | FileCheck %s
+
+#include "Inputs/coroutine.h"
+
+void _exit(int status) __attribute__ ((__noreturn__));
+
+class Promise;
+
+// An object that can be co_awaited, but we always resume immediately from
+// await_suspend.
+struct ResumeFromAwaitSuspend{};
+
+struct Task {
+  using promise_type = Promise;
+  Promise& promise;
+};
+
+struct Promise {
+  static std::coroutine_handle GetHandle(Promise& promise) {
+return std::coroutine_handle::from_promise(promise);
+  }
+
+  void unhandled_exception() {}
+  Task get_return_object() { return Task{*this}; }
+  void return_void() {}
+
+  // Always suspend before starting the coroutine body. We actually run the 
body
+  // when we are co_awaited.
+  std::suspend_always initial_suspend() { return {}; }
+
+  // We support awaiting tasks. We do so by configuring them to resume us when
+  // they are finished, and then resuming them from their initial suspend.
+  auto await_transform(Task&& task) {
+struct Awaiter {
+  bool await_ready() { return false; }
+
+  std::coroutine_handle<> await_suspend(
+  const std::coroutine_handle<> handle) {
+// Tell the child to resume the parent once it finishes.
+child.resume_at_final_suspend = GetHandle(parent);
+
+// Run the child.
+return GetHandle(child);
+  }
+
+  void await_resume() {
+// The child is now at its final suspend point, and can be destroyed.
+return GetHandle(child).destroy();
+  }
+
+  Promise& parent;
+  Promise& child;
+};
+
+return Awaiter{
+.parent = *this,
+.child = task.promise,
+};
+  }
+
+  // Make evaluation of `co_await ResumeFromAwaitSuspend{}` go through the
+  // await_suspend path, but cause it to resume immediately by returning our 
own
+  // handle to resume.
+  auto await_transform(ResumeFromAwaitSuspend) {
+struct Awaiter {
+  bool await_ready() { return false; }
+
+  std::coroutine_handle<> await_suspend(const std::coroutine_handle<> h) {
+return h;
+  }
+
+  void await_resume() {}
+};
+
+return Awaiter{};
+  }
+
+  // Always suspend at the final suspend point, transferring control back to 
our
+  // caller. We expect never to be resumed from the final suspend.
+  auto final_suspend() noexcept {
+struct FinalSuspendAwaitable final {
+  bool await_ready() noexcept { return false; }
+
+  std::coroutine_handle<> await_suspend(std::coroutine_handle<>) noexcept {
+return promise.resume_at_final_suspend;
+  }
+
+  void await_resume() noexcept {
+_exit(1);
+  }
+
+  Promise& promise;
+};
+
+return FinalSuspendAwaitable{.promise = *this};
+  }
+
+  // The handle we will resume once we hit final suspend.
+  std::coroutine_handle<> resume_at_final_suspend;
+};
+
+Task Inner();
+
+Task Outer() {
+  co_await ResumeFromAwaitSuspend();
+  co_await Inner();
+}
+
+// CHECK: define{{.*}}@_Z5Outerv.resume(
+// CHECK-NOT: }
+// CHECK-NOT: _exit
+// CHECK: musttail call
+// CHECK: musttail call
+// CHECK-NEXT: ret void
+// CHEKC-NEXT: }

diff  --git a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp 
b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
index 722a1c6ec0cee..5efe377f1f938 100644
--- a/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroSplit.cpp
@@ -1555,6 +1555,8 @@ static void simplifySuspendPoints(coro::Shape ) {
   size_t I = 0, N = S.size();
   if (N == 0)
 return;
+
+  size_t ChangedFinalIndex = std::numeric_limits::max();
   while (true) {
 auto SI = cast(S[I]);
 // Leave final.suspend to handleFinalSuspend since it is undefined behavior
@@ -1562,13 

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/lib/Basic/TypeTraits.cpp:64
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};

inclyc wrote:
> shafik wrote:
> > @aaron.ballman do we really have to include this three times? We are 
> > defining different macros so shouldn't we be able to include is just once? 
> > 
> > I see we do this in several other places but a few we don't.
> I've tried 
> 
> ```
> #define TYPE_TRAIT(N, I, K) N,
> #include "clang/Basic/TokenKinds.def"
> ```
> 
> But using enum `TypeTrait` as array index seems to have incorrect mapping.
> @aaron.ballman do we really have to include this three times? We are defining 
> different macros so shouldn't we be able to include is just once? 
> 
> I see we do this in several other places but a few we don't.

Type trait definitions in `#include "clang/Basic/TokenKinds.def"` may not be 
sorted by the number of arguments.

Example:

```
#define TYPE_TRAIT_1(some_stuff)
#define TYPE_TRAIT_N(some_stuff)
#define TYPE_TRAIT_2(some_stuff)
```

Might be necessary to include this 3 times to get sorted layouts, like

`[1, 1, 1, 2, 2, 2, 0, 0]`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

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

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/lib/Basic/TypeTraits.cpp:64
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};

shafik wrote:
> @aaron.ballman do we really have to include this three times? We are defining 
> different macros so shouldn't we be able to include is just once? 
> 
> I see we do this in several other places but a few we don't.
I've tried 

```
#define TYPE_TRAIT(N, I, K) N,
#include "clang/Basic/TokenKinds.def"
```

But using enum `TypeTrait` as array index seems to have incorrect mapping.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:3789
-
-  if (!Arity && Args.empty()) {
-Diag(EndLoc, diag::err_type_trait_arity)

shafik wrote:
> Why doesn't this catch our case but moving the check into `evaluateTypeTrait` 
> does?
In this case:

```
template bool b = __is_constructible(Ts...); // Parser: 1 argument 
`Ts`, no problem
bool x = b<>; // After template template instantiation tree transformation: 0 
argument, but missing checks!
```

`Ts...` is considered as 1 argument in Parsing, so passed this check.

> Why doesn't this catch our case but moving the check into `evaluateTypeTrait` 
> does?

This patch moved the check to `Sema::BuildTypeTrait`, this function will be 
invoked by tree transformation procedure.

>  invoked by tree transformation procedure

In fact: `clang::TreeTransform<(anonymous 
namespace)::TemplateInstantiator>::TransformTypeTraitExpr(clang::TypeTraitExpr*)
 SemaTemplateInstantiate.cpp`). 

I think it is not a good idea to move the check into  `evaluateTypeTrait`, 
because this function might be designed to return the final evaluation result, 
we should perform the check _before_ this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

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

[PATCH] D131639: [OpenMP] Remove 'stdbool.h' from OpenMP header wrappers

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

I'm fine with the change. @JonChesterfield WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131639

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

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

[Sunil Shrestha via Phabricator via cfe-commits]

ssquare08 added a comment.

In D129694#3717208 , @jhuber6 wrote:

> In D129694#3717166 , @ssquare08 
> wrote:
>
>> The OpenMP kernel names you mentioned are also generated separately by the 
>> host and the device. Would you be okay generating declare target mangle 
>> names separately by host and device using the same utility function 
>> `getTargetEntryUniqueInfo`?
>>
>> If you still think it should only be generated only once by the host, what 
>> is a good way of doing this since we can't modify the name in VarDecl?
>
> I thought we already emitted the mangled name at least on the device side. I 
> was suggesting that we just use the same name on the host so we don't need to 
> worry about a host-side and device-side name difference and we can get rid of 
> the extra argument to all the offload entry functions.

Yes, that is correct. My question is, is it okay to mangle the host and the 
device side independently using `getTargetEntryUniqueInfo`? The reason I am 
asking is because you had expressed some concerns regarding mangling them 
separately. Or, maybe there is a way to mangle the original name before the 
host and device compilation split?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129694

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

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/Basic/TypeTraits.cpp:64
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};

@aaron.ballman do we really have to include this three times? We are defining 
different macros so shouldn't we be able to include is just once? 

I see we do this in several other places but a few we don't.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:3789
-
-  if (!Arity && Args.empty()) {
-Diag(EndLoc, diag::err_type_trait_arity)

Why doesn't this catch our case but moving the check into `evaluateTypeTrait` 
does?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

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

[PATCH] D123967: Disable update_cc_test_checks.py tests in stand-alone builds

[Tom Stellard via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG74384c7fcec7: Disable update_cc_test_checks.py tests in 
stand-alone builds (authored by tstellar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123967

Files:
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  clang/test/utils/update_cc_test_checks/lit.local.cfg


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -10,27 +10,36 @@
 from pipes import quote as shell_quote
 
 
-config.test_format = lit.formats.ShTest(execute_external=False)
-config.suffixes = ['.test']
-
-clang_path = os.path.join(config.clang_tools_dir, 'clang')
-extra_args = '--clang ' + shell_quote(clang_path)
-opt_path = os.path.join(config.llvm_tools_dir, 'opt')
-extra_args += ' --opt ' + shell_quote(opt_path)
-script_path = os.path.join(config.llvm_src_root, 'utils',
-   'update_cc_test_checks.py')
-assert os.path.isfile(script_path)
-# Windows: llvm-lit.py, Linux: llvm-lit
-if config.llvm_external_lit:
-lit = config.llvm_external_lit
+if config.standalone_build:
+# These tests require the update_cc_test_checks.py script from the llvm
+# source tree, so skip these tests if we are doing standalone builds.
+# These tests are only relevant to developers working with the
+# update_cc_test_checks.py tool; they don't provide any coverage
+# for any of the clang source code.
+config.unsupported = True
 else:
-lit = shell_quote(glob.glob(os.path.join(config.llvm_tools_dir, 
'llvm-lit*'))[0])
-python = shell_quote(config.python_executable)
-config.substitutions.append(
-('%update_cc_test_checks', "%s %s %s" % (
-python, shell_quote(script_path), extra_args)))
-config.substitutions.append(
-('%clang_tools_dir', shell_quote(config.clang_tools_dir)))
-config.substitutions.append(
-('%lit', "%s %s -Dclang_lit_site_cfg=%s -j1 -vv" % (
-python, lit, shell_quote(config.clang_lit_site_cfg
+
+config.test_format = lit.formats.ShTest(execute_external=False)
+config.suffixes = ['.test']
+
+clang_path = os.path.join(config.clang_tools_dir, 'clang')
+extra_args = '--clang ' + shell_quote(clang_path)
+opt_path = os.path.join(config.llvm_tools_dir, 'opt')
+extra_args += ' --opt ' + shell_quote(opt_path)
+script_path = os.path.join(config.llvm_src_root, 'utils',
+   'update_cc_test_checks.py')
+assert os.path.isfile(script_path)
+# Windows: llvm-lit.py, Linux: llvm-lit
+if config.llvm_external_lit:
+lit = config.llvm_external_lit
+else:
+lit = shell_quote(glob.glob(os.path.join(config.llvm_tools_dir, 
'llvm-lit*'))[0])
+python = shell_quote(config.python_executable)
+config.substitutions.append(
+('%update_cc_test_checks', "%s %s %s" % (
+python, shell_quote(script_path), extra_args)))
+config.substitutions.append(
+('%clang_tools_dir', shell_quote(config.clang_tools_dir)))
+config.substitutions.append(
+('%lit', "%s %s -Dclang_lit_site_cfg=%s -j1 -vv" % (
+python, lit, shell_quote(config.clang_lit_site_cfg
Index: clang/test/lit.site.cfg.py.in
===
--- clang/test/lit.site.cfg.py.in
+++ clang/test/lit.site.cfg.py.in
@@ -37,6 +37,7 @@
 config.has_plugins = @CLANG_PLUGIN_SUPPORT@
 config.clang_vendor_uti = "@CLANG_VENDOR_UTI@"
 config.llvm_external_lit = path(r"@LLVM_EXTERNAL_LIT@")
+config.standalone_build = @CLANG_BUILT_STANDALONE@
 
 import lit.llvm
 lit.llvm.initialize(lit_config, config)
Index: clang/test/CMakeLists.txt
===
--- clang/test/CMakeLists.txt
+++ clang/test/CMakeLists.txt
@@ -3,6 +3,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
+  CLANG_BUILT_STANDALONE
   CLANG_DEFAULT_PIE_ON_LINUX
   CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL
   CLANG_ENABLE_ARCMT


Index: clang/test/utils/update_cc_test_checks/lit.local.cfg
===
--- clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -10,27 +10,36 @@
 from pipes import quote as shell_quote
 
 
-config.test_format = lit.formats.ShTest(execute_external=False)
-config.suffixes = ['.test']
-
-clang_path = os.path.join(config.clang_tools_dir, 'clang')
-extra_args = '--clang ' + shell_quote(clang_path)
-opt_path = os.path.join(config.llvm_tools_dir, 'opt')
-extra_args += ' --opt ' + shell_quote(opt_path)
-script_path = 

[clang] 74384c7 - Disable update_cc_test_checks.py tests in stand-alone builds

[Tom Stellard via cfe-commits]

Author: Tom Stellard
Date: 2022-08-11T20:53:37-07:00
New Revision: 74384c7fcec71cb040b0c874743e5fc38b2cd7a6

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

LOG: Disable update_cc_test_checks.py tests in stand-alone builds

The script is located in the llvm/ sub-directory, so it is not available
for when doing a stand-alone build.

See https://discourse.llvm.org/t/rfc-stand-alone-build-support/61291

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

Added: 


Modified: 
clang/test/CMakeLists.txt
clang/test/lit.site.cfg.py.in
clang/test/utils/update_cc_test_checks/lit.local.cfg

Removed: 




diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index a05c372fbd346..08166a1d5cfd7 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -3,6 +3,7 @@
 
 llvm_canonicalize_cmake_booleans(
   CLANG_BUILD_EXAMPLES
+  CLANG_BUILT_STANDALONE
   CLANG_DEFAULT_PIE_ON_LINUX
   CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL
   CLANG_ENABLE_ARCMT

diff  --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index 8a9849fe4549d..9c903b2d31cb8 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -37,6 +37,7 @@ config.use_z3_solver = lit_config.params.get('USE_Z3_SOLVER', 
"@USE_Z3_SOLVER@")
 config.has_plugins = @CLANG_PLUGIN_SUPPORT@
 config.clang_vendor_uti = "@CLANG_VENDOR_UTI@"
 config.llvm_external_lit = path(r"@LLVM_EXTERNAL_LIT@")
+config.standalone_build = @CLANG_BUILT_STANDALONE@
 
 import lit.llvm
 lit.llvm.initialize(lit_config, config)

diff  --git a/clang/test/utils/update_cc_test_checks/lit.local.cfg 
b/clang/test/utils/update_cc_test_checks/lit.local.cfg
index d57fa6832d425..b78c4ffdab585 100644
--- a/clang/test/utils/update_cc_test_checks/lit.local.cfg
+++ b/clang/test/utils/update_cc_test_checks/lit.local.cfg
@@ -10,27 +10,36 @@ except ImportError:
 from pipes import quote as shell_quote
 
 
-config.test_format = lit.formats.ShTest(execute_external=False)
-config.suffixes = ['.test']
-
-clang_path = os.path.join(config.clang_tools_dir, 'clang')
-extra_args = '--clang ' + shell_quote(clang_path)
-opt_path = os.path.join(config.llvm_tools_dir, 'opt')
-extra_args += ' --opt ' + shell_quote(opt_path)
-script_path = os.path.join(config.llvm_src_root, 'utils',
-   'update_cc_test_checks.py')
-assert os.path.isfile(script_path)
-# Windows: llvm-lit.py, Linux: llvm-lit
-if config.llvm_external_lit:
-lit = config.llvm_external_lit
+if config.standalone_build:
+# These tests require the update_cc_test_checks.py script from the llvm
+# source tree, so skip these tests if we are doing standalone builds.
+# These tests are only relevant to developers working with the
+# update_cc_test_checks.py tool; they don't provide any coverage
+# for any of the clang source code.
+config.unsupported = True
 else:
-lit = shell_quote(glob.glob(os.path.join(config.llvm_tools_dir, 
'llvm-lit*'))[0])
-python = shell_quote(config.python_executable)
-config.substitutions.append(
-('%update_cc_test_checks', "%s %s %s" % (
-python, shell_quote(script_path), extra_args)))
-config.substitutions.append(
-('%clang_tools_dir', shell_quote(config.clang_tools_dir)))
-config.substitutions.append(
-('%lit', "%s %s -Dclang_lit_site_cfg=%s -j1 -vv" % (
-python, lit, shell_quote(config.clang_lit_site_cfg
+
+config.test_format = lit.formats.ShTest(execute_external=False)
+config.suffixes = ['.test']
+
+clang_path = os.path.join(config.clang_tools_dir, 'clang')
+extra_args = '--clang ' + shell_quote(clang_path)
+opt_path = os.path.join(config.llvm_tools_dir, 'opt')
+extra_args += ' --opt ' + shell_quote(opt_path)
+script_path = os.path.join(config.llvm_src_root, 'utils',
+   'update_cc_test_checks.py')
+assert os.path.isfile(script_path)
+# Windows: llvm-lit.py, Linux: llvm-lit
+if config.llvm_external_lit:
+lit = config.llvm_external_lit
+else:
+lit = shell_quote(glob.glob(os.path.join(config.llvm_tools_dir, 
'llvm-lit*'))[0])
+python = shell_quote(config.python_executable)
+config.substitutions.append(
+('%update_cc_test_checks', "%s %s %s" % (
+python, shell_quote(script_path), extra_args)))
+config.substitutions.append(
+('%clang_tools_dir', shell_quote(config.clang_tools_dir)))
+config.substitutions.append(
+('%lit', "%s %s -Dclang_lit_site_cfg=%s -j1 -vv" % (
+python, lit, shell_quote(config.clang_lit_site_cfg



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

[PATCH] D131651: [AST] [Modules] Introduce Decl::getNonTransparentDeclContext to handle exported friends

[Chuanqi Xu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0bc993edf4ec: [AST] [Modules] Introduce 
Decl::getNonTransparentDeclContext to handle exported… (authored by ChuanqiXu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131651

Files:
  clang/include/clang/AST/DeclBase.h
  clang/lib/AST/DeclBase.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/Modules/pr56826.cppm
  clang/unittests/AST/DeclTest.cpp

Index: clang/unittests/AST/DeclTest.cpp
===
--- clang/unittests/AST/DeclTest.cpp
+++ clang/unittests/AST/DeclTest.cpp
@@ -260,3 +260,22 @@
   EXPECT_EQ(b->getLinkageInternal(), ModuleLinkage);
   EXPECT_EQ(g->getLinkageInternal(), ModuleLinkage);
 }
+
+TEST(Decl, GetNonTransparentDeclContext) {
+  llvm::Annotations Code(R"(
+export module m3;
+export template  struct X {
+  template  friend void f(Self &) {
+(Self&)self;
+  }
+};)");
+
+  auto AST =
+  tooling::buildASTFromCodeWithArgs(Code.code(), /*Args=*/{"-std=c++20"});
+  ASTContext  = AST->getASTContext();
+
+  auto *f = selectFirst(
+  "f", match(functionDecl(hasName("f")).bind("f"), Ctx));
+
+  EXPECT_TRUE(f->getNonTransparentDeclContext()->isFileContext());
+}
Index: clang/test/Modules/pr56826.cppm
===
--- /dev/null
+++ clang/test/Modules/pr56826.cppm
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+export module m3;
+export template  struct X {
+  template  friend void f(Self &) {
+(Self&)self; // expected-warning {{expression result unused}}
+  }
+};
+void g() { f(X{}); } // expected-note {{in instantiation of function template specialization}}
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6147,6 +6147,8 @@
 
   // Move to the outer template scope.
   if (FunctionDecl *FD = dyn_cast(DC)) {
+// FIXME: We should use `getNonTransparentDeclContext()` here instead
+// of `getDeclContext()` once we find the invalid test case.
 if (FD->getFriendObjectKind() && FD->getDeclContext()->isFileContext()){
   DC = FD->getLexicalDeclContext();
   continue;
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -168,7 +168,7 @@
   // instead of its semantic parent, unless of course the pattern we're
   // instantiating actually comes from the file's context!
   if (Function->getFriendObjectKind() &&
-  Function->getDeclContext()->isFileContext() &&
+  Function->getNonTransparentDeclContext()->isFileContext() &&
   (!Pattern || !Pattern->getLexicalDeclContext()->isFileContext())) {
 Ctx = Function->getLexicalDeclContext();
 RelativeToPrimary = false;
Index: clang/lib/AST/DeclBase.cpp
===
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -1032,6 +1032,11 @@
   return Ty->getAs();
 }
 
+DeclContext *Decl::getNonTransparentDeclContext() {
+  assert(getDeclContext());
+  return getDeclContext()->getNonTransparentContext();
+}
+
 /// Starting at a given context (a Decl or DeclContext), look for a
 /// code context that is not a closure (a lambda, block, etc.).
 template  static Decl *getNonClosureContext(T *D) {
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -447,6 +447,14 @@
 return const_cast(this)->getDeclContext();
   }
 
+  /// Return the non transparent context.
+  /// See the comment of `DeclContext::isTransparentContext()` for the
+  /// definition of transparent context.
+  DeclContext *getNonTransparentDeclContext();
+  const DeclContext *getNonTransparentDeclContext() const {
+return const_cast(this)->getNonTransparentDeclContext();
+  }
+
   /// Find the innermost non-closure ancestor of this declaration,
   /// walking up through blocks, lambdas, etc.  If that ancestor is
   /// not a code context (!isFunctionOrMethod()), returns null.
@@ -2009,7 +2017,7 @@
   /// Here, E is a transparent context, so its enumerator (Val1) will
   /// appear (semantically) that it is in the same context of E.
   /// Examples of transparent contexts include: enumerations (except for
-  /// C++0x scoped enums), and C++ linkage specifications.
+  /// C++0x scoped enums), C++ linkage specifications and export declaration.
   bool 

[clang] 0bc993e - [AST] [Modules] Introduce Decl::getNonTransparentDeclContext to handle exported friends

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-08-12T11:50:35+08:00
New Revision: 0bc993edf4ec57993c8fc11ea511b3d62ffd3c93

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

LOG: [AST] [Modules] Introduce Decl::getNonTransparentDeclContext to handle 
exported friends

Closing https://github.com/llvm/llvm-project/issues/56826.

The root cause for pr56826 is: when we collect the template args for the
friend, we need to judge if the friend lives in file context. However,
if the friend lives in ExportDecl lexically, the judgement here is
invalid.

The solution is easy. We should judge the non transparent context and
the ExportDecl is transparent context. So the solution should be good.

A main concern may be the patch doesn't handle all the places of the
same defect. I think it might not be bad since the patch itself should
be innocent.

Reviewed By: erichkeane

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

Added: 
clang/test/Modules/pr56826.cppm

Modified: 
clang/include/clang/AST/DeclBase.h
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/unittests/AST/DeclTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index d1193161fd754..1c4e726817bbf 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -447,6 +447,14 @@ class alignas(8) Decl {
 return const_cast(this)->getDeclContext();
   }
 
+  /// Return the non transparent context.
+  /// See the comment of `DeclContext::isTransparentContext()` for the
+  /// definition of transparent context.
+  DeclContext *getNonTransparentDeclContext();
+  const DeclContext *getNonTransparentDeclContext() const {
+return const_cast(this)->getNonTransparentDeclContext();
+  }
+
   /// Find the innermost non-closure ancestor of this declaration,
   /// walking up through blocks, lambdas, etc.  If that ancestor is
   /// not a code context (!isFunctionOrMethod()), returns null.
@@ -2009,7 +2017,7 @@ class DeclContext {
   /// Here, E is a transparent context, so its enumerator (Val1) will
   /// appear (semantically) that it is in the same context of E.
   /// Examples of transparent contexts include: enumerations (except for
-  /// C++0x scoped enums), and C++ linkage specifications.
+  /// C++0x scoped enums), C++ linkage specifications and export declaration.
   bool isTransparentContext() const;
 
   /// Determines whether this context or some of its ancestors is a

diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index d12330de1500c..f22bf6b0937ee 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -1032,6 +1032,11 @@ const FunctionType *Decl::getFunctionType(bool 
BlocksToo) const {
   return Ty->getAs();
 }
 
+DeclContext *Decl::getNonTransparentDeclContext() {
+  assert(getDeclContext());
+  return getDeclContext()->getNonTransparentContext();
+}
+
 /// Starting at a given context (a Decl or DeclContext), look for a
 /// code context that is not a closure (a lambda, block, etc.).
 template  static Decl *getNonClosureContext(T *D) {

diff  --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index c4e5fb50f8cc8..1dd4baf901827 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -168,7 +168,7 @@ MultiLevelTemplateArgumentList 
Sema::getTemplateInstantiationArgs(
   // instead of its semantic parent, unless of course the pattern we're
   // instantiating actually comes from the file's context!
   if (Function->getFriendObjectKind() &&
-  Function->getDeclContext()->isFileContext() &&
+  Function->getNonTransparentDeclContext()->isFileContext() &&
   (!Pattern || !Pattern->getLexicalDeclContext()->isFileContext())) {
 Ctx = Function->getLexicalDeclContext();
 RelativeToPrimary = false;

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index d303e9f1724bb..140844ca4e811 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -6147,6 +6147,8 @@ NamedDecl *Sema::FindInstantiatedDecl(SourceLocation Loc, 
NamedDecl *D,
 
   // Move to the outer template scope.
   if (FunctionDecl *FD = dyn_cast(DC)) {
+// FIXME: We should use `getNonTransparentDeclContext()` here instead
+// of `getDeclContext()` once we find the invalid test case.
 if (FD->getFriendObjectKind() && 
FD->getDeclContext()->isFileContext()){
   DC = FD->getLexicalDeclContext();
   continue;

diff  --git a/clang/test/Modules/pr56826.cppm 

[PATCH] D127641: [clang-cl][MSVC] Enable /Zc:alignedNew for C++17 and /Zc:sizedDealloc by default

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 452062.
steplong added a comment.

- Change (1|2) to [12]. Test is failing because () isn't supported


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127641

Files:
  clang-tools-extra/test/clang-tidy/checkers/misc-new-delete-overloads.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/cl-zc.cpp
  clang/unittests/StaticAnalyzer/CallEventTest.cpp

Index: clang/unittests/StaticAnalyzer/CallEventTest.cpp
===
--- clang/unittests/StaticAnalyzer/CallEventTest.cpp
+++ clang/unittests/StaticAnalyzer/CallEventTest.cpp
@@ -8,6 +8,7 @@
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/CallEvent.h"
 #include "CheckerRegistration.h"
+#include "gmock/gmock.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
@@ -22,6 +23,8 @@
 #include "clang/Tooling/Tooling.h"
 #include "gtest/gtest.h"
 
+using testing::MatchesRegex;
+
 namespace clang {
 namespace ento {
 namespace {
@@ -81,7 +84,7 @@
 }
   )",
  Diags));
-  EXPECT_EQ(Diags, "test.CXXDeallocator: NumArgs: 1\n");
+  EXPECT_THAT(Diags, MatchesRegex("test.CXXDeallocator: NumArgs: [12]\n"));
 }
 
 } // namespace
Index: clang/test/Driver/cl-zc.cpp
===
--- clang/test/Driver/cl-zc.cpp
+++ clang/test/Driver/cl-zc.cpp
@@ -1,6 +1,19 @@
 // Note: %s must be preceded by --, otherwise it may be interpreted as a
 // command-line option, e.g. on Mac where %s is commonly under /Users.
 
+// RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=SIZED-DEALLOC %s
+// RUN: %clang_cl /c -### -fms-compatibility-version=18 -- %s 2>&1 | FileCheck -check-prefix=SIZED-DEALLOC-OFF %s
+// SIZED-DEALLOC: "-fsized-deallocation"
+// SIZED-DEALLOC-OFF-NOT: "-fsized-deallocation"
+
+// RUN: %clang_cl /c /std:c++11 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-BEFORE-CPP17 %s
+// RUN: %clang_cl /c /std:c++14 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-BEFORE-CPP17 %s
+// RUN: %clang_cl /c /std:c++17 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS %s
+// RUN: %clang_cl /c /std:c++20 -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS %s
+// RUN: %clang_cl /c /std:c++latest -### -- %s 2>&1 | FileCheck -check-prefix=ALIGNED-NEW-CPP17ONWARDS %s
+// ALIGNED-NEW-BEFORE-CPP17-NOT: "-faligned-allocation"
+// ALIGNED-NEW-CPP17ONWARDS: "-faligned-allocation"
+
 // RUN: %clang_cl /c -### -- %s 2>&1 | FileCheck -check-prefix=TRIGRAPHS-DEFAULT %s
 // cc1 will disable trigraphs for -fms-compatibility as long as -ftrigraphs
 // isn't explicitly passed.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6470,6 +6470,7 @@
 }
 CmdArgs.push_back(LanguageStandard.data());
   }
+  bool IsCPP17Onwards = false;
   if (ImplyVCPPCXXVer) {
 StringRef LanguageStandard;
 if (const Arg *StdArg = Args.getLastArg(options::OPT__SLASH_std)) {
@@ -6493,6 +6494,9 @@
 }
 
 CmdArgs.push_back(LanguageStandard.data());
+
+IsCPP17Onwards =
+LanguageStandard != "-std=c++11" && LanguageStandard != "-std=c++14";
   }
 
   Args.addOptInFlag(CmdArgs, options::OPT_fborland_extensions,
@@ -6622,9 +6626,15 @@
 options::OPT_fno_relaxed_template_template_args);
 
   // -fsized-deallocation is off by default, as it is an ABI-breaking change for
-  // most platforms.
-  Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
-options::OPT_fno_sized_deallocation);
+  // most platforms. MSVC turns on /Zc:sizedDealloc by default, starting in
+  // MSVC 2015.
+  if (IsWindowsMSVC && IsMSVC2015Compatible &&
+  !Args.getLastArg(options::OPT_fsized_deallocation,
+   options::OPT_fno_sized_deallocation))
+CmdArgs.push_back("-fsized-deallocation");
+  else
+Args.addOptInFlag(CmdArgs, options::OPT_fsized_deallocation,
+  options::OPT_fno_sized_deallocation);
 
   // -faligned-allocation is on by default in C++17 onwards and otherwise off
   // by default.
@@ -6635,6 +6645,8 @@
   CmdArgs.push_back("-fno-aligned-allocation");
 else
   CmdArgs.push_back("-faligned-allocation");
+  } else if (IsCPP17Onwards) {
+CmdArgs.push_back("-faligned-allocation");
   }
 
   // The default new alignment can be specified using a dedicated option or via
Index: clang-tools-extra/test/clang-tidy/checkers/misc-new-delete-overloads.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/misc-new-delete-overloads.cpp
+++ 

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[chenglin.bi via Phabricator via cfe-commits]

bcl5980 added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5128
+// to the function itself; it points to a stub for the compiler.
+// FIXME: We also need to emit an entry thunk.
+SmallString<256> MangledName;

efriedma wrote:
> bcl5980 wrote:
> > efriedma wrote:
> > > bcl5980 wrote:
> > > > efriedma wrote:
> > > > > bcl5980 wrote:
> > > > > > A headache thing here.
> > > > > > We need to get the function definition with triple x64 to define 
> > > > > > entry thunk. For now the function definition here is aarch64 
> > > > > > version.
> > > > > > For example the case in Microsoft doc "Understanding Arm64EC ABI 
> > > > > > and assembly code":
> > > > > > 
> > > > > > ```
> > > > > > struct SC {
> > > > > > char a;
> > > > > > char b;
> > > > > > char c;
> > > > > > };
> > > > > > int fB(int a, double b, int i1, int i2, int i3);
> > > > > > int fC(int a, struct SC c, int i1, int i2, int i3);
> > > > > > int fA(int a, double b, struct SC c, int i1, int i2, int i3) {
> > > > > > return fB(a, b, i1, i2, i3) + fC(a, c, i1, i2, i3);
> > > > > > }
> > > > > > ```
> > > > > > 
> > > > > > x64 version IR for fA is:
> > > > > > ```
> > > > > > define dso_local i32 @fA(i32 noundef %a, double noundef %b, ptr 
> > > > > > nocapture noundef readonly %c, i32 noundef %i1, i32 noundef %i2, 
> > > > > > i32 noundef %i3) local_unnamed_addr #0 { ... }
> > > > > > ```
> > > > > > aarch64 version IR for fA is:
> > > > > > 
> > > > > > ```
> > > > > > define dso_local i32 @"#fA"(i32 noundef %a, double noundef %b, i64 
> > > > > > %c.coerce, i32 noundef %i1, i32 noundef %i2, i32 noundef %i3) #0 
> > > > > > {...}
> > > > > > ```
> > > > > > Arm64 will allow any size structure to be assigned to a register 
> > > > > > directly. x64 only allows sizes 1, 2, 4 and 8. 
> > > > > > Entry thunk follow x64 version function type. But we only have 
> > > > > > aarch64 version function type.
> > > > > > 
> > > > > > I think the best way to do is create a x64 version codeGenModule 
> > > > > > and use the x64 CGM to generate the function type for entry thunk. 
> > > > > > But it is hard for me to do here. I tried a little but a lot of 
> > > > > > issues happen.
> > > > > > 
> > > > > > One other way is only modify 
> > > > > > `AArch64ABIInfo::classifyArgumentType`, copy the x64 code into the 
> > > > > > function and add a flag to determine which version will the 
> > > > > > function use. It is easier but I'm not sure it is the only 
> > > > > > difference between x64 and aarch64. Maybe the classify return also 
> > > > > > need to do this. And it is not a clean way I think.
> > > > > Oh, that's annoying... I hadn't considered the case of a struct of 
> > > > > size 3/5/6/7.
> > > > > 
> > > > > Like I noted on D126811, attaching thunks to calls is tricky if we 
> > > > > try to do it from clang.
> > > > > 
> > > > > Computing the right IR type shouldn't be that hard by itself; we can 
> > > > > call into call lowering code in TargetInfo without modifying much 
> > > > > else.  (We just need a bit to tell the TargetInfo to redirect the 
> > > > > call, like D125419.  Use an entry point like 
> > > > > CodeGenTypes::arrangeCall.)  You don't need to mess with the type 
> > > > > system or anything like that.
> > > > > 
> > > > > The problem is correctly representing the lowered call in IR; we 
> > > > > really don't want to do lowering early because it will block 
> > > > > optimizations.  I considered using an operand bundle; we can probably 
> > > > > make that work, but it's complicated, and probably disables some 
> > > > > optimizations.
> > > > > 
> > > > > I think the best thing we can do here is add an IR attribute to mark 
> > > > > arguments which are passed directly on AArch64, but need to be passed 
> > > > > indirectly for the x64 ABI.  Then AArch64Arm64ECCallLowering can 
> > > > > check for the attribute and modify its behavior.  This isn't really 
> > > > > clean in the sense that it's specific to the x64/aarch64 pair of 
> > > > > calling conventions, but I think the alternative is worse.
> > > > It looks not only 3/5/6/7, but also all size exclusive larger than 8 
> > > > and less than 16 are difference between x86 ABI and Aarch64 ABI.
> > > > Maybe we can emit a function declaration here for the x86ABI thunk, 
> > > > then define it in Arm64ECCallLowering.
> > > > 
> > > I think the sizes between 8 and 16 work correctly already?  All sizes 
> > > greater than 8 are passed indirectly on x86, and the thunk generation 
> > > code accounts for that.  But that's not really important for the general 
> > > question.
> > > 
> > > We need to preserve the required semantics for both the AArch64 and x86 
> > > calling conventions.  There are basically the following possibilities:
> > > 
> > > - We compute the declaration of the thunk in the frontend, and attach it 
> > > to the call with an operand bundle.  Like I mentioned, I don't want to go 
> > 

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[dongjunduo via Phabricator via cfe-commits]

dongjunduo updated this revision to Diff 452058.
dongjunduo added a comment.

Add assert messages


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,10 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+assert(!TracePath.empty() && "`-ftime-trace=` is empty");
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,9 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s -###
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -94,6 +94,7 @@
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -4658,6 +4659,91 @@
   llvm_unreachable("invalid phase in ConstructPhaseAction");
 }
 
+// Infer data storing path of the options `-ftime-trace`, `-ftime-trace=`
+void InferTimeTracePath(Compilation ) {
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile =
+  C.getArgs().getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  // if `-ftime-trace` or `-ftime-trace=` are specified
+  if (TimeTrace || TimeTraceFile) {
+SmallString<128> TracePath("");
+// get the linking executable file's parent path, default is "."
+// e.g. executable file's path: /usr/local/a.out
+//  its parent's path:  /usr/local
+for (auto  : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {
+if (llvm::sys::path::has_parent_path(
+SmallString<128>(J.getOutputFilenames()[0].c_str( {
+  TracePath = llvm::sys::path::parent_path(
+  SmallString<128>(J.getOutputFilenames()[0].c_str()));
+} else {
+  TracePath = SmallString<128>(".");
+}
+break;
+  }
+}
+// Add or replace -ftime-trace` to the correct one to all clang jobs
+for (auto  : C.getJobs()) {
+  if (J.getSource().getKind() == Action::AssembleJobClass ||
+  J.getSource().getKind() == Action::BackendJobClass) {
+SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+std::string arg = std::string("-ftime-trace=");
+if (!TimeTraceFile) {
+  if (TracePath.empty()) {
+// /xxx/yyy.o => /xxx/yyy.json
+llvm::sys::path::replace_extension(OutputPath, "json");
+arg += std::string(OutputPath.c_str());
+  } else {
+// /xxx/yyy.o => /executable_file_parent_path/yyy.json
+llvm::sys::path::append(TracePath,
+llvm::sys::path::filename(OutputPath));
+llvm::sys::path::replace_extension(TracePath, "json");
+arg += std::string(TracePath.c_str());
+  }
+} else {
+  // /full_file_path_specified or 

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/tools/driver/cc1_main.cpp:260
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+assert(!TracePath.empty());
 if (auto profilerOutput = Clang->createOutputFile(

According to llvm coding standards:
To further assist with debugging, make sure to put some kind of error message 
in the assertion statement (which is printed if the assertion is tripped).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[dongjunduo via Phabricator via cfe-commits]

dongjunduo updated this revision to Diff 452054.
dongjunduo added a comment.

Add necessary asserts


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,10 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
+assert(!TracePath.empty());
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,9 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s -###
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -94,6 +94,7 @@
 #include "llvm/Support/StringSaver.h"
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
+#include 
 #include 
 #include 
 #include 
@@ -4658,6 +4659,90 @@
   llvm_unreachable("invalid phase in ConstructPhaseAction");
 }
 
+// Infer data storing path of the options `-ftime-trace`, `-ftime-trace=`
+void InferTimeTracePath(Compilation ) {
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile =
+  C.getArgs().getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  // if `-ftime-trace` or `-ftime-trace=` are specified
+  if (TimeTrace || TimeTraceFile) {
+SmallString<128> TracePath("");
+// get the linking executable file's parent path, default is "."
+// e.g. executable file's path: /usr/local/a.out
+//  its parent's path:  /usr/local
+for (auto  : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {
+if (llvm::sys::path::has_parent_path(
+SmallString<128>(J.getOutputFilenames()[0].c_str( {
+  TracePath = llvm::sys::path::parent_path(
+  SmallString<128>(J.getOutputFilenames()[0].c_str()));
+} else {
+  TracePath = SmallString<128>(".");
+}
+break;
+  }
+}
+// Add or replace -ftime-trace` to the correct one to all clang jobs
+for (auto  : C.getJobs()) {
+  if (J.getSource().getKind() == Action::AssembleJobClass ||
+  J.getSource().getKind() == Action::BackendJobClass) {
+SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+std::string arg = std::string("-ftime-trace=");
+if (!TimeTraceFile) {
+  if (TracePath.empty()) {
+// /xxx/yyy.o => /xxx/yyy.json
+llvm::sys::path::replace_extension(OutputPath, "json");
+arg += std::string(OutputPath.c_str());
+  } else {
+// /xxx/yyy.o => /executable_file_parent_path/yyy.json
+llvm::sys::path::append(TracePath,
+llvm::sys::path::filename(OutputPath));
+llvm::sys::path::replace_extension(TracePath, "json");
+arg += std::string(TracePath.c_str());
+  }
+} else {
+  // /full_file_path_specified or /path_specified/yyy.json
+  

[PATCH] D127873: [clang-format] Fix misplacement of `*` in declaration of pointer to struct

[Jack Huang via Phabricator via cfe-commits]

jackhong12 added a comment.

Right, I think we can fix this in the same way. I'll look into it soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127873

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

[PATCH] D129771: [clang-format] distinguish multiplication after brace-init from pointer

[Jack Huang via Phabricator via cfe-commits]

jackhong12 added a comment.

Right, I think we can fix this in the same way. I'll look into it soon.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129771

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

[PATCH] D131677: [clang][RISCV] Fix incorrect ABI lowering for inherited structs under hard-float ABIs

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/test/CodeGen/RISCV/riscv-abi.cpp:1
 // RUN: %clang_cc1 -triple riscv32 -emit-llvm -x c++ %s -o - \
 // RUN:   | FileCheck -check-prefixes=ILP32,ILP32-ILP32F,ILP32-ILP32F-ILP32D %s

Not sure why you need -x c++ when it's a .cpp, that should be the default.

Also C++ tests belong in CodeGenCXX surely? Though I do see a lot of .cpp files 
in CodeGen...


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

https://reviews.llvm.org/D131677

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

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

[Paul Fultz II via Phabricator via cfe-commits]

pfultz2 added a comment.

> What is the motivation for requiring these to be the arguments of call or 
> construct expressions?

It is to just to try and limit the possible false positives at first. Usually a 
function call it is not clear locally that it will be converted to an integer 
but perhaps its common for users to assign an enum it an integer?

I could extend it to support everything except explicit casts if you think that 
would be better.


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

https://reviews.llvm.org/D129570

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

[PATCH] D127873: [clang-format] Fix misplacement of `*` in declaration of pointer to struct

[Ben Smith via Phabricator via cfe-commits]

binji added a comment.

This change seems to have also regressed code like this:

  // before
  bool b = 3 == int{3} && true;
  // after
  bool b = 3 == int{3}&& true;

I suppose a similar fix to the one done for multiply should be done for `&&` 
too?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127873

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

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D131704#3717774 , @akhuang wrote:

> We're seeing this warning in code with global constants, e.g.
>
>   const Enum x = static_cast(-1);
>
> is this intended?

This is constant initialization, so this is indeed expected.

Depending on your situation you can fix this by using a fixed underlying type 
e.g. : `enum A : int` or by making it a scoped enum `enum class A` or by 
extending the enumerators themselves to include the value you are using.

If this not your code you can also turn the error into a warning using 
`-Wno-error=enum-constexpr-conversion`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131704

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

[PATCH] D131739: Fix some clang-doc issues.

[Paul Kirth via Phabricator via cfe-commits]

paulkirth added a comment.

would you mind splitting this patch into separate patches? I'd like to keep the 
fix for the assertion in SmallString separate. the rest seems to make sense 
together. otherwise LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131739

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

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I guess the other way to test for pod-for-purposes-of-ABI is IRgen. Looks like 
MSVC isn't observable based on sizeof or alignof on these issues (the previous 
godbolt shows MSVC's answer to alignment for the packed and size for the 
trailing packing don't change based on any of the variations (pod, non-pod, 
pod-with-defaulted-special-members)) - but should be observable based on the 
returning ABI.

Ah, here we go: https://godbolt.org/z/sd88zTjPP

So MSVC does consider the defaulted special member as still a valid 
pod-for-purposes-of-ABI and clang is incorrect/not ABI compatible with this. So 
MSVC does want this fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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

[PATCH] D130033: [HLSL] Add resource binding attribute for HLSL.

[Xiang Li via Phabricator via cfe-commits]

python3kgae updated this revision to Diff 452037.
python3kgae added a comment.

Add fixit for case like space 1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130033

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/HLSL/resource_binding_attr.hlsl
  clang/test/SemaHLSL/resource_binding_attr_error.hlsl

Index: clang/test/SemaHLSL/resource_binding_attr_error.hlsl
===
--- /dev/null
+++ clang/test/SemaHLSL/resource_binding_attr_error.hlsl
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -o - -fsyntax-only %s -verify
+
+// expected-error@+5 {{expected ';' after top level declarator}}
+// expected-error@+4 {{expected ')'}}
+// expected-note@+3 {{to match this '('}}
+// expected-error@+2 {{a type specifier is required for all declarations}}
+// expected-error@+1 {{illegal storage class on file-scoped variable}}
+float a : register(c0, space1);
+
+// expected-error@+1 {{invalid resource class specifier 'i' used; expected 'b', 's', 't', or 'u'}}
+cbuffer a : register(i0) {
+
+}
+// expected-error@+1 {{invalid space specifier 's2' used; expected 'space' followed by an integer, like space1}}
+cbuffer a : register(b0, s2) {
+
+}
+// expected-error@+1 {{register number should be an integer}}
+cbuffer a : register(bf, s2) {
+
+}
+// expected-error@+1 {{invalid space specifier 'spaces' used; expected 'space' followed by an integer, like space1}}
+cbuffer a : register(b2, spaces) {
+
+}
+
+// expected-error@+1 {{expected identifier}}
+cbuffer A : register() {}
+
+// expected-error@+1 {{register number should be an integer}}
+cbuffer B : register(space1) {}
+
+// expected-error@+1 {{wrong argument format for hlsl attribute, use b2 instead}}
+cbuffer B : register(b 2) {}
+
+// expected-error@+2 {{wrong argument format for hlsl attribute, use b2 instead}}
+// expected-error@+1 {{wrong argument format for hlsl attribute, use space3 instead}}
+cbuffer B : register(b 2, space 3) {}
Index: clang/test/AST/HLSL/resource_binding_attr.hlsl
===
--- /dev/null
+++ clang/test/AST/HLSL/resource_binding_attr.hlsl
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.3-library -x hlsl -ast-dump -o - %s | FileCheck %s
+
+// CHECK:HLSLBufferDecl 0x[[CB:[0-9a-f]+]] {{.*}} line:6:9 cbuffer CB
+// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}}  "b3" "space2"
+// CHECK-NEXT:VarDecl 0x[[A:[0-9a-f]+]] {{.*}} col:9 used a 'float'
+cbuffer CB : register(b3, space2) {
+  float a;
+}
+
+// CHECK:HLSLBufferDecl 0x[[TB:[0-9a-f]+]] {{.*}} line:13:9 tbuffer TB
+// CHECK-NEXT:HLSLResourceBindingAttr 0x{{[0-9a-f]+}}  "t2" "space1"
+// CHECK-NEXT:VarDecl 0x[[B:[0-9a-f]+]] {{.*}} col:9 used b 'float'
+tbuffer TB : register(t2, space1) {
+  float b;
+}
+
+float foo() {
+// CHECK: BinaryOperator 0x{{[0-9a-f]+}}  'float' '+'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}}  'float' 
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}}  'float' lvalue Var 0x[[A]] 'a' 'float'
+// CHECK-NEXT: ImplicitCastExpr 0x{{[0-9a-f]+}}  'float' 
+// CHECK-NEXT: DeclRefExpr 0x{{[0-9a-f]+}}  'float' lvalue Var 0x[[B]] 'b' 'float'
+  return a + b;
+}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6934,6 +6934,92 @@
   return HLSLShaderAttr::Create(Context, ShaderType, AL);
 }
 
+static void handleHLSLResourceBindingAttr(Sema , Decl *D,
+  const ParsedAttr ) {
+  StringRef Space = "space0";
+  StringRef Slot = "";
+
+  StringRef Str;
+  SourceLocation ArgLoc;
+  if (!AL.isArgIdent(0)) {
+S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+<< AL << AANT_ArgumentIdentifier;
+return;
+  }
+
+  IdentifierLoc *Loc = AL.getArgAsIdent(0);
+  Str = Loc->Ident->getName();
+  ArgLoc = Loc->Loc;
+
+  SourceLocation SpaceArgLoc;
+  if (AL.getNumArgs() == 2) {
+Slot = Str;
+if (!AL.isArgIdent(1)) {
+  S.Diag(AL.getLoc(), diag::err_attribute_argument_type)
+  << AL << AANT_ArgumentIdentifier;
+  return;
+}
+
+IdentifierLoc *Loc = AL.getArgAsIdent(1);
+Space = Loc->Ident->getName();
+SpaceArgLoc = Loc->Loc;
+  } else {
+Slot = Str;
+  }
+
+  // Validate.
+  if (!Slot.empty()) {
+switch (Slot[0]) {
+case 'u':
+case 'b':
+case 's':
+case 't':
+  break;
+default:
+  S.Diag(ArgLoc, diag::err_hlsl_unsupported_register_type)
+  << Slot.substr(0, 1);
+  return;
+}
+
+StringRef 

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

> So I could test this other ways that actually impact layout - like whether 
> things can be packed in tail padding (can pack in tail padding for non-pod, 
> right?). Or we could ast dump and inspect the property directly? Maybe there 
> are some other options?

Here's what the tail-padding based testing looks like: 
https://godbolt.org/z/Te3d4fYjj (though the asserts all fail on MSVC... not 
sure what size MSVC makes these structs at all)

  namespace trailing {
  struct t1 {
  int x;
  char c;
  };
  struct t2 : t1 {
  char c;  
  };
  static_assert(sizeof(t2) == sizeof(int) * 3, "");
  } // namespace trailing
  namespace trailing_nonpod {
  struct t1 {
  protected:
  int x;
  char c;
  };
  struct t2 : t1  {
  char c;
  };
  static_assert(sizeof(t2) == sizeof(int) * 2, "");
  }  // namespace trailing_nonpod
  namespace trailing_smf_defaulted_pod {
  struct t1 {
  t1() = default;
  int a;
  char c;
  };
  struct t2 : t1 {
  char c;
  };
  static_assert(sizeof(t2) == sizeof(int) * 3, "");
  }

(GCC passes all these assertions, Clang (without this patch we're reviewing) 
fails the last of them)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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

[PATCH] D131739: Fix some clang-doc issues.

[Brett Wilson via Phabricator via cfe-commits]

brettw created this revision.
brettw added a reviewer: paulkirth.
brettw added a project: clang-tools-extra.
Herald added subscribers: usaxena95, arphaman.
Herald added a project: All.
brettw requested review of this revision.
Herald added a subscriber: cfe-commits.

Always emit the TagType for RecordInfo in YAML output. Previously this omitted 
the type for "struct", considering it the default. But records in C++ don't 
really have a default type so always emitting this is more clear.

  

Fix crash on startup when generating HTML code. SmallString (surprisingly) 
can't handle self-assignment which was happening because parent_path returns a 
string piece into its argument. This avoids the self-assignment by using 
another intermediate string.

  

Emit IsTypeDef in YAML. Previously this existed only in the Representation but 
was never written. Additionally, adds IsTypeDef to the record merge operation 
which was clearing it (all RecordInfo structures are merged with am empty 
RecordInfo during the reduce phase).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131739

Files:
  clang-tools-extra/clang-doc/Representation.cpp
  clang-tools-extra/clang-doc/Representation.h
  clang-tools-extra/clang-doc/YAMLGenerator.cpp
  clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
  clang-tools-extra/unittests/clang-doc/MergeTest.cpp
  clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp

Index: clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
+++ clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp
@@ -76,6 +76,7 @@
   RecordInfo I;
   I.Name = "r";
   I.Path = "path/to/A";
+  I.IsTypeDef = true;
   I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   I.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
@@ -136,6 +137,7 @@
   - LineNumber:  12
 Filename:'test.cpp'
 TagType: Class
+IsTypeDef:   true
 Members:
   - Type:
   Name:'int'
@@ -154,6 +156,7 @@
   - USR: ''
 Name:'F'
 Path:'path/to/F'
+TagType: Struct
 Members:
   - Type:
   Name:'int'
Index: clang-tools-extra/unittests/clang-doc/MergeTest.cpp
===
--- clang-tools-extra/unittests/clang-doc/MergeTest.cpp
+++ clang-tools-extra/unittests/clang-doc/MergeTest.cpp
@@ -78,6 +78,7 @@
 TEST(MergeTest, mergeRecordInfos) {
   RecordInfo One;
   One.Name = "r";
+  One.IsTypeDef = true;
   One.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   One.DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
@@ -119,6 +120,7 @@
 
   auto Expected = std::make_unique();
   Expected->Name = "r";
+  Expected->IsTypeDef = true;
   Expected->Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace);
 
   Expected->DefLoc = Location(10, llvm::SmallString<16>{"test.cpp"});
Index: clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
===
--- clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -231,9 +231,10 @@
   if (Format == "html") {
 void *MainAddr = (void *)(intptr_t)GetExecutablePath;
 std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
+llvm::SmallString<128> NativeClangDocPath;
+llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
 llvm::SmallString<128> AssetsPath;
-llvm::sys::path::native(ClangDocPath, AssetsPath);
-AssetsPath = llvm::sys::path::parent_path(AssetsPath);
+AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
 llvm::sys::path::append(AssetsPath, "..", "share", "clang");
 llvm::SmallString<128> DefaultStylesheet;
 llvm::sys::path::native(AssetsPath, DefaultStylesheet);
Index: clang-tools-extra/clang-doc/YAMLGenerator.cpp
===
--- clang-tools-extra/clang-doc/YAMLGenerator.cpp
+++ clang-tools-extra/clang-doc/YAMLGenerator.cpp
@@ -127,7 +127,8 @@
 
 static void RecordInfoMapping(IO , RecordInfo ) {
   SymbolInfoMapping(IO, I);
-  IO.mapOptional("TagType", I.TagType, clang::TagTypeKind::TTK_Struct);
+  IO.mapOptional("TagType", I.TagType);
+  IO.mapOptional("IsTypeDef", I.IsTypeDef, false);
   IO.mapOptional("Members", I.Members);
   IO.mapOptional("Bases", I.Bases);
   IO.mapOptional("Parents", I.Parents, llvm::SmallVector());
Index: clang-tools-extra/clang-doc/Representation.h
===
--- clang-tools-extra/clang-doc/Representation.h
+++ clang-tools-extra/clang-doc/Representation.h
@@ -348,10 +348,15 @@
 
   void merge(RecordInfo &);
 
-  TagTypeKind TagType = TagTypeKind::TTK_Struct; // Type of this record

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

I guess the other packed behavior/ABI checking 
277123376ce08c98b07c154bf83e4092a5d4d3c6 


In D119051#3715939 , @aaron.ballman 
wrote:

> In D119051#3714645 , @dblaikie 
> wrote:
>
>> Realized maybe we don't need a separate driver flag for this at all, and 
>> rely only on the abi-compat flag? That seems to be how (at least some) other 
>> ABI compat changes have been handled, looking at other uses of `ClangABI` 
>> enum values.
>
> Agreed, I think this is the better approach.
>
>> There could be more testing than only the indirect result of the packing 
>> problem that first inspired this patch. Any suggestions on what might be the 
>> most direct way to test whether the type's been considered pod in this sense?
>
> I would have thought use of `__is_pod` would tell us, but I'm not seeing the 
> behavior described in the test case when using that: 
> https://godbolt.org/z/1vr3MK4KW Oddly, it seems that 
> `QualType::isCXX11PODType()` doesn't look at `PlainOldData` at all! What is 
> your expectation as to how the type trait should be behaving?

Oh, yeah, seems @rsmith and I discussed this naming/expectations issue a bit 
over here previously: https://reviews.llvm.org/D117616#inline-1132622

So I could test this other ways that actually impact layout - like whether 
things can be packed in tail padding (can pack in tail padding for non-pod, 
right?). Or we could ast dump and inspect the property directly? Maybe there 
are some other options?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

[Amy Huang via Phabricator via cfe-commits]

akhuang added a comment.

We're seeing this warning in code with global constants, e.g.

  const Enum x = static_cast(-1);

is this intended?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131704

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

[PATCH] D131675: [clang] SIGSEGV fix at clang::ASTContext::getRawCommentForDeclNoCacheImpl

[Ivan Murashko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG557e32e002ed: [clang] SIGSEGV fix at 
clang::ASTContext::getRawCommentForDeclNoCacheImpl (authored by ivanmurashko).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131675

Files:
  clang/lib/AST/ASTContext.cpp
  clang/test/AST/ast-crash-doc.cpp


Index: clang/test/AST/ast-crash-doc.cpp
===
--- /dev/null
+++ clang/test/AST/ast-crash-doc.cpp
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-module -x c++ -fmodules -I %t/Inputs -fmodule-name=aa 
%t/Inputs/module.modulemap -o %t/aa.pcm
+// RUN: rm %t/Inputs/b.h
+// RUN: not %clang_cc1 -x c++ -Wdocumentation -ast-dump-all -fmodules -I 
%t/Inputs -fmodule-file=%t/aa.pcm %t/test.cpp | FileCheck %s
+
+//--- Inputs/module.modulemap
+module aa {
+header "a.h"
+header "b.h"
+}
+
+//--- Inputs/a.h
+// empty file
+
+//--- Inputs/b.h
+/// test foo @return
+int foo();
+
+
+//--- test.cpp
+#include "a.h"
+
+/// test comment at the primary file
+
+int a = foo();
+
+
+// CHECK: TranslationUnitDecl
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -298,6 +298,9 @@
 return nullptr;
 
   const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
+  if (!File.isValid()) {
+return nullptr;
+  }
   const auto CommentsInThisFile = Comments.getCommentsInFile(File);
   if (!CommentsInThisFile || CommentsInThisFile->empty())
 return nullptr;


Index: clang/test/AST/ast-crash-doc.cpp
===
--- /dev/null
+++ clang/test/AST/ast-crash-doc.cpp
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-module -x c++ -fmodules -I %t/Inputs -fmodule-name=aa %t/Inputs/module.modulemap -o %t/aa.pcm
+// RUN: rm %t/Inputs/b.h
+// RUN: not %clang_cc1 -x c++ -Wdocumentation -ast-dump-all -fmodules -I %t/Inputs -fmodule-file=%t/aa.pcm %t/test.cpp | FileCheck %s
+
+//--- Inputs/module.modulemap
+module aa {
+header "a.h"
+header "b.h"
+}
+
+//--- Inputs/a.h
+// empty file
+
+//--- Inputs/b.h
+/// test foo @return
+int foo();
+
+
+//--- test.cpp
+#include "a.h"
+
+/// test comment at the primary file
+
+int a = foo();
+
+
+// CHECK: TranslationUnitDecl
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -298,6 +298,9 @@
 return nullptr;
 
   const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
+  if (!File.isValid()) {
+return nullptr;
+  }
   const auto CommentsInThisFile = Comments.getCommentsInFile(File);
   if (!CommentsInThisFile || CommentsInThisFile->empty())
 return nullptr;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 557e32e - [clang] SIGSEGV fix at clang::ASTContext::getRawCommentForDeclNoCacheImpl

[Ivan Murashko via cfe-commits]

Author: Ivan Murashko
Date: 2022-08-12T00:05:59+01:00
New Revision: 557e32e002edd2a5a9e728d96b098bffa33e34d0

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

LOG: [clang] SIGSEGV fix at clang::ASTContext::getRawCommentForDeclNoCacheImpl

The `File` might point to an invalid `FileID` when the AST is broken. That 
leads to clang/clangd crashes while processing comments. The relevant part of 
the crash is below
```
 #4 0x7f1d7fbf95bc std::_Rb_tree, std::_Select1st>, std::less, 
std::allocator>>::_M_lower_bound(std::_Rb_tree_node> const*, std::_Rb_tree_node_base const*, 
unsigned int const&) const /usr/include/c++/8/bits/stl_tree.h:1911:2
 #5 0x7f1d7fbf95bc std::_Rb_tree, std::_Select1st>, std::less, 
std::allocator>>::lower_bound(unsigned int const&) const 
/usr/include/c++/8/bits/stl_tree.h:1214:56
 #6 0x7f1d7fbf95bc std::map, std::allocator>>::lower_bound(unsigned int const&) const 
/usr/include/c++/8/bits/stl_map.h:1264:36
 #7 0x7f1d7fbf95bc 
clang::ASTContext::getRawCommentForDeclNoCacheImpl(clang::Decl const*, 
clang::SourceLocation, std::map, std::allocator>> const&) const 
/home/ivanmurashko/local/llvm-project/clang/lib/AST/ASTContext.cpp:226:57
```

The corresponding LIT test that reproduces the crash was also added

Same issue is described at https://bugs.llvm.org/show_bug.cgi?id=49707

Reviewed By: gribozavr2

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

Added: 
clang/test/AST/ast-crash-doc.cpp

Modified: 
clang/lib/AST/ASTContext.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index e4933fb108543..2c2f4661a95ef 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -298,6 +298,9 @@ RawComment *ASTContext::getRawCommentForDeclNoCache(const 
Decl *D) const {
 return nullptr;
 
   const FileID File = SourceMgr.getDecomposedLoc(DeclLoc).first;
+  if (!File.isValid()) {
+return nullptr;
+  }
   const auto CommentsInThisFile = Comments.getCommentsInFile(File);
   if (!CommentsInThisFile || CommentsInThisFile->empty())
 return nullptr;

diff  --git a/clang/test/AST/ast-crash-doc.cpp 
b/clang/test/AST/ast-crash-doc.cpp
new file mode 100644
index 0..c4959647fc0fb
--- /dev/null
+++ b/clang/test/AST/ast-crash-doc.cpp
@@ -0,0 +1,30 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -emit-module -x c++ -fmodules -I %t/Inputs -fmodule-name=aa 
%t/Inputs/module.modulemap -o %t/aa.pcm
+// RUN: rm %t/Inputs/b.h
+// RUN: not %clang_cc1 -x c++ -Wdocumentation -ast-dump-all -fmodules -I 
%t/Inputs -fmodule-file=%t/aa.pcm %t/test.cpp | FileCheck %s
+
+//--- Inputs/module.modulemap
+module aa {
+header "a.h"
+header "b.h"
+}
+
+//--- Inputs/a.h
+// empty file
+
+//--- Inputs/b.h
+/// test foo @return
+int foo();
+
+
+//--- test.cpp
+#include "a.h"
+
+/// test comment at the primary file
+
+int a = foo();
+
+
+// CHECK: TranslationUnitDecl



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

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie updated this revision to Diff 452023.
dblaikie added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118511

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/lib/AST/RecordLayoutBuilder.cpp
  clang/test/CodeGenCXX/warn-padded-packed.cpp

Index: clang/test/CodeGenCXX/warn-padded-packed.cpp
===
--- clang/test/CodeGenCXX/warn-padded-packed.cpp
+++ clang/test/CodeGenCXX/warn-padded-packed.cpp
@@ -146,8 +146,28 @@
   unsigned char b : 8;
 } __attribute__((packed));
 
+struct S28_non_pod {
+ protected:
+  int i;
+};
+struct S28 {
+  char c1;
+  short s1;
+  char c2;
+  S28_non_pod p1; // expected-warning {{not packing field 'p1' as it is non-POD}}
+} __attribute__((packed));
+
+struct S29_non_pod_align_1 {
+ protected:
+  char c;
+};
+struct S29 {
+  S29_non_pod_align_1 p1;
+  int i;
+} __attribute__((packed)); // no warning
+static_assert(alignof(S29) == 1, "");
 
 // The warnings are emitted when the layout of the structs is computed, so we have to use them.
 void f(S1*, S2*, S3*, S4*, S5*, S6*, S7*, S8*, S9*, S10*, S11*, S12*, S13*,
S14*, S15*, S16*, S17*, S18*, S19*, S20*, S21*, S22*, S23*, S24*, S25*,
-   S26*, S27*){}
+   S26*, S27*, S28*, S29*){}
Index: clang/lib/AST/RecordLayoutBuilder.cpp
===
--- clang/lib/AST/RecordLayoutBuilder.cpp
+++ clang/lib/AST/RecordLayoutBuilder.cpp
@@ -1890,11 +1890,6 @@
   LastBitfieldStorageUnitSize = 0;
 
   llvm::Triple Target = Context.getTargetInfo().getTriple();
-  bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
- Context.getLangOpts().getClangABICompat() <=
- LangOptions::ClangABI::Ver14 ||
- Target.isPS() || Target.isOSDarwin())) ||
- D->hasAttr();
 
   AlignRequirementKind AlignRequirement = AlignRequirementKind::None;
   CharUnits FieldSize;
@@ -1975,6 +1970,12 @@
 }
   }
 
+  bool FieldPacked = (Packed && (!FieldClass || FieldClass->isPOD() ||
+ Context.getLangOpts().getClangABICompat() <=
+ LangOptions::ClangABI::Ver14 ||
+ Target.isPS() || Target.isOSDarwin())) ||
+ D->hasAttr();
+
   // When used as part of a typedef, or together with a 'packed' attribute, the
   // 'aligned' attribute can be used to decrease alignment. In that case, it
   // overrides any computed alignment we have, and there is no need to upgrade
@@ -2025,28 +2026,34 @@
 
   // The align if the field is not packed. This is to check if the attribute
   // was unnecessary (-Wpacked).
-  CharUnits UnpackedFieldAlign =
-  !DefaultsToAIXPowerAlignment ? FieldAlign : PreferredAlign;
+  CharUnits UnpackedFieldAlign = FieldAlign; 
+  CharUnits PackedFieldAlign = CharUnits::One();
   CharUnits UnpackedFieldOffset = FieldOffset;
   CharUnits OriginalFieldAlign = UnpackedFieldAlign;
 
-  if (FieldPacked) {
-FieldAlign = CharUnits::One();
-PreferredAlign = CharUnits::One();
-  }
   CharUnits MaxAlignmentInChars =
   Context.toCharUnitsFromBits(D->getMaxAlignment());
-  FieldAlign = std::max(FieldAlign, MaxAlignmentInChars);
+  PackedFieldAlign = std::max(PackedFieldAlign, MaxAlignmentInChars);
   PreferredAlign = std::max(PreferredAlign, MaxAlignmentInChars);
   UnpackedFieldAlign = std::max(UnpackedFieldAlign, MaxAlignmentInChars);
 
   // The maximum field alignment overrides the aligned attribute.
   if (!MaxFieldAlignment.isZero()) {
-FieldAlign = std::min(FieldAlign, MaxFieldAlignment);
+PackedFieldAlign = std::min(PackedFieldAlign, MaxFieldAlignment);
 PreferredAlign = std::min(PreferredAlign, MaxFieldAlignment);
 UnpackedFieldAlign = std::min(UnpackedFieldAlign, MaxFieldAlignment);
   }
 
+
+  if (!FieldPacked)
+FieldAlign = UnpackedFieldAlign;
+  if (DefaultsToAIXPowerAlignment)
+UnpackedFieldAlign = PreferredAlign;
+  if (FieldPacked) {
+PreferredAlign = PackedFieldAlign;
+FieldAlign = PackedFieldAlign;
+  }
+
   CharUnits AlignTo =
   !DefaultsToAIXPowerAlignment ? FieldAlign : PreferredAlign;
   // Round up the current record size to the field's alignment boundary.
@@ -2129,6 +2136,9 @@
 << Context.getTypeDeclType(RD) << D->getName() << D->getType();
 }
   }
+
+  if (Packed && !FieldPacked && PackedFieldAlign < FieldAlign)
+Diag(D->getLocation(), diag::warn_unpacked_field) << D;
 }
 
 void ItaniumRecordLayoutBuilder::FinishLayout(const NamedDecl *D) {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ 

[PATCH] D118511: Add a warning for not packing non-POD members in packed structs

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.



In D118511#3373181 , @dblaikie wrote:

> In D118511#3373173 , @tstellar 
> wrote:
>
>> In D118511#3373082 , @dblaikie 
>> wrote:
>>
>>> In D118511#3372728 , @jyknight 
>>> wrote:
>>>
 In D118511#3371432 , @tstellar 
 wrote:

> I'm fine with reverting if you think this is the best solution.  I just 
> would like to conclude soon so I can make the final release candidate.

 ISTM that reverting the ABI change in the 14.x branch makes sense, to 
 avoid ping-ponging the ABI for packed structs which would become 
 non-packed (breaking ABI) in 14.x and packed again (breaking ABI) in 
 https://reviews.llvm.org/D119051.
>>>
>>> Yeah - I think it'd be a pretty niche amount of code that'd churn like 
>>> that, but doesn't seem super important to rush this either.
>>>
>>> @tstellar - can/do you want to revert this on the release branch yourself? 
>>> Is that something I should do? Should I revert this on trunk (would be a 
>>> bit awkward/more churny for users - maybe not a full revert, but one that 
>>> leaves the new ABI version flag available as a no-op so users opting out 
>>> don't need to remove the flag only to add it back in later) so it can be 
>>> integrated to the release?
>>
>> I can revert in the release branch.  Is this the commit: 
>> https://reviews.llvm.org/rG277123376ce08c98b07c154bf83e4092a5d4d3c6?
>
> Yep, that's the one!
>
>> It doesn't seem necessary to me to revert in the main branch, but I think 
>> that can be your call.
>
> Yeah, I'm leaning towards not reverting on main & hoping we can sort out the 
> POD ABI issue.

@tstellar seems we didn't sort out the POD ABI issue in time for the 15 release 
branch - though we're making some progress on it now (see discussion here: 
D119051  ) - reckon we should again revert 
the packed ABI layout in 15, to try to put this off until the 3 issues 
(ignoring packed when packing non-pod (277123376ce08c98b07c154bf83e4092a5d4d3c6 
), pod 
with defaulted special members (D119051 ), 
warning for ignoring packed due to non-pod (this patch, D118511 
)) are put together in one release?

Sorry for the churn/slow-movement on these.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118511

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

[PATCH] D131717: [ADT] Replace STLForwardCompat.h's C++17 equivalents

[Joe Loser via Phabricator via cfe-commits]

jloser updated this revision to Diff 452022.
jloser added a comment.

Fix unqualified use of `in_place` so the delegating constructor in `Optional` 
calls the one taking the `std::in_place_t` tag.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131717

Files:
  clang/include/clang/Basic/DirectoryEntry.h
  clang/include/clang/Basic/FileEntry.h
  llvm/include/llvm/ADT/Any.h
  llvm/include/llvm/ADT/FunctionExtras.h
  llvm/include/llvm/ADT/Optional.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/Support/HashBuilder.h
  llvm/unittests/ADT/OptionalTest.cpp

Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -14,7 +14,7 @@
 #include "gtest/gtest.h"
 
 #include 
-
+#include 
 
 using namespace llvm;
 
@@ -201,7 +201,7 @@
 
 TEST(OptionalTest, InPlaceConstructionNonDefaultConstructibleTest) {
   NonDefaultConstructible::ResetCounts();
-  { Optional A{in_place, 1}; }
+  { Optional A{std::in_place, 1}; }
   EXPECT_EQ(0u, NonDefaultConstructible::CopyConstructions);
   EXPECT_EQ(0u, NonDefaultConstructible::CopyAssignments);
   EXPECT_EQ(1u, NonDefaultConstructible::Destructions);
@@ -247,7 +247,7 @@
 TEST(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.has_value());
   EXPECT_TRUE(A.has_value());
@@ -266,12 +266,12 @@
 TEST(OptionalTest, InPlaceConstructionMultiArgConstructorTest) {
   MultiArgConstructor::ResetCounts();
   {
-Optional A{in_place, 1, 2};
+Optional A{std::in_place, 1, 2};
 EXPECT_TRUE(A.has_value());
 EXPECT_TRUE(A.has_value());
 EXPECT_EQ(1, A->x);
 EXPECT_EQ(2, A->y);
-Optional B{in_place, 5, false};
+Optional B{std::in_place, 5, false};
 EXPECT_TRUE(B.has_value());
 EXPECT_TRUE(B.has_value());
 EXPECT_EQ(5, B->x);
@@ -284,7 +284,7 @@
 TEST(OptionalTest, InPlaceConstructionAndEmplaceEquivalentTest) {
   MultiArgConstructor::ResetCounts();
   {
-Optional A{in_place, 1, 2};
+Optional A{std::in_place, 1, 2};
 Optional B;
 B.emplace(1, 2);
 EXPECT_EQ(0u, MultiArgConstructor::Destructions);
@@ -442,7 +442,7 @@
 
 TEST(OptionalTest, ImmovableInPlaceConstruction) {
   Immovable::ResetCounts();
-  Optional A{in_place, 4};
+  Optional A{std::in_place, 4};
   EXPECT_TRUE((bool)A);
   EXPECT_EQ(4, A->val);
   EXPECT_EQ(1u, Immovable::Constructions);
Index: llvm/include/llvm/Support/HashBuilder.h
===
--- llvm/include/llvm/Support/HashBuilder.h
+++ llvm/include/llvm/Support/HashBuilder.h
@@ -76,7 +76,7 @@
 
   template 
   explicit HashBuilderBase(ArgTypes &&...Args)
-  : OptionalHasher(in_place, std::forward(Args)...),
+  : OptionalHasher(std::in_place, std::forward(Args)...),
 Hasher(*OptionalHasher) {}
 
 private:
Index: llvm/include/llvm/ADT/STLExtras.h
===
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -155,12 +155,12 @@
 /// traits class for checking whether type T is one of any of the given
 /// types in the variadic list.
 template 
-using is_one_of = disjunction...>;
+using is_one_of = std::disjunction...>;
 
 /// traits class for checking whether type T is a base class for all
 ///  the given types in the variadic list.
 template 
-using are_base_of = conjunction...>;
+using are_base_of = std::conjunction...>;
 
 namespace detail {
 template  struct TypesAreDistinct;
@@ -1386,12 +1386,12 @@
 /// traits class for checking whether type T is one of any of the given
 /// types in the variadic list.
 template 
-using is_one_of = disjunction...>;
+using is_one_of = std::disjunction...>;
 
 /// traits class for checking whether type T is a base class for all
 ///  the given types in the variadic list.
 template 
-using are_base_of = conjunction...>;
+using are_base_of = std::conjunction...>;
 
 namespace detail {
 template  struct Visitor;
@@ -1550,7 +1550,7 @@
 template 
 // We can use qsort if the iterator type is a pointer and the underlying value
 // is trivially copyable.
-using sort_trivially_copyable = conjunction<
+using sort_trivially_copyable = std::conjunction<
 std::is_pointer,
 std::is_trivially_copyable::value_type>>;
 } // namespace detail
Index: llvm/include/llvm/ADT/Optional.h
===
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -81,7 +81,7 @@
   }
 
   template 
-  constexpr explicit OptionalStorage(in_place_t, Args &&...args)
+  constexpr explicit OptionalStorage(std::in_place_t, Args &&...args)
   : val(std::forward(args)...), hasVal(true) {}
 
   void reset() noexcept {
@@ -196,7 +196,7 @@
   OptionalStorage =(OptionalStorage &) = default;
 
  

[PATCH] D131717: [ADT] Replace STLForwardCompat.h's C++17 equivalents

[Scott Linder via Phabricator via cfe-commits]

scott.linder accepted this revision.
scott.linder added a comment.

LGTM, thank you!

I have some pending changes which would add C++20 "compat" types, but removing 
it until those land sounds fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131717

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

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.



>> There could be more testing than only the indirect result of the packing 
>> problem that first inspired this patch. Any suggestions on what might be the 
>> most direct way to test whether the type's been considered pod in this sense?
>
> I would have thought use of `__is_pod` would tell us, but I'm not seeing the 
> behavior described in the test case when using that: 
> https://godbolt.org/z/1vr3MK4KW Oddly, it seems that 
> `QualType::isCXX11PODType()` doesn't look at `PlainOldData` at all! What is 
> your expectation as to how the type trait should be behaving?

Ah, yeah, my understanding is the POD we're talking about is basically "POD for 
the purposes of Itanium layout" - independent of various language definitions 
of POD.

Which got me searching around in the code for uses of the data - it's used in 
some warnings, but also in the MSVC layout ( 
https://github.com/llvm/llvm-project/blob/c8cf669f6025bdf0fc227f3ddbbf3523a5b32f0b/clang/lib/CodeGen/MicrosoftCXXABI.cpp#L1123
 and it looks like MSVC proper uses the non-GCC-compatible choice here, so this 
patch shouldn't apply to MSVC compatibility mode... 
https://godbolt.org/z/Mvroof8n4 ).

I realized when removing the explicit flag I lost some of the overrides - for 
PS and Darwin. Since we're checking this property in two places and now it'll 
involve about 4 different checks (or should all these platforms imply the older 
ABI? At least that seems correct for PS and Darwin where their ABIs are defined 
by the older version of clang, but the MSVC case is weirder - since their ABI 
isn't defined by the older version of clang, and the ABI may need to change as 
we discover mismatches/mistakes in Clang's ABI implementation)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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

[clang] 1067745 - [Clang] Fix for Tighten restrictions on enum out of range diagnostic

[Shafik Yaghmour via cfe-commits]

Author: Shafik Yaghmour
Date: 2022-08-11T15:34:58-07:00
New Revision: 106774515b77d55edeab6e80555c410efc1f586a

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

LOG: [Clang] Fix for Tighten restrictions on enum out of range diagnostic

Ok it looks like this is a bit more subtle, I broke the llvm-test-suite file
paq8p.cpp again. We need both conditions to be true Info.EvalMode ==
EvalInfo::EM_ConstantExpression && Info.InConstantContext. We need to be in a
context that requires a constant value but also in a constant expression 
context.

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

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 67a1fa4318661..f8de0f28d1840 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -13534,6 +13534,7 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) 
{
 }
 
 if (Info.Ctx.getLangOpts().CPlusPlus && Info.InConstantContext &&
+Info.EvalMode == EvalInfo::EM_ConstantExpression &&
 DestType->isEnumeralType()) {
   const EnumType *ET = dyn_cast(DestType.getCanonicalType());
   const EnumDecl *ED = ET->getDecl();



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

[PATCH] D131655: [analyzer] Nullability: Enable analysis of non-inlined nullable objc properties.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp:85-86
 
-  PostStmt PS(CallExpr, LCtx);
+  SimpleProgramPointTag T{"ExprEngine", "Trivial Copy"};
+  PostStmt PS(CallExpr, LCtx, );
   for (ExplodedNodeSet::iterator I = Dst.begin(), E = Dst.end();

Ooops that wasn't part of the patch I should remove that.


Repository:
  rC Clang

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

https://reviews.llvm.org/D131655

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

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:31
+  diag(MatchedExpr->getBeginLoc(),
+   "Enum is implictly converted to an integral.")
+  << MatchedExpr->getSourceRange();

diagnostics in clang aren't formatted as full sentences.


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

https://reviews.llvm.org/D129570

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

[PATCH] D129570: [clang-tidy] Add new clang-tidy check to find implicit conversions from enum to integer.

[Nathan James via Phabricator via cfe-commits]

njames93 removed reviewers: artem.tamazov, yaxunl, carlosgalvezp, MaskRay, 
mstorsjo, balazske.
njames93 added a comment.
This revision now requires review to proceed.

What is the motivation for requiring these to be the arguments of call or 
construct expressions?

Would be nice to add a note with an embedded fixit to silence this warning.
Something like this, though you may need to use the Lexer to get the correct 
location to insert the `)`.

  auto Note = diag(MatchedExpr->getBeginLoc(), "insert an explicit cast to 
silence this warning", DiagnosticIDs::Note);
  if (Result.Context->getLangOpts().CPlusPlus11)
Note << FixItHint::CreateInsertion(MatchedExpr->getBeginLoc(), 
"static_cast(")
 << 
FixItHint::CreateInsertion(MatchedExpr->getEndLoc().getLocWithOffset(1), ")");
  else
Note << FixItHint::CreateInsertion(MatchedExpr->getBeginLoc(), "(int)");




Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.cpp:28
+void EnumToIntCheck::check(const MatchFinder::MatchResult ) {
+  // FIXME: Add callback implementation.
+  const auto *MatchedExpr = Result.Nodes.getNodeAs("x");

Remove this.



Comment at: clang-tools-extra/clang-tidy/bugprone/EnumToIntCheck.h:18
+
+/// FIXME: Write a short description.
+///

FIXME


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

https://reviews.llvm.org/D129570

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

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:13069
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;

This was to suppress false positives. All instances we've seen of this are in 
fact false positives.

Was there any analysis on true / false positives for this change?

At least for our code, this seems to make the warning strictly worse.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

[PATCH] D131730: __has_trivial_copy should map to __is_trivially_copyable

[Zachary Henkel via Phabricator via cfe-commits]

zahen created this revision.
zahen added reviewers: royjacobson, aaron.ballman, cor3ntin, erichkeane.
Herald added a project: All.
zahen requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Found during clang 15 RC1 testing due to the new diagnostic added by 
@royjacobson since clang 14.  Uncertain if this fix meets the bar to also be 
applied to the release branch.

If accepted, I'll need someone with commit access to submit on my behalf.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131730

Files:
  clang/docs/LanguageExtensions.rst
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/deprecated-builtins.cpp


Index: clang/test/SemaCXX/deprecated-builtins.cpp
===
--- clang/test/SemaCXX/deprecated-builtins.cpp
+++ clang/test/SemaCXX/deprecated-builtins.cpp
@@ -11,7 +11,7 @@
 a = __has_nothrow_constructor(A);  // expected-warning-re 
{{__has_nothrow_constructor {{.*}} use __is_nothrow_constructible}}
 a = __has_trivial_assign(A);  // expected-warning-re 
{{__has_trivial_assign {{.*}} use __is_trivially_assignable}}
 a = __has_trivial_move_assign(A);  // expected-warning-re 
{{__has_trivial_move_assign {{.*}} use __is_trivially_assignable}}
-a = __has_trivial_copy(A);  // expected-warning-re {{__has_trivial_copy 
{{.*}} use __is_trivially_constructible}}
+a = __has_trivial_copy(A);  // expected-warning-re {{__has_trivial_copy 
{{.*}} use __is_trivially_copyable}}
 a = __has_trivial_constructor(A);  // expected-warning-re 
{{__has_trivial_constructor {{.*}} use __is_trivially_constructible}}
 a = __has_trivial_move_constructor(A);  // expected-warning-re 
{{__has_trivial_move_constructor {{.*}} use __is_trivially_constructible}}
 a = __has_trivial_destructor(A);  // expected-warning-re 
{{__has_trivial_destructor {{.*}} use __is_trivially_destructible}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5412,6 +5412,8 @@
   Replacement = BTT_IsTriviallyAssignable;
   break;
 case UTT_HasTrivialCopy:
+  Replacement = UTT_IsTriviallyCopyable;
+  break;
 case UTT_HasTrivialDefaultConstructor:
 case UTT_HasTrivialMoveConstructor:
   Replacement = TT_IsTriviallyConstructible;
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ clang/docs/LanguageExtensions.rst
@@ -1373,7 +1373,7 @@
 * ``__has_trivial_move_assign`` (GNU, Microsoft):
   Deprecated, use ``__is_trivially_assignable`` instead.
 * ``__has_trivial_copy`` (GNU, Microsoft):
-  Deprecated, use ``__is_trivially_constructible`` instead.
+  Deprecated, use ``__is_trivially_copyable`` instead.
 * ``__has_trivial_constructor`` (GNU, Microsoft):
   Deprecated, use ``__is_trivially_constructible`` instead.
 * ``__has_trivial_move_constructor`` (GNU, Microsoft):


Index: clang/test/SemaCXX/deprecated-builtins.cpp
===
--- clang/test/SemaCXX/deprecated-builtins.cpp
+++ clang/test/SemaCXX/deprecated-builtins.cpp
@@ -11,7 +11,7 @@
 a = __has_nothrow_constructor(A);  // expected-warning-re {{__has_nothrow_constructor {{.*}} use __is_nothrow_constructible}}
 a = __has_trivial_assign(A);  // expected-warning-re {{__has_trivial_assign {{.*}} use __is_trivially_assignable}}
 a = __has_trivial_move_assign(A);  // expected-warning-re {{__has_trivial_move_assign {{.*}} use __is_trivially_assignable}}
-a = __has_trivial_copy(A);  // expected-warning-re {{__has_trivial_copy {{.*}} use __is_trivially_constructible}}
+a = __has_trivial_copy(A);  // expected-warning-re {{__has_trivial_copy {{.*}} use __is_trivially_copyable}}
 a = __has_trivial_constructor(A);  // expected-warning-re {{__has_trivial_constructor {{.*}} use __is_trivially_constructible}}
 a = __has_trivial_move_constructor(A);  // expected-warning-re {{__has_trivial_move_constructor {{.*}} use __is_trivially_constructible}}
 a = __has_trivial_destructor(A);  // expected-warning-re {{__has_trivial_destructor {{.*}} use __is_trivially_destructible}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5412,6 +5412,8 @@
   Replacement = BTT_IsTriviallyAssignable;
   break;
 case UTT_HasTrivialCopy:
+  Replacement = UTT_IsTriviallyCopyable;
+  break;
 case UTT_HasTrivialDefaultConstructor:
 case UTT_HasTrivialMoveConstructor:
   Replacement = TT_IsTriviallyConstructible;
Index: clang/docs/LanguageExtensions.rst
===
--- clang/docs/LanguageExtensions.rst
+++ 

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added a comment.

In D131528#3717509 , @shafik wrote:

> In D131528#3717141 , @gulfem wrote:
>
>> Can somebody please clarify this? Is the following code results in 
>> `undefined` behavior?
>>
>>   enum E1 {e11=-4, e12=4};
>>   E1 x2b = static_cast(8);
>>
>> `constexpr E1 x2 = static_cast(8)` seems like `undefined`, so 
>> `-Wenum-constexpr-conversion` is triggered.
>> We started seeing `-Wenum-constexpr-conversion` in our codebase after 
>> https://reviews.llvm.org/D131307, but we stopped seeing them after this 
>> review.
>> Is the first example that I show above still undefined, but 
>> `-Wenum-constexpr-conversion` is not going to warn in such cases?
>
> Yes, this is indeed undefined behavior. The initial patch effected a lot of 
> users and several third party packages and so I am trying to narrow the 
> impact by restricting the error to those cases that are constant expression 
> contexts only.
>
> So you may indeed less diagnostics now.

Thanks for your response @shafik. This is something temporary to ease the 
transition, but we will enable `-Wenum-constexpr-conversion` for non-const 
expressions at some point, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528

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

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D131528#3717141 , @gulfem wrote:

> Can somebody please clarify this? Is the following code results in 
> `undefined` behavior?
>
>   enum E1 {e11=-4, e12=4};
>   E1 x2b = static_cast(8);
>
> `constexpr E1 x2 = static_cast(8)` seems like `undefined`, so 
> `-Wenum-constexpr-conversion` is triggered.
> We started seeing `-Wenum-constexpr-conversion` in our codebase after 
> https://reviews.llvm.org/D131307, but we stopped seeing them after this 
> review.
> Is the first example that I show above still undefined, but 
> `-Wenum-constexpr-conversion` is not going to warn in such cases?

Yes, this is indeed undefined behavior. The initial patch effected a lot of 
users and several third party packages and so I am trying to narrow the impact 
by restricting the error to those cases that are constant expression contexts 
only.

So you may indeed less diagnostics now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528

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

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

Ok it looks like this is a bit more subtle, I broke the llvm-test-suite file 
`paq8p.cpp` again. We need both conditions to be true `Info.EvalMode == 
EvalInfo::EM_ConstantExpression && Info.InConstantContext`. We need to be in a 
context that requires a constant value but also in a constant expression 
context.

We are doing a little bit of an odd thing here and so this is a bit unique. I 
am doing a quick `check-clang` and will patch this up as long as that passes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131704

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

[PATCH] D131616: [clang][dataflow] Generalise match switch utility to other AST types and add a `CFGMatchSwitch` which currently handles `CFGStmt` and `CFGInitializer`.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

LGTM so far, please finish the patch and tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131616

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

[PATCH] D131448: Introduce iterator sentinel to make graph traversal implementation more efficient and cleaner

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

dexonsmith added inline comments.



Comment at: llvm/include/llvm/ADT/SCCIterator.h:165-170
+  SCCProxy operator*() const {
 assert(!CurrentSCC.empty() && "Dereferencing END SCC iterator!");
 return CurrentSCC;
   }
 
+  SCCProxy operator->() const { return operator*(); }

dblaikie wrote:
> rusyaev-roman wrote:
> > dblaikie wrote:
> > > I always forget in which cases you're allowed to return a proxy object 
> > > from an iterator - I thought some iterator concepts (maybe random access 
> > > is the level at which this kicks in?) that required something that 
> > > amounts to "there has to be a real object that outlives the iterator"
> > > 
> > > Could you refresh my memory on that/on why proxy objects are acceptable 
> > > for this iterator type? (where/how does this iterator declare what 
> > > concept it models anyway, since this removed the facade helper?)
> > A proxy object is allowed to be returned while dereferencing an `input 
> > iterator` (https://en.cppreference.com/w/cpp/named_req/InputIterator#Notes)
> > 
> > ```
> > The reference type for an input iterator that is not also a 
> > LegacyForwardIterator does not have to be a reference type: dereferencing 
> > an input iterator may return a proxy object or value_type itself by value
> > ```
> > 
> > For our case (that's `forward iterator`) we need to satisfy the following 
> > thing:
> > ```
> >  The type std::iterator_traits::reference must be exactly 
> >...
> >* const T& otherwise (It is constant), 
> > 
> > (where T is the type denoted by std::iterator_traits::value_type) 
> > ```
> > I'll also update the patch according to this point. Other things are ok for 
> > using a proxy object.
> Thanks for doing the legwork/quotations there.
> 
> so what's the solution here, if we're going to meet the forward iterator 
> requirements but want a proxy object?
IIRC, one solution is to store a proxy object inside the iterator, make `using 
value_type = ProxyT`, update what the stored proxy points at on `operator*()`, 
and return `ProxyT&` when dereferencing. But maybe I'm misremembering. (I'm 
pretty sure `iterator_facade_base` has machinery to help with this stuff, 
either way; might be worth looking at other uses of it.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131448

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

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I think I've proved to myself that there IS a compile-time regression, but it 
is reasonably minor.  I suspect a lot of it is going to be the 'friend' 
comparisons not caching, which I might look into doing.


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

https://reviews.llvm.org/D126907

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

[PATCH] D131724: [pseudo] Eliminate an ambiguity for the empty member declaration.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

We happened to introduce a `member-declaration := ;` rule
when inlining the `member-declaration := decl-specifier-seq_opt
member-declarator-list_opt ;`.
And with the `member-declaration := empty-declaration` rule, we had two parses 
of `;`.

This patch is to restrict the grammar to eliminate the
`member-declaration := ;` rule.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131724

Files:
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp


Index: clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest 
--forest-abbrev=false | FileCheck %s
+class A {
+;
+// CHECK-NOT: member-declaration := ;
+// CHECK: member-declaration := empty-declaration
+// CHECK-NOT: member-declaration := ;
+};
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -557,7 +557,8 @@
 class-key := UNION
 member-specification := member-declaration member-specification_opt
 member-specification := access-specifier : member-specification_opt
-member-declaration := decl-specifier-seq_opt member-declarator-list_opt ;
+member-declaration := decl-specifier-seq member-declarator-list_opt ;
+member-declaration := member-declarator-list ;
 member-declaration := function-definition
 member-declaration := using-declaration
 member-declaration := using-enum-declaration


Index: clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/empty-member-declaration.cpp
@@ -0,0 +1,7 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest --forest-abbrev=false | FileCheck %s
+class A {
+;
+// CHECK-NOT: member-declaration := ;
+// CHECK: member-declaration := empty-declaration
+// CHECK-NOT: member-declaration := ;
+};
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -557,7 +557,8 @@
 class-key := UNION
 member-specification := member-declaration member-specification_opt
 member-specification := access-specifier : member-specification_opt
-member-declaration := decl-specifier-seq_opt member-declarator-list_opt ;
+member-declaration := decl-specifier-seq member-declarator-list_opt ;
+member-declaration := member-declarator-list ;
 member-declaration := function-definition
 member-declaration := using-declaration
 member-declaration := using-enum-declaration
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131717: [ADT] Replace STLForwardCompat.h's C++17 equivalents

[Joe Loser via Phabricator via cfe-commits]

jloser added a comment.

In D131717#3717363 , @MaskRay wrote:

> The llvm/include/llvm/ADT/STLForwardCompat.h and 
> llvm/unittests/ADT/STLForwardCompatTest.cpp removal can be in a separate 
> patch.

Sure. I just split them out of this patch and I'll do the removal in a 
follow-up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131717

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

[PATCH] D131717: [ADT] Replace STLForwardCompat.h's C++17 equivalents

[Joe Loser via Phabricator via cfe-commits]

jloser updated this revision to Diff 451977.
jloser added a comment.

Revert removal of these utilities and type traits from STLForwardCompat.h and 
their corresponding unit tests. The removal of these will be done in a separate 
patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131717

Files:
  clang/include/clang/Basic/DirectoryEntry.h
  clang/include/clang/Basic/FileEntry.h
  llvm/include/llvm/ADT/Any.h
  llvm/include/llvm/ADT/FunctionExtras.h
  llvm/include/llvm/ADT/Optional.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/Support/HashBuilder.h
  llvm/unittests/ADT/OptionalTest.cpp

Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -14,7 +14,7 @@
 #include "gtest/gtest.h"
 
 #include 
-
+#include 
 
 using namespace llvm;
 
@@ -201,7 +201,7 @@
 
 TEST(OptionalTest, InPlaceConstructionNonDefaultConstructibleTest) {
   NonDefaultConstructible::ResetCounts();
-  { Optional A{in_place, 1}; }
+  { Optional A{std::in_place, 1}; }
   EXPECT_EQ(0u, NonDefaultConstructible::CopyConstructions);
   EXPECT_EQ(0u, NonDefaultConstructible::CopyAssignments);
   EXPECT_EQ(1u, NonDefaultConstructible::Destructions);
@@ -247,7 +247,7 @@
 TEST(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.has_value());
   EXPECT_TRUE(A.has_value());
@@ -266,12 +266,12 @@
 TEST(OptionalTest, InPlaceConstructionMultiArgConstructorTest) {
   MultiArgConstructor::ResetCounts();
   {
-Optional A{in_place, 1, 2};
+Optional A{std::in_place, 1, 2};
 EXPECT_TRUE(A.has_value());
 EXPECT_TRUE(A.has_value());
 EXPECT_EQ(1, A->x);
 EXPECT_EQ(2, A->y);
-Optional B{in_place, 5, false};
+Optional B{std::in_place, 5, false};
 EXPECT_TRUE(B.has_value());
 EXPECT_TRUE(B.has_value());
 EXPECT_EQ(5, B->x);
@@ -284,7 +284,7 @@
 TEST(OptionalTest, InPlaceConstructionAndEmplaceEquivalentTest) {
   MultiArgConstructor::ResetCounts();
   {
-Optional A{in_place, 1, 2};
+Optional A{std::in_place, 1, 2};
 Optional B;
 B.emplace(1, 2);
 EXPECT_EQ(0u, MultiArgConstructor::Destructions);
@@ -442,7 +442,7 @@
 
 TEST(OptionalTest, ImmovableInPlaceConstruction) {
   Immovable::ResetCounts();
-  Optional A{in_place, 4};
+  Optional A{std::in_place, 4};
   EXPECT_TRUE((bool)A);
   EXPECT_EQ(4, A->val);
   EXPECT_EQ(1u, Immovable::Constructions);
Index: llvm/include/llvm/Support/HashBuilder.h
===
--- llvm/include/llvm/Support/HashBuilder.h
+++ llvm/include/llvm/Support/HashBuilder.h
@@ -76,7 +76,7 @@
 
   template 
   explicit HashBuilderBase(ArgTypes &&...Args)
-  : OptionalHasher(in_place, std::forward(Args)...),
+  : OptionalHasher(std::in_place, std::forward(Args)...),
 Hasher(*OptionalHasher) {}
 
 private:
Index: llvm/include/llvm/ADT/STLExtras.h
===
--- llvm/include/llvm/ADT/STLExtras.h
+++ llvm/include/llvm/ADT/STLExtras.h
@@ -155,12 +155,12 @@
 /// traits class for checking whether type T is one of any of the given
 /// types in the variadic list.
 template 
-using is_one_of = disjunction...>;
+using is_one_of = std::disjunction...>;
 
 /// traits class for checking whether type T is a base class for all
 ///  the given types in the variadic list.
 template 
-using are_base_of = conjunction...>;
+using are_base_of = std::conjunction...>;
 
 namespace detail {
 template  struct TypesAreDistinct;
@@ -1386,12 +1386,12 @@
 /// traits class for checking whether type T is one of any of the given
 /// types in the variadic list.
 template 
-using is_one_of = disjunction...>;
+using is_one_of = std::disjunction...>;
 
 /// traits class for checking whether type T is a base class for all
 ///  the given types in the variadic list.
 template 
-using are_base_of = conjunction...>;
+using are_base_of = std::conjunction...>;
 
 namespace detail {
 template  struct Visitor;
@@ -1550,7 +1550,7 @@
 template 
 // We can use qsort if the iterator type is a pointer and the underlying value
 // is trivially copyable.
-using sort_trivially_copyable = conjunction<
+using sort_trivially_copyable = std::conjunction<
 std::is_pointer,
 std::is_trivially_copyable::value_type>>;
 } // namespace detail
Index: llvm/include/llvm/ADT/Optional.h
===
--- llvm/include/llvm/ADT/Optional.h
+++ llvm/include/llvm/ADT/Optional.h
@@ -81,7 +81,7 @@
   }
 
   template 
-  constexpr explicit OptionalStorage(in_place_t, Args &&...args)
+  constexpr explicit OptionalStorage(std::in_place_t, Args &&...args)
   : val(std::forward(args)...), hasVal(true) {}
 
   void reset() noexcept {
@@ -196,7 +196,7 @@
   

[PATCH] D131388: [docs] Add "C++20 Modules"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/docs/CPlusPlus20Modules.rst:395-396
+
+Roughly, this theory is correct. But the problem is that it is too rough. 
Let's see what actually happens.
+For example, the behavior also depends on the optimization level, as we will 
illustrate below.
+

ChuanqiXu wrote:
> dblaikie wrote:
> > ChuanqiXu wrote:
> > > dblaikie wrote:
> > > > I'm not sure I'm able to follow the example and how it justifies the 
> > > > rough theory as inadequate to explain the motivation for modules - 
> > > > could you clarify more directly (in comments, and then we can discuss 
> > > > how to word it) what the motivation for this section is/what you're 
> > > > trying to convey?
> > > Let me answer the motivation first. The motivation comes from my personal 
> > > experience. I feel like when most people heard modules, they would ask 
> > > "how much speedup could we get"? And there are some other questions like 
> > > "why does modules speedup the compilation?". So I guess the readers of 
> > > the document may have similar questions and I try to answer it here.
> > > 
> > > The complexity theory is correct but it may be too abstract to our users. 
> > > Since the complexity theory is about the scaling. But for certain users, 
> > > the scales of their codes are temporarily fixed. So when they try to use 
> > > modules but find the speedup doesn't meet their expectation in O2. They 
> > > may feel frustrated. And it doesn't work if I say, "hey, you'll get much 
> > > better speedup if the your codes get 10x longer." I guess they won't buy 
> > > in. So what I try to do here is to manage the user's expectation to avoid 
> > > any misunderstanding.
> > > 
> > > Following off is about the explanation. For example, there are `1` module 
> > > interface and `10` users. There is a function `F` in the module interface 
> > > and the function is used by every users. And let's say we need a `T` time 
> > > to compile the function `F` and each users without the function `F`.
> > > In O0, the function `F` will get compiled completely once and get 
> > > involved in the Sema part 10 times. Due to the Sema part is relatively 
> > > fast and let's say the Sema part would take `0.1T`. Given we compile them 
> > > serially, we need `12T` to compile the project.
> > > 
> > > But if we are with optimizations, each function `F` will get involved in 
> > > optimizations and IPO in every users. And these optimizations are most 
> > > time-consuming. Let's say these optimizations will consume `0.8T`. And 
> > > the time required will be `19T`. It is easy to say the we need `20T` to 
> > > compile the project if we're using headers. So we could find the speedup 
> > > with optimization is much slower.
> > > 
> > > BTW, if we write the required time with variables, it will be `nT + mT + 
> > > T*m*additional_compilation_part`. The `additional_compilation_part ` here 
> > > corresponds to the time percentage of `Sema` or `Optimizations`. And 
> > > since `T` and `additional_compilation_part ` are both constant. So if we 
> > > write them in `O()` form, it would be `O(n+m)`.
> > > So the theory is still correct.
> > > 
> > > 
> > I think the message is getting a bit lost in the text (both in the proposed 
> > text, and the comment here).
> > 
> > "At -O0 implementations of non-inline functions defined in a module will 
> > not impact module users, but at higher optimization levels the definitions 
> > of such functions are provided to user compilations for the purposes of 
> > optimization (but definitions of these functions are still not included in 
> > the use's object file) - this means build speed at higher optimization 
> > levels may be lower than expected given -O0 experience, but does provide by 
> > more optimization opportunities"
> > 
> Yes, it is hard to talk clearly and briefly. In your suggested wording, you 
> mentioned `non-inline` function, it is accurate but bring new information to 
> this document. I'm worrying if the reader could understand it if the reader 
> don't know c++ so much.
> 
> I put the suggested wording as the conclusion paragraph for the section and 
> hope it could make the reader focus on the intention of the section.
Maybe "non-inline" could be replaced by "module implementation details" (but 
"function bodies" sounds OK too)

I think the issue for me is that the current description seems to go into more 
detail about compiler implementation details than might be helpful for a 
document at this level. I was/am hoping maybe a one paragraph summary might be 
simpler/more approachable/sufficiently accurate for the audience.


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

https://reviews.llvm.org/D131388

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

[PATCH] D131718: [HLSL] Add abs library function

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 451974.
beanz added a comment.

Adding missing test coverage for 64-bit types


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131718

Files:
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/test/CodeGenHLSL/builtins/abs.hlsl


Index: clang/test/CodeGenHLSL/builtins/abs.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/builtins/abs.hlsl
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple 
dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes -o - | 
FileCheck %s
+
+
+float abs_float(float X) {
+  return abs(X);
+}
+
+// CHECK: define noundef float @"?abs_float@@YAMM@Z"(
+// CHECK: call float @llvm.fabs.f32(float %0)
+
+double abs_double(double X) {
+  return abs(X);
+}
+
+// CHECK: define noundef double @"?abs_double@@YANN@Z"(
+// CHECK: call double @llvm.fabs.f64(double %0)
+
+int abs_int(int X) {
+  return abs(X);
+}
+
+// CHECK: define noundef i32 @"?abs_int@@YAHH@Z"(i32
+// CHECK: [[A_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i32 [[A:%.*]], ptr [[A_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
+// CHECK-NEXT:[[NEG:%.*]] = sub nsw i32 0, [[TMP0]]
+// CHECK-NEXT:[[ABSCOND:%.*]] = icmp slt i32 [[TMP0]], 0
+// CHECK-NEXT:[[ABS:%.*]] = select i1 [[ABSCOND]], i32 [[NEG]], i32 
[[TMP0]]
+// CHECK-NEXT:ret i32 [[ABS]]
+
+int64_t abs_int64(int64_t X) {
+  return abs(X);
+}
+
+// CHECK: define noundef i64 @"?abs_int64@@YAJJ@Z"(i64
+// CHECK: [[A_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:store i64 [[A:%.*]], ptr [[A_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load i64, ptr [[A_ADDR]], align 8
+// CHECK-NEXT:[[NEG:%.*]] = sub nsw i64 0, [[TMP0]]
+// CHECK-NEXT:[[ABSCOND:%.*]] = icmp slt i64 [[TMP0]], 0
+// CHECK-NEXT:[[ABS:%.*]] = select i1 [[ABSCOND]], i64 [[NEG]], i64 
[[TMP0]]
+// CHECK-NEXT:ret i64 [[ABS]]
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -12,4 +12,9 @@
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) 
uint
 WaveActiveCountBits(bool bBit);
 
+__attribute__((clang_builtin_alias(__builtin_abs))) int abs(int In);
+__attribute__((clang_builtin_alias(__builtin_labs))) int64_t abs(int64_t In);
+__attribute__((clang_builtin_alias(__builtin_fabsf))) float abs(float In);
+__attribute__((clang_builtin_alias(__builtin_fabs))) double abs(double In);
+
 #endif //_HLSL_HLSL_INTRINSICS_H_


Index: clang/test/CodeGenHLSL/builtins/abs.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/builtins/abs.hlsl
@@ -0,0 +1,42 @@
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+
+float abs_float(float X) {
+  return abs(X);
+}
+
+// CHECK: define noundef float @"?abs_float@@YAMM@Z"(
+// CHECK: call float @llvm.fabs.f32(float %0)
+
+double abs_double(double X) {
+  return abs(X);
+}
+
+// CHECK: define noundef double @"?abs_double@@YANN@Z"(
+// CHECK: call double @llvm.fabs.f64(double %0)
+
+int abs_int(int X) {
+  return abs(X);
+}
+
+// CHECK: define noundef i32 @"?abs_int@@YAHH@Z"(i32
+// CHECK: [[A_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i32 [[A:%.*]], ptr [[A_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
+// CHECK-NEXT:[[NEG:%.*]] = sub nsw i32 0, [[TMP0]]
+// CHECK-NEXT:[[ABSCOND:%.*]] = icmp slt i32 [[TMP0]], 0
+// CHECK-NEXT:[[ABS:%.*]] = select i1 [[ABSCOND]], i32 [[NEG]], i32 [[TMP0]]
+// CHECK-NEXT:ret i32 [[ABS]]
+
+int64_t abs_int64(int64_t X) {
+  return abs(X);
+}
+
+// CHECK: define noundef i64 @"?abs_int64@@YAJJ@Z"(i64
+// CHECK: [[A_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:store i64 [[A:%.*]], ptr [[A_ADDR]], align 8
+// CHECK-NEXT:[[TMP0:%.*]] = load i64, ptr [[A_ADDR]], align 8
+// CHECK-NEXT:[[NEG:%.*]] = sub nsw i64 0, [[TMP0]]
+// CHECK-NEXT:[[ABSCOND:%.*]] = icmp slt i64 [[TMP0]], 0
+// CHECK-NEXT:[[ABS:%.*]] = select i1 [[ABSCOND]], i64 [[NEG]], i64 [[TMP0]]
+// CHECK-NEXT:ret i64 [[ABS]]
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -12,4 +12,9 @@
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) uint
 WaveActiveCountBits(bool bBit);
 
+__attribute__((clang_builtin_alias(__builtin_abs))) int abs(int In);
+__attribute__((clang_builtin_alias(__builtin_labs))) int64_t abs(int64_t In);

[PATCH] D131718: [HLSL] Add abs library function

[Xiang Li via Phabricator via cfe-commits]

python3kgae added inline comments.



Comment at: clang/lib/Headers/hlsl/hlsl_intrinsics.h:17
+__attribute__((clang_builtin_alias(__builtin_labs))) int64_t abs(int64_t In);
+__attribute__((clang_builtin_alias(__builtin_fabsf))) float abs(float In);
+__attribute__((clang_builtin_alias(__builtin_fabs))) double abs(double In);

How to add 16bit abs?
Something like this?

#ifdef __HLSL_ENABLE_16_BIT
__attribute__((clang_builtin_alias(__builtin_fabsf16))) half abs(half In);
#endif


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131718

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

[PATCH] D131717: [ADT] Replace STLForwardCompat.h's C++17 equivalents

[Fangrui Song via Phabricator via cfe-commits]

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

The llvm/include/llvm/ADT/STLForwardCompat.h and 
llvm/unittests/ADT/STLForwardCompatTest.cpp removal can be in a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131717

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

[PATCH] D131720: [pseudo] Apply the function-declarator to member functions.

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: kadircet.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

A followup patch of d489b3807f096584175c321ce7f20e9dcd49b1da 
, but for
member functions, this will eliminate a false parse of member
declaration.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131720

Files:
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp


Index: clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Similiar to declarator-function.cpp, but for member functions.
+class Foo {
+  void foo() {};
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+// CHECK: member-declaration~function-definition := decl-specifier-seq 
function-declarator function-body
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+};
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -570,9 +570,9 @@
 member-declaration := empty-declaration
 member-declarator-list := member-declarator
 member-declarator-list := member-declarator-list , member-declarator
-member-declarator := declarator virt-specifier-seq_opt pure-specifier_opt
-member-declarator := declarator requires-clause
-member-declarator := declarator brace-or-equal-initializer
+member-declarator := function-declarator virt-specifier-seq_opt 
pure-specifier_opt
+member-declarator := function-declarator requires-clause
+member-declarator := non-function-declarator brace-or-equal-initializer_opt
 member-declarator := IDENTIFIER_opt : constant-expression 
brace-or-equal-initializer_opt
 virt-specifier-seq := virt-specifier
 virt-specifier-seq := virt-specifier-seq virt-specifier


Index: clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/declator-member-function.cpp
@@ -0,0 +1,9 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+
+// Similiar to declarator-function.cpp, but for member functions.
+class Foo {
+  void foo() {};
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+// CHECK: member-declaration~function-definition := decl-specifier-seq function-declarator function-body
+// CHECK-NOT: member-declarator := declarator brace-or-equal-initializer
+};
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -570,9 +570,9 @@
 member-declaration := empty-declaration
 member-declarator-list := member-declarator
 member-declarator-list := member-declarator-list , member-declarator
-member-declarator := declarator virt-specifier-seq_opt pure-specifier_opt
-member-declarator := declarator requires-clause
-member-declarator := declarator brace-or-equal-initializer
+member-declarator := function-declarator virt-specifier-seq_opt pure-specifier_opt
+member-declarator := function-declarator requires-clause
+member-declarator := non-function-declarator brace-or-equal-initializer_opt
 member-declarator := IDENTIFIER_opt : constant-expression brace-or-equal-initializer_opt
 virt-specifier-seq := virt-specifier
 virt-specifier-seq := virt-specifier-seq virt-specifier
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131718: [HLSL] Add abs library function

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: tex3d, python3kgae, bogner, pow2clk, bob80905.
Herald added a subscriber: Anastasia.
Herald added a project: All.
beanz requested review of this revision.
Herald added a project: clang.

This change exposes the abs library function for HLSL scalar types. Abs
is supported for all scalar, vector and matrix types. This patch only
adds a subset of scalar type support.

Half support is missing in this patch due to issue #57100.

The full documentation of the HLSL abs function is available here:
https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-abs


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131718

Files:
  clang/lib/Headers/hlsl/hlsl_intrinsics.h
  clang/test/CodeGenHLSL/builtins/abs.hlsl


Index: clang/test/CodeGenHLSL/builtins/abs.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/builtins/abs.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple 
dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes -o - | 
FileCheck %s
+
+
+float abs_float(float X) {
+  return abs(X);
+}
+
+// CHECK: define noundef float @"?abs_float@@YAMM@Z"(float
+// CHECK: call float @llvm.fabs.f32(float %0)
+
+int abs_int(int X) {
+  return abs(X);
+}
+
+// CHECK: define noundef i32 @"?abs_int@@YAHH@Z"(i32
+// CHECK: [[A_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i32 [[A:%.*]], ptr [[A_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
+// CHECK-NEXT:[[NEG:%.*]] = sub nsw i32 0, [[TMP0]]
+// CHECK-NEXT:[[ABSCOND:%.*]] = icmp slt i32 [[TMP0]], 0
+// CHECK-NEXT:[[ABS:%.*]] = select i1 [[ABSCOND]], i32 [[NEG]], i32 
[[TMP0]]
+// CHECK-NEXT:ret i32 [[ABS]]
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -12,4 +12,9 @@
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) 
uint
 WaveActiveCountBits(bool bBit);
 
+__attribute__((clang_builtin_alias(__builtin_abs))) int abs(int In);
+__attribute__((clang_builtin_alias(__builtin_labs))) int64_t abs(int64_t In);
+__attribute__((clang_builtin_alias(__builtin_fabsf))) float abs(float In);
+__attribute__((clang_builtin_alias(__builtin_fabs))) double abs(double In);
+
 #endif //_HLSL_HLSL_INTRINSICS_H_


Index: clang/test/CodeGenHLSL/builtins/abs.hlsl
===
--- /dev/null
+++ clang/test/CodeGenHLSL/builtins/abs.hlsl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=hlsl2021 -finclude-default-header -x hlsl -triple dxil-pc-shadermodel6.3-library %s -emit-llvm -disable-llvm-passes -o - | FileCheck %s
+
+
+float abs_float(float X) {
+  return abs(X);
+}
+
+// CHECK: define noundef float @"?abs_float@@YAMM@Z"(float
+// CHECK: call float @llvm.fabs.f32(float %0)
+
+int abs_int(int X) {
+  return abs(X);
+}
+
+// CHECK: define noundef i32 @"?abs_int@@YAHH@Z"(i32
+// CHECK: [[A_ADDR:%.*]] = alloca i32, align 4
+// CHECK-NEXT:store i32 [[A:%.*]], ptr [[A_ADDR]], align 4
+// CHECK-NEXT:[[TMP0:%.*]] = load i32, ptr [[A_ADDR]], align 4
+// CHECK-NEXT:[[NEG:%.*]] = sub nsw i32 0, [[TMP0]]
+// CHECK-NEXT:[[ABSCOND:%.*]] = icmp slt i32 [[TMP0]], 0
+// CHECK-NEXT:[[ABS:%.*]] = select i1 [[ABSCOND]], i32 [[NEG]], i32 [[TMP0]]
+// CHECK-NEXT:ret i32 [[ABS]]
Index: clang/lib/Headers/hlsl/hlsl_intrinsics.h
===
--- clang/lib/Headers/hlsl/hlsl_intrinsics.h
+++ clang/lib/Headers/hlsl/hlsl_intrinsics.h
@@ -12,4 +12,9 @@
 __attribute__((clang_builtin_alias(__builtin_hlsl_wave_active_count_bits))) uint
 WaveActiveCountBits(bool bBit);
 
+__attribute__((clang_builtin_alias(__builtin_abs))) int abs(int In);
+__attribute__((clang_builtin_alias(__builtin_labs))) int64_t abs(int64_t In);
+__attribute__((clang_builtin_alias(__builtin_fabsf))) float abs(float In);
+__attribute__((clang_builtin_alias(__builtin_fabs))) double abs(double In);
+
 #endif //_HLSL_HLSL_INTRINSICS_H_
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

[Shafik Yaghmour via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2608f553b8fd: [Clang] Tighten restrictions on enum out of 
range diagnostic (authored by shafik).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131704

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2455,4 +2455,18 @@
   constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1);
   // expected-error@-1 {{integer value 2147483648 is outside the valid range 
of values [-2147483648, 2147483647] for this enumeration type}}
 }
+
+enum SortOrder {
+  AscendingOrder,
+  DescendingOrder
+};
+
+class A {
+  static void f(SortOrder order);
+};
+
+void A::f(SortOrder order) {
+  if (order == SortOrder(-1)) // ok, not a constant expression context
+return;
+}
 }
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13533,8 +13533,7 @@
   return Info.Ctx.getTypeSize(DestType) == Info.Ctx.getTypeSize(SrcType);
 }
 
-if (Info.Ctx.getLangOpts().CPlusPlus &&
-Info.EvalMode == EvalInfo::EM_ConstantExpression &&
+if (Info.Ctx.getLangOpts().CPlusPlus && Info.InConstantContext &&
 DestType->isEnumeralType()) {
   const EnumType *ET = dyn_cast(DestType.getCanonicalType());
   const EnumDecl *ED = ET->getDecl();


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2455,4 +2455,18 @@
   constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1);
   // expected-error@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for this enumeration type}}
 }
+
+enum SortOrder {
+  AscendingOrder,
+  DescendingOrder
+};
+
+class A {
+  static void f(SortOrder order);
+};
+
+void A::f(SortOrder order) {
+  if (order == SortOrder(-1)) // ok, not a constant expression context
+return;
+}
 }
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13533,8 +13533,7 @@
   return Info.Ctx.getTypeSize(DestType) == Info.Ctx.getTypeSize(SrcType);
 }
 
-if (Info.Ctx.getLangOpts().CPlusPlus &&
-Info.EvalMode == EvalInfo::EM_ConstantExpression &&
+if (Info.Ctx.getLangOpts().CPlusPlus && Info.InConstantContext &&
 DestType->isEnumeralType()) {
   const EnumType *ET = dyn_cast(DestType.getCanonicalType());
   const EnumDecl *ED = ET->getDecl();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 2608f55 - [Clang] Tighten restrictions on enum out of range diagnostic

[Shafik Yaghmour via cfe-commits]

Author: Shafik Yaghmour
Date: 2022-08-11T13:44:26-07:00
New Revision: 2608f553b8fd02bfd5a81d9e45406cee0c2dfe26

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

LOG: [Clang] Tighten restrictions on enum out of range diagnostic

In D131528 using Info.EvalMode == EvalInfo::EM_ConstantExpression is not strict
enough to restrict the diagnostic to only constant expression contexts. It is
sometimes set in cases where we are still determining if we are in a constant
expression context.

Using InConstantContext will tighten the restriction.

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

Added: 


Modified: 
clang/lib/AST/ExprConstant.cpp
clang/test/SemaCXX/constant-expression-cxx11.cpp

Removed: 




diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 992bc9aa5008f..67a1fa4318661 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -13533,8 +13533,7 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) 
{
   return Info.Ctx.getTypeSize(DestType) == Info.Ctx.getTypeSize(SrcType);
 }
 
-if (Info.Ctx.getLangOpts().CPlusPlus &&
-Info.EvalMode == EvalInfo::EM_ConstantExpression &&
+if (Info.Ctx.getLangOpts().CPlusPlus && Info.InConstantContext &&
 DestType->isEnumeralType()) {
   const EnumType *ET = dyn_cast(DestType.getCanonicalType());
   const EnumDecl *ED = ET->getDecl();

diff  --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp 
b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 23dfde872f121..c4f64ff9cd30a 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2455,4 +2455,18 @@ void testValueInRangeOfEnumerationValues() {
   constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1);
   // expected-error@-1 {{integer value 2147483648 is outside the valid range 
of values [-2147483648, 2147483647] for this enumeration type}}
 }
+
+enum SortOrder {
+  AscendingOrder,
+  DescendingOrder
+};
+
+class A {
+  static void f(SortOrder order);
+};
+
+void A::f(SortOrder order) {
+  if (order == SortOrder(-1)) // ok, not a constant expression context
+return;
+}
 }



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

[PATCH] D131717: [ADT] Replace STLForwardCompat.h's C++17 equivalents

[Joe Loser via Phabricator via cfe-commits]

jloser created this revision.
jloser added reviewers: scott.linder, MaskRay, bkramer, zero9178, dblaikie.
Herald added a subscriber: StephenFan.
Herald added a project: All.
jloser requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

STLForwardCompat.h defines several utilities and type traits to mimic that of
the ones in the C++17 standard library. Now that LLVM is built with the C++17
standards mode, remove these equivalents in favor of the ones from the
standard library.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131717

Files:
  clang/include/clang/Basic/DirectoryEntry.h
  clang/include/clang/Basic/FileEntry.h
  llvm/include/llvm/ADT/Any.h
  llvm/include/llvm/ADT/FunctionExtras.h
  llvm/include/llvm/ADT/Optional.h
  llvm/include/llvm/ADT/STLExtras.h
  llvm/include/llvm/ADT/STLForwardCompat.h
  llvm/include/llvm/Support/HashBuilder.h
  llvm/unittests/ADT/OptionalTest.cpp
  llvm/unittests/ADT/STLForwardCompatTest.cpp

Index: llvm/unittests/ADT/STLForwardCompatTest.cpp
===
--- llvm/unittests/ADT/STLForwardCompatTest.cpp
+++ llvm/unittests/ADT/STLForwardCompatTest.cpp
@@ -11,37 +11,6 @@
 
 namespace {
 
-TEST(STLForwardCompatTest, NegationTest) {
-  EXPECT_TRUE((llvm::negation::value));
-  EXPECT_FALSE((llvm::negation::value));
-}
-
-struct incomplete_type;
-
-TEST(STLForwardCompatTest, ConjunctionTest) {
-  EXPECT_TRUE((llvm::conjunction<>::value));
-  EXPECT_FALSE((llvm::conjunction::value));
-  EXPECT_TRUE((llvm::conjunction::value));
-  EXPECT_FALSE((llvm::conjunction::value));
-  EXPECT_FALSE((llvm::conjunction::value));
-  EXPECT_FALSE((llvm::conjunction::value));
-  EXPECT_TRUE((llvm::conjunction::value));
-  EXPECT_TRUE((llvm::conjunction::value));
-}
-
-TEST(STLForwardCompatTest, DisjunctionTest) {
-  EXPECT_FALSE((llvm::disjunction<>::value));
-  EXPECT_FALSE((llvm::disjunction::value));
-  EXPECT_TRUE((llvm::disjunction::value));
-  EXPECT_TRUE((llvm::disjunction::value));
-  EXPECT_TRUE((llvm::disjunction::value));
-  EXPECT_TRUE((llvm::disjunction::value));
-  EXPECT_TRUE((llvm::disjunction::value));
-  EXPECT_TRUE((llvm::disjunction::value));
-}
-
 template 
 class STLForwardCompatRemoveCVRefTest : public ::testing::Test {};
 
Index: llvm/unittests/ADT/OptionalTest.cpp
===
--- llvm/unittests/ADT/OptionalTest.cpp
+++ llvm/unittests/ADT/OptionalTest.cpp
@@ -14,7 +14,7 @@
 #include "gtest/gtest.h"
 
 #include 
-
+#include 
 
 using namespace llvm;
 
@@ -201,7 +201,7 @@
 
 TEST(OptionalTest, InPlaceConstructionNonDefaultConstructibleTest) {
   NonDefaultConstructible::ResetCounts();
-  { Optional A{in_place, 1}; }
+  { Optional A{std::in_place, 1}; }
   EXPECT_EQ(0u, NonDefaultConstructible::CopyConstructions);
   EXPECT_EQ(0u, NonDefaultConstructible::CopyAssignments);
   EXPECT_EQ(1u, NonDefaultConstructible::Destructions);
@@ -247,7 +247,7 @@
 TEST(OptionalTest, Emplace) {
   MultiArgConstructor::ResetCounts();
   Optional A;
-  
+
   A.emplace(1, 2);
   EXPECT_TRUE(A.has_value());
   EXPECT_TRUE(A.has_value());
@@ -266,12 +266,12 @@
 TEST(OptionalTest, InPlaceConstructionMultiArgConstructorTest) {
   MultiArgConstructor::ResetCounts();
   {
-Optional A{in_place, 1, 2};
+Optional A{std::in_place, 1, 2};
 EXPECT_TRUE(A.has_value());
 EXPECT_TRUE(A.has_value());
 EXPECT_EQ(1, A->x);
 EXPECT_EQ(2, A->y);
-Optional B{in_place, 5, false};
+Optional B{std::in_place, 5, false};
 EXPECT_TRUE(B.has_value());
 EXPECT_TRUE(B.has_value());
 EXPECT_EQ(5, B->x);
@@ -284,7 +284,7 @@
 TEST(OptionalTest, InPlaceConstructionAndEmplaceEquivalentTest) {
   MultiArgConstructor::ResetCounts();
   {
-Optional A{in_place, 1, 2};
+Optional A{std::in_place, 1, 2};
 Optional B;
 B.emplace(1, 2);
 EXPECT_EQ(0u, MultiArgConstructor::Destructions);
@@ -442,7 +442,7 @@
 
 TEST(OptionalTest, ImmovableInPlaceConstruction) {
   Immovable::ResetCounts();
-  Optional A{in_place, 4};
+  Optional A{std::in_place, 4};
   EXPECT_TRUE((bool)A);
   EXPECT_EQ(4, A->val);
   EXPECT_EQ(1u, Immovable::Constructions);
Index: llvm/include/llvm/Support/HashBuilder.h
===
--- llvm/include/llvm/Support/HashBuilder.h
+++ llvm/include/llvm/Support/HashBuilder.h
@@ -76,7 +76,7 @@
 
   template 
   explicit HashBuilderBase(ArgTypes &&...Args)
-  : OptionalHasher(in_place, std::forward(Args)...),
+  : OptionalHasher(std::in_place, std::forward(Args)...),
 Hasher(*OptionalHasher) {}
 
 private:
Index: llvm/include/llvm/ADT/STLForwardCompat.h
===
--- llvm/include/llvm/ADT/STLForwardCompat.h
+++ llvm/include/llvm/ADT/STLForwardCompat.h
@@ -21,49 +21,6 @@
 
 namespace llvm {
 

[PATCH] D129833: Use @llvm.threadlocal.address intrinsic to access TLS

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Note that this change caused LLVM to no longer be aware that a TLS variable 
cannot be NULL. Thus, code like:

  __thread int x;
  int main() {
int* y = 
return *y;
  }

built with `clang -O -fsanitize=null` emits a null-pointer check, when it 
wouldn't previously. I think llvm.threadlocal.address's return value probably 
ought to be given the nonnull attribute. We also might want to infer other 
properties of the returned pointer based on knowledge of the global value 
parameter, such as alignment.

Furthermore, while the above problem should simply be a very minor optimization 
degradation, in fact it caused miscompiles, due to a pre-existing bug in the 
X86 backend. I've sent https://reviews.llvm.org/D131716 to fix //that// part of 
the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129833

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

[PATCH] D129608: [Clang][OpenMP] Fix segmentation fault when data field is used in is_device_pt.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_is_device_ptr_codegen.cpp:296
+// CK2-DAG: [[A8:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* 
[[THIS1]], i32 0, i32 0
+// CK2-DAG: [[B9:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* 
[[THIS1]], i32 0, i32 1
 // CK2-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, 
i{{.+}} 0

What about this one? Is it used in the test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129608

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

[PATCH] D131448: Introduce iterator sentinel to make graph traversal implementation more efficient and cleaner

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: llvm/include/llvm/ADT/BreadthFirstIterator.h:128-131
+  bool operator==(iterator_sentinel) const { return VisitQueue.empty(); }
+
+  bool operator!=(iterator_sentinel RHS) const { return !(*this == RHS); }
+

rusyaev-roman wrote:
> dblaikie wrote:
> > Generally any operator that can be a non-member should be a non-member (but 
> > can still be a friend) so there's equal conversion handling for LHS and 
> > RHS. Could you make these non-members? (maybe a separate patch to do the 
> > same to the existing op overloads, so the new ones don't look weird)
> > 
> > do you need the inverse operators too, so the sentinel can appear on either 
> > side of the comparison? 
> Absolutely agree with all your points!
> 
> But I didn't want to make the code inconsistent and complicated in this 
> patch. So, I suggest making all these operators 'friend' in a separate patch, 
> otherwise it can lead to some boilerplate code like this:
> ```
>   friend bool operator==(const scc_iterator , iterator_sentinel) {
> return SCCI.isAtEnd();
>   }
> 
>   friend bool operator==(iterator_sentinel IS, const scc_iterator ) {
> return SCCI == IS;
>   }
> 
>   friend bool operator!=(const scc_iterator , iterator_sentinel IS) {
> return !(SCCI == IS);
>   }
> 
>   friend bool operator!=(const scc_iterator , iterator_sentinel IS) {
> return !(IS == SCCI);
>   }
> ```
> This boilerplate code can be avoided using special helper classes, but I 
> wouldn't like to implement them in this patch in order to keep it simple.
> 
> What do you think?
Sure sure - before or after's fine by me.



Comment at: llvm/include/llvm/ADT/SCCIterator.h:152-162
+bool hasCycle() const {
+  assert(!SCC.empty() && "Dereferencing END SCC iterator!");
+  if (SCC.size() > 1)
+return true;
+  NodeRef N = SCC.front();
+  for (ChildItTy CI = GT::child_begin(N), CE = GT::child_end(N); CI != CE;
+   ++CI)

rusyaev-roman wrote:
> dblaikie wrote:
> > I'm not quite following why this requires the proxy object - even after 
> > reading the comment above. It looks like this function is entirely in terms 
> > of the `SCC` object that's returned from `operator*` - so maybe this could 
> > be a free function, called with `hasCycle(*some_iterator)`?
> > maybe this could be a free function, called with hasCycle(*some_iterator)?
> 
> This was my initial intention.
> 
> But in the case of free function (or maybe static function of scc_iterator 
> class) a user should write the following code:
> ```
>  for (const auto& SCC : scc_traversal(Graph))
>if (hasCycle(SCC)) // or in more complicated case 
> when GraphTraits cannot be deduced from Graph type -- 
> hasCycle(SCC))
>   ...
> ```
> 
> This is the main reason of SCCProxy introduction -- to make it possible to 
> write like this:
> ```
>  for (const auto& SCC : scc_traversal(Graph))
>if (SCC.hasCycle())
>   ...
> ```
Ooh, it's the graph traits that's the extra (type) parameter. Makes sense.

Yeah, if this is workable while meeting the iterator requirements/proxy 
discussion elsewhere, sounds good.



Comment at: llvm/include/llvm/ADT/SCCIterator.h:165-170
+  SCCProxy operator*() const {
 assert(!CurrentSCC.empty() && "Dereferencing END SCC iterator!");
 return CurrentSCC;
   }
 
+  SCCProxy operator->() const { return operator*(); }

rusyaev-roman wrote:
> dblaikie wrote:
> > I always forget in which cases you're allowed to return a proxy object from 
> > an iterator - I thought some iterator concepts (maybe random access is the 
> > level at which this kicks in?) that required something that amounts to 
> > "there has to be a real object that outlives the iterator"
> > 
> > Could you refresh my memory on that/on why proxy objects are acceptable for 
> > this iterator type? (where/how does this iterator declare what concept it 
> > models anyway, since this removed the facade helper?)
> A proxy object is allowed to be returned while dereferencing an `input 
> iterator` (https://en.cppreference.com/w/cpp/named_req/InputIterator#Notes)
> 
> ```
> The reference type for an input iterator that is not also a 
> LegacyForwardIterator does not have to be a reference type: dereferencing an 
> input iterator may return a proxy object or value_type itself by value
> ```
> 
> For our case (that's `forward iterator`) we need to satisfy the following 
> thing:
> ```
>  The type std::iterator_traits::reference must be exactly 
>...
>* const T& otherwise (It is constant), 
> 
> (where T is the type denoted by std::iterator_traits::value_type) 
> ```
> I'll also update the patch according to this point. Other things are ok for 
> using a proxy object.
Thanks for doing the legwork/quotations there.

so what's the solution here, if we're going to meet the forward iterator 
requirements 

[PATCH] D131623: [clang-tidy] Improve modernize-use-emplace check

[Joey Watts via Phabricator via cfe-commits]

joeywatts updated this revision to Diff 451960.
joeywatts added a comment.

Address code review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131623

Files:
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/use-emplace.cpp
@@ -62,6 +62,9 @@
   class const_iterator {};
   const_iterator begin() { return const_iterator{}; }
 
+  void push_front(const T &) {}
+  void push_front(T &&) {}
+
   void push_back(const T &) {}
   void push_back(T &&) {}
 
@@ -86,6 +89,9 @@
   void push_back(const T &) {}
   void push_back(T &&) {}
 
+  void push_front(const T &) {}
+  void push_front(T &&) {}
+
   template 
   iterator emplace(const_iterator pos, Args &&...args){};
   template 
@@ -104,6 +110,9 @@
   class const_iterator {};
   const_iterator begin() { return const_iterator{}; }
 
+  void push_front(const T &) {}
+  void push_front(T &&) {}
+
   template 
   void emplace_front(Args &&...args){};
   template 
@@ -235,6 +244,9 @@
 public:
   using value_type = T;
 
+  void push(const T &) {}
+  void push(T &&) {}
+
   template 
   void emplace(Args &&...args){};
 };
@@ -244,6 +256,9 @@
 public:
   using value_type = T;
 
+  void push(const T &) {}
+  void push(T &&) {}
+
   template 
   void emplace(Args &&...args){};
 };
@@ -253,6 +268,9 @@
 public:
   using value_type = T;
 
+  void push(const T &) {}
+  void push(T &&) {}
+
   template 
   void emplace(Args &&...args){};
 };
@@ -667,15 +685,43 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: l.emplace_back(42, 41);
 
+  l.push_front(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_front
+  // CHECK-FIXES: l.emplace_front(42, 41);
+
   std::deque d;
   d.push_back(Something(42));
   // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_back
   // CHECK-FIXES: d.emplace_back(42);
 
+  d.push_front(Something(42));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace_front
+  // CHECK-FIXES: d.emplace_front(42);
+
   llvm::LikeASmallVector ls;
   ls.push_back(Something(42));
   // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_back
   // CHECK-FIXES: ls.emplace_back(42);
+
+  std::stack s;
+  s.push(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace
+  // CHECK-FIXES: s.emplace(42, 41);
+
+  std::queue q;
+  q.push(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: use emplace
+  // CHECK-FIXES: q.emplace(42, 41);
+
+  std::priority_queue pq;
+  pq.push(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace
+  // CHECK-FIXES: pq.emplace(42, 41);
+
+  std::forward_list fl;
+  fl.push_front(Something(42, 41));
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use emplace_front
+  // CHECK-FIXES: fl.emplace_front(42, 41);
 }
 
 class IntWrapper {
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/use-emplace.rst
@@ -4,16 +4,22 @@
 =
 
 The check flags insertions to an STL-style container done by calling the
-``push_back`` method with an explicitly-constructed temporary of the container
-element type. In this case, the corresponding ``emplace_back`` method
-results in less verbose and potentially more efficient code.
-Right now the check doesn't support ``push_front`` and ``insert``.
-It also doesn't support ``insert`` functions for associative containers
-because replacing ``insert`` with ``emplace`` may result in
+``push_back``, ``push``, or ``push_front`` methods with an
+explicitly-constructed temporary of the container element type. In this case,
+the corresponding ``emplace`` equivalent methods result in less verbose and
+potentially more efficient code.  Right now the check doesn't support
+``insert``. It also doesn't support ``insert`` functions for associative
+containers because replacing ``insert`` with ``emplace`` may result in
 `speed regression `_, but it might get support with some addition flag in the future.
 
-By default only ``std::vector``, ``std::deque``, ``std::list`` are considered.
-This list can be modified using the :option:`ContainersWithPushBack` 

[PATCH] D129608: [Clang][OpenMP] Fix segmentation fault when data field is used in is_device_pt.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 451959.
jyu2 added a comment.

Thanks Alexey.  This to fix test format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129608

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_is_device_ptr_codegen.cpp
  openmp/libomptarget/test/mapping/is_device_ptr.cpp

Index: openmp/libomptarget/test/mapping/is_device_ptr.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/is_device_ptr.cpp
@@ -0,0 +1,32 @@
+// RUN: %libomptarget-compilexx-run-and-check-generic
+
+#include 
+#include 
+#include 
+
+struct view {
+  const int size = 10;
+  int *data_host;
+  int *data_device;
+  void foo() {
+std::size_t bytes = size * sizeof(int);
+const int host_id = omp_get_initial_device();
+const int device_id = omp_get_default_device();
+data_host = (int *)malloc(bytes);
+data_device = (int *)omp_target_alloc(bytes, device_id);
+#pragma omp target teams distribute parallel for is_device_ptr(data_device)
+for (int i = 0; i < size; ++i)
+  data_device[i] = i;
+omp_target_memcpy(data_host, data_device, bytes, 0, 0, host_id, device_id);
+for (int i = 0; i < size; ++i)
+  assert(data_host[i] == i);
+  }
+};
+
+int main() {
+  view a;
+  a.foo();
+  // CHECK: PASSED
+  printf("PASSED\n");
+}
+
Index: clang/test/OpenMP/target_is_device_ptr_codegen.cpp
===
--- clang/test/OpenMP/target_is_device_ptr_codegen.cpp
+++ clang/test/OpenMP/target_is_device_ptr_codegen.cpp
@@ -252,12 +252,13 @@
 // CK2-DAG: [[BPGEP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
 // CK2-DAG: [[PGEP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
+// CK2-DAG: [[A:%.*]] = getelementptr inbounds [[STRUCT_ST:%.*]], %struct.ST* [[THIS1:%.+]], i32 0, i32 0
 // CK2-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[CBP0:%.+]] = bitcast i8** [[BP0]] to [[ST]]**
-// CK2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to [[ST]]**
-// CK2-DAG: store [[ST]]* [[VAR0:%.+]], [[ST]]** [[CBP0]]
-// CK2-DAG: store [[ST]]* [[VAR0]], [[ST]]** [[CP0]]
+// CK2-DAG: store [[ST]]* [[THIS1]], [[ST]]** [[CBP0]]
+// CK2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to double**
+// CK2-DAG: store double** [[A]], double*** [[CP0]]
 #pragma omp target is_device_ptr(a)
 {
   a++;
@@ -271,12 +272,13 @@
 // CK2-DAG: [[BPGEP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
 // CK2-DAG: [[PGEP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
+// CK2-DAG: [[B:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* [[THIS1]], i32 0, i32 1
 // CK2-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[CBP0:%.+]] = bitcast i8** [[BP0]] to [[ST]]**
-// CK2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to [[ST]]**
-// CK2-DAG: store [[ST]]* [[VAR0:%.+]], [[ST]]** [[CBP0]]
-// CK2-DAG: store [[ST]]* [[VAR0]], [[ST]]** [[CP0]]
+// CK2-DAG:  store %struct.ST* [[THIS1]], %struct.ST** [[CBP0]]
+// CH2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to double***
+// CK2-DAG: store double*** [[B]], double [[CP0]]
 #pragma omp target is_device_ptr(b)
 {
   b++;
@@ -290,12 +292,14 @@
 // CK2-DAG: [[BPGEP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
 // CK2-DAG: [[PGEP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
+// CK2-DAG: [[A8:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* [[THIS1]], i32 0, i32 0
+// CK2-DAG: [[B9:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* [[THIS1]], i32 0, i32 1
 // CK2-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[CBP0:%.+]] = bitcast i8** [[BP0]] to [[ST]]**
-// CK2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to [[ST]]**
-// CK2-DAG: store [[ST]]* [[VAR0:%.+]], [[ST]]** [[CBP0]]
-// CK2-DAG: store [[ST]]* [[VAR0]], [[ST]]** [[CP0]]
+// CK2-DAG:  store %struct.ST* [[THIS1]], %struct.ST** [[CBP0]]
+// CH2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to to double***
+// CK2-DAG: store double** [[A8]], double*** [[TMP64:%.+]]
 #pragma omp target is_device_ptr(a, b)
 {
   a++;
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -9052,7 +9052,7 @@
 // If this declaration appears in a is_device_ptr clause we just have to
 // pass the pointer by value. If it is a reference to a declaration, we just
 // pass its value.
-if (DevPointersMap.count(VD)) {
+if (VD && DevPointersMap.count(VD)) {
   CombinedInfo.Exprs.push_back(VD);
   

[PATCH] D131678: hicpp-signed-bitwise - Return location of the operand (and not of the operator beginning)

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

The pre-merge bot is failing because clang-format hasn't been run. Can you 
please update the diff by running git clang-format first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131678

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

[PATCH] D131714: [compiler-rt][builtins] Add compiler flags to catch potential errors that can lead to security vulnerabilities

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

AFAICT, on Apple's platforms, `__eprintf` is needed only on OSX and only if the 
version is older than 10.7.

https://opensource.apple.com/source/Libc/Libc-1439.40.11/include/assert.h.auto.html


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131714

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

[PATCH] D131590: Fixed page title for abseil-no-internal-dependencies check documentation

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c7b049f6eb7: [clang-tidy][docs] Fixed page title for 
abseil-no-internal-dependencies check… (authored by vladimir.plyashkun, 
committed by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131590

Files:
  clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst


Index: 
clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst
+++ clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst
@@ -1,4 +1,4 @@
-subl.. title:: clang-tidy - abseil-no-internal-dependencies
+.. title:: clang-tidy - abseil-no-internal-dependencies
 
 abseil-no-internal-dependencies
 ===


Index: clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst
+++ clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst
@@ -1,4 +1,4 @@
-subl.. title:: clang-tidy - abseil-no-internal-dependencies
+.. title:: clang-tidy - abseil-no-internal-dependencies
 
 abseil-no-internal-dependencies
 ===
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 6c7b049 - [clang-tidy][docs] Fixed page title for abseil-no-internal-dependencies check documentation

[Nathan James via cfe-commits]

Author: Vladimir Plyashkun
Date: 2022-08-11T21:09:39+01:00
New Revision: 6c7b049f6eb78edf83506a858c7b211a7c70afd8

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

LOG: [clang-tidy][docs] Fixed page title for abseil-no-internal-dependencies 
check documentation

It seems that documentation for abseil-no-internal-dependencies has invalid 
title.
This can be checked by looking at the actual web-site - 
https://clang.llvm.org/extra/clang-tidy/checks/abseil/no-internal-dependencies.html
There is redundant "subl.. title:: clang-tidy - 
abseil-no-internal-dependencies" paragraph in the beginning.

Reviewed By: njames93, sylvestre.ledru

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

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst

Removed: 




diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst 
b/clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst
index 3ddb023db8baf..c06057b313da5 100644
--- 
a/clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst
+++ 
b/clang-tools-extra/docs/clang-tidy/checks/abseil/no-internal-dependencies.rst
@@ -1,4 +1,4 @@
-subl.. title:: clang-tidy - abseil-no-internal-dependencies
+.. title:: clang-tidy - abseil-no-internal-dependencies
 
 abseil-no-internal-dependencies
 ===



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

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D129694#3717166 , @ssquare08 wrote:

> The OpenMP kernel names you mentioned are also generated separately by the 
> host and the device. Would you be okay generating declare target mangle names 
> separately by host and device using the same utility function 
> `getTargetEntryUniqueInfo`?
>
> If you still think it should only be generated only once by the host, what is 
> a good way of doing this since we can't modify the name in VarDecl?

I thought we already emitted the mangled name at least on the device side. I 
was suggesting that we just use the same name on the host so we don't need to 
worry about a host-side and device-side name difference and we can get rid of 
the extra argument to all the offload entry functions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129694

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

[PATCH] D131714: [compiler-rt][builtins] Add compiler flags to catch potential errors that can lead to security vulnerabilities

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: compiler-rt/lib/builtins/CMakeLists.txt:702
   set(BUILTIN_CFLAGS "")
+  add_security_warnings(BUILTIN_CFLAGS -1)
 

I'm passing -1 here since I wasn't sure whether the definition of `eprintf` is 
needed on non-apple platforms. If it isn't needed. we can pass 0 instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131714

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

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

[Sunil Shrestha via Phabricator via cfe-commits]

ssquare08 marked an inline comment as not done.
ssquare08 added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9439
 const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule ) const {
+  // Make sure any variable with OpenMP declare target is visible to runtime
+  // except for those with hidden visibility

ssquare08 wrote:
> jhuber6 wrote:
> > ssquare08 wrote:
> > > jhuber6 wrote:
> > > > jhuber6 wrote:
> > > > > ssquare08 wrote:
> > > > > > jhuber6 wrote:
> > > > > > > Just spitballing, is it possible to do this when we make the 
> > > > > > > global instead?
> > > > > > This is something I was wondering as well.  In 
> > > > > > CodeGenModule::GetOrCreateLLVMGlobal, when it creates a new global 
> > > > > > variable, it always uses the llvm::GlobalValue::ExternalLinkage. 
> > > > > > Seems like this changes somewhere later to internal for static 
> > > > > > globals. Do you know where that would be?
> > > > > I'm not exactly sure, I remember deleting some code in D117806 that 
> > > > > did something like that, albeit incorrectly. But I'm not sure if 
> > > > > you'd have the necessary information to check whether or not there 
> > > > > are updates attached to it. We don't want to externalize things if we 
> > > > > don't need to, otherwise we'd get a lot of our device runtime 
> > > > > variables with external visibility that now can't be optimized out.
> > > > Were you able to find a place for this when we generate the variable? 
> > > > You should be able to do something similar to the patch above if it's a 
> > > > declare target static to force it to have external visibility, but as 
> > > > mentioned before I would prefer we only do this if necessary which 
> > > > might take some extra analysis.
> > > If you are asking about the GV, it is created in 
> > > 'CodeGenModule::GetOrCreateLLVMGlobal' with external linkage always.
> > > ```
> > >   auto *GV = new llvm::GlobalVariable(
> > >   getModule(), Ty, false, llvm::GlobalValue::ExternalLinkage, nullptr,
> > >   MangledName, nullptr, llvm::GlobalVariable::NotThreadLocal,
> > >   getContext().getTargetAddressSpace(DAddrSpace));
> > > ```
> > > The linkage, however, changes in 'CodeGenModule::EmitGlobalVarDefinition' 
> > > based on the information VarDecl
> > > 
> > > ```
> > >   llvm::GlobalValue::LinkageTypes Linkage =
> > >   getLLVMLinkageVarDefinition(D, GV->isConstant());
> > > ```
> > > 
> > > Maybe you are suggesting changing the linkage information in 'VarDecl' 
> > > itself?
> > Yes, the patch I linked previously did something like that where it set the 
> > `LinkageValue` based on some information. Although I'm not sure if it would 
> > be excessively difficult to try to prune definitions that don't need to be 
> > externalized. I haven't looked too deep into this, but I believe CUDA does 
> > this inside of `adjustGVALinkageForAttributes`, there we also check some 
> > variable called `CUDADeviceVarODRUsedByHost` that I'm assuming tracks if we 
> > need to bother externalizing this.
> The exter
Thanks for the information, I'll take a look


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129694

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

[PATCH] D131714: [compiler-rt][builtins] Add compiler flags to catch potential errors that can lead to security vulnerabilities

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: dcoughlin, kubamracek, ravikandhadai, compnerd.
Herald added subscribers: Enna1, mgorny, dberris.
Herald added a project: All.
ahatanak requested review of this revision.
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

Also, fix a few places that were causing `-Wshadow` and `-Wformat-nonliteral` 
warnings to be emitted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131714

Files:
  compiler-rt/cmake/Modules/CompilerRTCompile.cmake
  compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
  compiler-rt/include/profile/InstrProfData.inc
  compiler-rt/lib/builtins/CMakeLists.txt
  compiler-rt/lib/builtins/eprintf.c
  compiler-rt/lib/profile/InstrProfiling.c
  compiler-rt/lib/profile/InstrProfilingWriter.c

Index: compiler-rt/lib/profile/InstrProfilingWriter.c
===
--- compiler-rt/lib/profile/InstrProfilingWriter.c
+++ compiler-rt/lib/profile/InstrProfilingWriter.c
@@ -291,8 +291,8 @@
 // TODO: Unfortunately the header's fields are named DataSize and
 // CountersSize when they should be named NumData and NumCounters,
 // respectively.
-const uint64_t CountersSize = NumCounters;
-const uint64_t DataSize = NumData;
+const uint64_t CountersSizeInitVal = NumCounters;
+const uint64_t DataSizeInitVal = NumData;
 /* Initialize header structure.  */
 #define INSTR_PROF_RAW_HEADER(Type, Name, Init) Header.Name = Init;
 #include "profile/InstrProfData.inc"
Index: compiler-rt/lib/profile/InstrProfiling.c
===
--- compiler-rt/lib/profile/InstrProfiling.c
+++ compiler-rt/lib/profile/InstrProfiling.c
@@ -64,11 +64,11 @@
   CurrentVSiteCount += DI->NumValueSites[VKI];
 
 for (i = 0; i < CurrentVSiteCount; ++i) {
-  ValueProfNode *CurrentVNode = ValueCounters[i];
+  ValueProfNode *CurrVNode = ValueCounters[i];
 
-  while (CurrentVNode) {
-CurrentVNode->Count = 0;
-CurrentVNode = CurrentVNode->Next;
+  while (CurrVNode) {
+CurrVNode->Count = 0;
+CurrVNode = CurrVNode->Next;
   }
 }
   }
Index: compiler-rt/lib/builtins/eprintf.c
===
--- compiler-rt/lib/builtins/eprintf.c
+++ compiler-rt/lib/builtins/eprintf.c
@@ -15,6 +15,7 @@
 //
 // It should never be exported from a dylib, so it is marked
 // visibility hidden.
+#ifndef DONT_DEFINE_EPRINTF
 #ifndef _WIN32
 __attribute__((visibility("hidden")))
 #endif
@@ -25,3 +26,4 @@
   fflush(stderr);
   compilerrt_abort();
 }
+#endif
Index: compiler-rt/lib/builtins/CMakeLists.txt
===
--- compiler-rt/lib/builtins/CMakeLists.txt
+++ compiler-rt/lib/builtins/CMakeLists.txt
@@ -699,6 +699,7 @@
   darwin_add_builtin_libraries(${BUILTIN_SUPPORTED_OS})
 else ()
   set(BUILTIN_CFLAGS "")
+  add_security_warnings(BUILTIN_CFLAGS -1)
 
   if (COMPILER_RT_HAS_FCF_PROTECTION_FLAG)
 append_list_if(COMPILER_RT_ENABLE_CET -fcf-protection=full BUILTIN_CFLAGS)
Index: compiler-rt/include/profile/InstrProfData.inc
===
--- compiler-rt/include/profile/InstrProfData.inc
+++ compiler-rt/include/profile/InstrProfData.inc
@@ -129,10 +129,10 @@
 INSTR_PROF_RAW_HEADER(uint64_t, Version, __llvm_profile_get_version())
 INSTR_PROF_RAW_HEADER(uint64_t, BinaryIdsSize, __llvm_write_binary_ids(NULL))
 /* FIXME: A more accurate name is NumData */
-INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSize)
+INSTR_PROF_RAW_HEADER(uint64_t, DataSize, DataSizeInitVal)
 INSTR_PROF_RAW_HEADER(uint64_t, PaddingBytesBeforeCounters, PaddingBytesBeforeCounters)
 /* FIXME: A more accurate name is NumCounters */
-INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSize)
+INSTR_PROF_RAW_HEADER(uint64_t, CountersSize, CountersSizeInitVal)
 INSTR_PROF_RAW_HEADER(uint64_t, PaddingBytesAfterCounters, PaddingBytesAfterCounters)
 INSTR_PROF_RAW_HEADER(uint64_t, NamesSize,  NamesSize)
 INSTR_PROF_RAW_HEADER(uint64_t, CountersDelta,
Index: compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
===
--- compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
+++ compiler-rt/cmake/Modules/CompilerRTDarwinUtils.cmake
@@ -413,6 +413,12 @@
   ../profile/InstrProfilingInternal.c
   ../profile/InstrProfilingVersionVar.c)
   foreach (os ${ARGN})
+set(macosx_sdk_version 0)
+if ("${os}" STREQUAL "osx")
+  find_darwin_sdk_version(macosx_sdk_version "macosx")
+endif()
+add_security_warnings(CFLAGS ${macosx_sdk_version})
+
 list_intersect(DARWIN_BUILTIN_ARCHS DARWIN_${os}_BUILTIN_ARCHS BUILTIN_SUPPORTED_ARCH)
 
 if((arm64 IN_LIST DARWIN_BUILTIN_ARCHS OR arm64e IN_LIST DARWIN_BUILTIN_ARCHS) AND NOT TARGET lse_builtin_symlinks)

[PATCH] D131646: [clang][dataflow] Restructure loops to call widen on back edges

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:228-234
+  // If we are at the start of a loop, we will have two precessors, but we 
don't
+  // want to join these two predecessors. Instead, we want to take the back 
edge
+  // block (i.e. the result of the previous iteration) and use that directly as
+  // the input block.
+  //
+  // For the first iteration of loop, the "zeroth" iteration state is set up by
+  // `prepareBackEdges`.

xazax.hun wrote:
> li.zhe.hua wrote:
> > xazax.hun wrote:
> > > Could you elaborate on this? Let's consider this loop:
> > > 
> > > ```
> > >  Pred
> > >|
> > >v
> > >  LoopHeader <---BackEdge
> > > ```
> > > 
> > > Do we ignore the state coming from `Pred` on purpose? Is that sound?
> > > 
> > > I would expect the analysis to always compute `join(PredState, 
> > > BackEdgeState)`, and I would expect the widening to happen between the 
> > > previous iteration of `BackEdgeState` and the current iteration of 
> > > `BackEdgeState`. So, I wonder if we already invoke the widening operator 
> > > along back edges, wouldn't the regular logic work just fine? Do I miss 
> > > something?
> > > 
> > > Do we ignore the state coming from `Pred` on purpose? Is that sound?
> > 
> > We don't, and this is what the comment
> > 
> > ```
> >   // For the first iteration of loop, the "zeroth" iteration state is set 
> > up by
> >   // `prepareBackEdges`.
> > ```
> > failed to explain. After transferring `PredState`, we copy `PredState` into 
> > `BackEdgeState`, which is done in `prepareBackEdges`.
> > 
> > > I would expect the analysis to always compute `join(PredState, 
> > > BackEdgeState)`
> > 
> > I'm not sure I see that we should always join `PredState` and 
> > `BackEdgeState`. Execution doesn't go from `Pred` into the Nth iteration of 
> > the loop, it only goes from `Pred` into the first iteration of the loop, 
> > e.g. the predecessor for the 4th iteration of the loop is only the 
> > back-edge from the 3rd iteration of the loop, not `Pred`.
> > 
> > Let me know if this makes sense.
> > I'm not sure I see that we should always join `PredState` and 
> > `BackEdgeState`. Execution doesn't go from `Pred` into the Nth iteration of 
> > the loop, it only goes from `Pred` into the first iteration of the loop, 
> > e.g. the predecessor for the 4th iteration of the loop is only the 
> > back-edge from the 3rd iteration of the loop, not `Pred`.
> > 
> > Let me know if this makes sense.
> 
> The analysis state of the dataflow analysis supposed to overestimate all of 
> the paths. Consider the following loop and imagine we want to calculate 
> intervals for integer variables:
> 
> ```
> int i = 0;
> while (...) {
>   [[program point A]];
>   ++i;
> }
> ```
> 
> During the analysis of this loop, the value `i ==0` flows to `[[program point 
> A]]`. This is the motivation to join the state from the back edge and from 
> PredState. As far as I understand, you want to achieve this by copying 
> PredState to the BackEdgeState before the first iteration. But this also 
> means that we will use the widening operator to combine PredState with the 
> state after N iterations instead of the regular join operation. I am not 
> entirely sure whether these two approaches always produce the same results. 
> 
> 
A contrived example (just came up with this in a couple minutes so feel free to 
double check if it is OK):
Consider:
```
int i = 0;
while (...) {
  if (i == 0)
i = -2;
  ++i;
}
```

Some states:
```
PredState = i : [0, 0]
BackEdgeState (after first iteration) = i : [-1, -1]
```

And the results of join vs widen:
```
PredState.join(BackEdgeState) = i : [-1, 0]
PredState.widen(BackEdge) = i : [-inf, 0]
```

The extra widening with the PredState can result in extra imprecision. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131646

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

[PATCH] D131533: [Flang][Driver] Enable PIC in the frontend

[Usman Nadeem via Phabricator via cfe-commits]

mnadeem added inline comments.



Comment at: flang/test/Driver/pic-flags.f90:3
 
-! RUN: %flang -### %s --target=aarch64-linux-gnu 2>&1 | FileCheck %s 
--check-prefix=CHECK-NOPIE
-! RUN: %flang -### %s --target=aarch64-linux-gnu -fno-pie 2>&1 | FileCheck %s 
--check-prefix=CHECK-NOPIE
-
-! RUN: %flang -### %s --target=aarch64-linux-gnu -fpie 2>&1 | FileCheck %s 
--check-prefix=CHECK-PIE
-
-! CHECK-NOPIE: "-fc1"
-! CHECk-NOPIE-NOT: "-fpic"
-! CHECK-NOPIE: "{{.*}}ld"
-! CHECK-NOPIE-NOT: "-pie"
-
-! CHECK-PIE: "-fc1"
-!! TODO Once Flang supports `-fpie`, it //should// use -fpic when invoking 
`flang -fc1`. Update the following line once `-fpie` is
-! available.
-! CHECk-PIE-NOT: "-fpic"
-! CHECK-PIE: "{{.*}}ld"
-! CHECK-PIE-NOT: "-pie"
+! RUN: %flang -v -S -emit-llvm -o - %s --target=aarch64-linux-gnu -fno-pie 
2>&1 | FileCheck %s --check-prefix=CHECK-NOPIE
+

awarzynski wrote:
> Why would you replace `-###` with `-v`? Same for other RUN lines. 
I needed the command to run because I added check lines for the emitted LLVM 
IR, for example:
! CHECK-PIE-LEVEL1: !"PIC Level", i32 1}
! CHECK-PIE-LEVEL1: !"PIE Level", i32 1}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131533

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

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

[Sunil Shrestha via Phabricator via cfe-commits]

ssquare08 added a comment.

In D129694#3717008 , @ssquare08 wrote:

> In D129694#3708297 , @jhuber6 wrote:
>
>> In D129694#3708214 , @ssquare08 
>> wrote:
>>
>>> If that's the preference I can make changes as suggested. You mentioned 
>>> CUDA and HIP mangle the declaration directly. To me it looks like they 
>>> mangle it on host and device separately. Is that not correct? If so, can 
>>> you point me to the source you are referring to?
>>
>> You're right, they mangle them separately like in 
>> https://godbolt.org/z/r6hG4brqx, this is most likely because they already 
>> had separate "device side" names. For OpenMP we currently just use the same 
>> name for the variable on the host and device side like in 
>> https://godbolt.org/z/eaGo9qsW3 where we just use the same kernel names. 
>> Thinking again, I'm still wondering if there's any utility in keeping the 
>> names separate. Correct me if I'm wrong, but the host-side variable should 
>> be able to remain internal so this mangled device name shouldn't show up in 
>> the final executable. In that case the only benefit is slightly nicer IR, 
>> which I'm not super concerned with.
>
> Yes, the host-side variable should be able to remain internal.

The OpenMP kernel names you mentioned are also generated separately by the host 
and the device. Would you be okay generating declare target mangle names 
separately by host and device using the same utility function 
`getTargetEntryUniqueInfo`?

If you still think it should only be generated only once by the host, what is a 
good way of doing this since we can't modify the name in VarDecl?




Comment at: clang/lib/CodeGen/TargetInfo.cpp:9439
 const Decl *D, llvm::GlobalValue *GV, CodeGen::CodeGenModule ) const {
+  // Make sure any variable with OpenMP declare target is visible to runtime
+  // except for those with hidden visibility

jhuber6 wrote:
> ssquare08 wrote:
> > jhuber6 wrote:
> > > jhuber6 wrote:
> > > > ssquare08 wrote:
> > > > > jhuber6 wrote:
> > > > > > Just spitballing, is it possible to do this when we make the global 
> > > > > > instead?
> > > > > This is something I was wondering as well.  In 
> > > > > CodeGenModule::GetOrCreateLLVMGlobal, when it creates a new global 
> > > > > variable, it always uses the llvm::GlobalValue::ExternalLinkage. 
> > > > > Seems like this changes somewhere later to internal for static 
> > > > > globals. Do you know where that would be?
> > > > I'm not exactly sure, I remember deleting some code in D117806 that did 
> > > > something like that, albeit incorrectly. But I'm not sure if you'd have 
> > > > the necessary information to check whether or not there are updates 
> > > > attached to it. We don't want to externalize things if we don't need 
> > > > to, otherwise we'd get a lot of our device runtime variables with 
> > > > external visibility that now can't be optimized out.
> > > Were you able to find a place for this when we generate the variable? You 
> > > should be able to do something similar to the patch above if it's a 
> > > declare target static to force it to have external visibility, but as 
> > > mentioned before I would prefer we only do this if necessary which might 
> > > take some extra analysis.
> > If you are asking about the GV, it is created in 
> > 'CodeGenModule::GetOrCreateLLVMGlobal' with external linkage always.
> > ```
> >   auto *GV = new llvm::GlobalVariable(
> >   getModule(), Ty, false, llvm::GlobalValue::ExternalLinkage, nullptr,
> >   MangledName, nullptr, llvm::GlobalVariable::NotThreadLocal,
> >   getContext().getTargetAddressSpace(DAddrSpace));
> > ```
> > The linkage, however, changes in 'CodeGenModule::EmitGlobalVarDefinition' 
> > based on the information VarDecl
> > 
> > ```
> >   llvm::GlobalValue::LinkageTypes Linkage =
> >   getLLVMLinkageVarDefinition(D, GV->isConstant());
> > ```
> > 
> > Maybe you are suggesting changing the linkage information in 'VarDecl' 
> > itself?
> Yes, the patch I linked previously did something like that where it set the 
> `LinkageValue` based on some information. Although I'm not sure if it would 
> be excessively difficult to try to prune definitions that don't need to be 
> externalized. I haven't looked too deep into this, but I believe CUDA does 
> this inside of `adjustGVALinkageForAttributes`, there we also check some 
> variable called `CUDADeviceVarODRUsedByHost` that I'm assuming tracks if we 
> need to bother externalizing this.
The exter


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129694

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

[PATCH] D131532: [Sema] Avoid isNullPointerConstant invocation

[Justin Stitt via Phabricator via cfe-commits]

justinstitt updated this revision to Diff 451955.
justinstitt added a comment.

move return to new line, as per clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131532

Files:
  clang/lib/Sema/SemaChecking.cpp


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13339,9 +13339,10 @@
 return;
 
   // Check for NULL (GNUNull) or nullptr (CXX11_nullptr).
-  const Expr::NullPointerConstantKind NullKind =
-  E->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull);
-  if (NullKind != Expr::NPCK_GNUNull && NullKind != Expr::NPCK_CXX11_nullptr)
+  const Expr *NewE = E->IgnoreParenImpCasts();
+  bool IsGNUNullExpr = isa(NewE);
+  bool HasNullPtrType = NewE->getType()->isNullPtrType();
+  if (!IsGNUNullExpr && !HasNullPtrType)
 return;
 
   // Return if target type is a safe conversion.
@@ -13358,7 +13359,7 @@
   CC = S.SourceMgr.getTopMacroCallerLoc(CC);
 
   // __null is usually wrapped in a macro.  Go up a macro if that is the case.
-  if (NullKind == Expr::NPCK_GNUNull && Loc.isMacroID()) {
+  if (IsGNUNullExpr && Loc.isMacroID()) {
 StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
 Loc, S.SourceMgr, S.getLangOpts());
 if (MacroName == "NULL")
@@ -13370,7 +13371,7 @@
 return;
 
   S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
-  << (NullKind == Expr::NPCK_CXX11_nullptr) << T << SourceRange(CC)
+  << HasNullPtrType << T << SourceRange(CC)
   << FixItHint::CreateReplacement(Loc,
   S.getFixItZeroLiteralForType(T, Loc));
 }


Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -13339,9 +13339,10 @@
 return;
 
   // Check for NULL (GNUNull) or nullptr (CXX11_nullptr).
-  const Expr::NullPointerConstantKind NullKind =
-  E->isNullPointerConstant(S.Context, Expr::NPC_ValueDependentIsNotNull);
-  if (NullKind != Expr::NPCK_GNUNull && NullKind != Expr::NPCK_CXX11_nullptr)
+  const Expr *NewE = E->IgnoreParenImpCasts();
+  bool IsGNUNullExpr = isa(NewE);
+  bool HasNullPtrType = NewE->getType()->isNullPtrType();
+  if (!IsGNUNullExpr && !HasNullPtrType)
 return;
 
   // Return if target type is a safe conversion.
@@ -13358,7 +13359,7 @@
   CC = S.SourceMgr.getTopMacroCallerLoc(CC);
 
   // __null is usually wrapped in a macro.  Go up a macro if that is the case.
-  if (NullKind == Expr::NPCK_GNUNull && Loc.isMacroID()) {
+  if (IsGNUNullExpr && Loc.isMacroID()) {
 StringRef MacroName = Lexer::getImmediateMacroNameForDiagnostics(
 Loc, S.SourceMgr, S.getLangOpts());
 if (MacroName == "NULL")
@@ -13370,7 +13371,7 @@
 return;
 
   S.Diag(Loc, diag::warn_impcast_null_pointer_to_integer)
-  << (NullKind == Expr::NPCK_CXX11_nullptr) << T << SourceRange(CC)
+  << HasNullPtrType << T << SourceRange(CC)
   << FixItHint::CreateReplacement(Loc,
   S.getFixItZeroLiteralForType(T, Loc));
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM presuming precommit CI comes back clean.


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

https://reviews.llvm.org/D131704

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

[PATCH] D125418: [Arm64EC 6/?] Implement C/C++ mangling for Arm64EC function definitions.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5128
+// to the function itself; it points to a stub for the compiler.
+// FIXME: We also need to emit an entry thunk.
+SmallString<256> MangledName;

bcl5980 wrote:
> efriedma wrote:
> > bcl5980 wrote:
> > > efriedma wrote:
> > > > bcl5980 wrote:
> > > > > A headache thing here.
> > > > > We need to get the function definition with triple x64 to define 
> > > > > entry thunk. For now the function definition here is aarch64 version.
> > > > > For example the case in Microsoft doc "Understanding Arm64EC ABI and 
> > > > > assembly code":
> > > > > 
> > > > > ```
> > > > > struct SC {
> > > > > char a;
> > > > > char b;
> > > > > char c;
> > > > > };
> > > > > int fB(int a, double b, int i1, int i2, int i3);
> > > > > int fC(int a, struct SC c, int i1, int i2, int i3);
> > > > > int fA(int a, double b, struct SC c, int i1, int i2, int i3) {
> > > > > return fB(a, b, i1, i2, i3) + fC(a, c, i1, i2, i3);
> > > > > }
> > > > > ```
> > > > > 
> > > > > x64 version IR for fA is:
> > > > > ```
> > > > > define dso_local i32 @fA(i32 noundef %a, double noundef %b, ptr 
> > > > > nocapture noundef readonly %c, i32 noundef %i1, i32 noundef %i2, i32 
> > > > > noundef %i3) local_unnamed_addr #0 { ... }
> > > > > ```
> > > > > aarch64 version IR for fA is:
> > > > > 
> > > > > ```
> > > > > define dso_local i32 @"#fA"(i32 noundef %a, double noundef %b, i64 
> > > > > %c.coerce, i32 noundef %i1, i32 noundef %i2, i32 noundef %i3) #0 {...}
> > > > > ```
> > > > > Arm64 will allow any size structure to be assigned to a register 
> > > > > directly. x64 only allows sizes 1, 2, 4 and 8. 
> > > > > Entry thunk follow x64 version function type. But we only have 
> > > > > aarch64 version function type.
> > > > > 
> > > > > I think the best way to do is create a x64 version codeGenModule and 
> > > > > use the x64 CGM to generate the function type for entry thunk. But it 
> > > > > is hard for me to do here. I tried a little but a lot of issues 
> > > > > happen.
> > > > > 
> > > > > One other way is only modify `AArch64ABIInfo::classifyArgumentType`, 
> > > > > copy the x64 code into the function and add a flag to determine which 
> > > > > version will the function use. It is easier but I'm not sure it is 
> > > > > the only difference between x64 and aarch64. Maybe the classify 
> > > > > return also need to do this. And it is not a clean way I think.
> > > > Oh, that's annoying... I hadn't considered the case of a struct of size 
> > > > 3/5/6/7.
> > > > 
> > > > Like I noted on D126811, attaching thunks to calls is tricky if we try 
> > > > to do it from clang.
> > > > 
> > > > Computing the right IR type shouldn't be that hard by itself; we can 
> > > > call into call lowering code in TargetInfo without modifying much else. 
> > > >  (We just need a bit to tell the TargetInfo to redirect the call, like 
> > > > D125419.  Use an entry point like CodeGenTypes::arrangeCall.)  You 
> > > > don't need to mess with the type system or anything like that.
> > > > 
> > > > The problem is correctly representing the lowered call in IR; we really 
> > > > don't want to do lowering early because it will block optimizations.  I 
> > > > considered using an operand bundle; we can probably make that work, but 
> > > > it's complicated, and probably disables some optimizations.
> > > > 
> > > > I think the best thing we can do here is add an IR attribute to mark 
> > > > arguments which are passed directly on AArch64, but need to be passed 
> > > > indirectly for the x64 ABI.  Then AArch64Arm64ECCallLowering can check 
> > > > for the attribute and modify its behavior.  This isn't really clean in 
> > > > the sense that it's specific to the x64/aarch64 pair of calling 
> > > > conventions, but I think the alternative is worse.
> > > It looks not only 3/5/6/7, but also all size exclusive larger than 8 and 
> > > less than 16 are difference between x86 ABI and Aarch64 ABI.
> > > Maybe we can emit a function declaration here for the x86ABI thunk, then 
> > > define it in Arm64ECCallLowering.
> > > 
> > I think the sizes between 8 and 16 work correctly already?  All sizes 
> > greater than 8 are passed indirectly on x86, and the thunk generation code 
> > accounts for that.  But that's not really important for the general 
> > question.
> > 
> > We need to preserve the required semantics for both the AArch64 and x86 
> > calling conventions.  There are basically the following possibilities:
> > 
> > - We compute the declaration of the thunk in the frontend, and attach it to 
> > the call with an operand bundle.  Like I mentioned, I don't want to go down 
> > this path: the operand bundle blocks optimizations, and it becomes more 
> > complicated for other code to generate arm64ec compatible calls.
> > - We don't compute the definition of the thunk in the frontend.  Given 
> > that, the only other way to 

[PATCH] D131646: [clang][dataflow] Restructure loops to call widen on back edges

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:228-234
+  // If we are at the start of a loop, we will have two precessors, but we 
don't
+  // want to join these two predecessors. Instead, we want to take the back 
edge
+  // block (i.e. the result of the previous iteration) and use that directly as
+  // the input block.
+  //
+  // For the first iteration of loop, the "zeroth" iteration state is set up by
+  // `prepareBackEdges`.

li.zhe.hua wrote:
> xazax.hun wrote:
> > Could you elaborate on this? Let's consider this loop:
> > 
> > ```
> >  Pred
> >|
> >v
> >  LoopHeader <---BackEdge
> > ```
> > 
> > Do we ignore the state coming from `Pred` on purpose? Is that sound?
> > 
> > I would expect the analysis to always compute `join(PredState, 
> > BackEdgeState)`, and I would expect the widening to happen between the 
> > previous iteration of `BackEdgeState` and the current iteration of 
> > `BackEdgeState`. So, I wonder if we already invoke the widening operator 
> > along back edges, wouldn't the regular logic work just fine? Do I miss 
> > something?
> > 
> > Do we ignore the state coming from `Pred` on purpose? Is that sound?
> 
> We don't, and this is what the comment
> 
> ```
>   // For the first iteration of loop, the "zeroth" iteration state is set up 
> by
>   // `prepareBackEdges`.
> ```
> failed to explain. After transferring `PredState`, we copy `PredState` into 
> `BackEdgeState`, which is done in `prepareBackEdges`.
> 
> > I would expect the analysis to always compute `join(PredState, 
> > BackEdgeState)`
> 
> I'm not sure I see that we should always join `PredState` and 
> `BackEdgeState`. Execution doesn't go from `Pred` into the Nth iteration of 
> the loop, it only goes from `Pred` into the first iteration of the loop, e.g. 
> the predecessor for the 4th iteration of the loop is only the back-edge from 
> the 3rd iteration of the loop, not `Pred`.
> 
> Let me know if this makes sense.
> I'm not sure I see that we should always join `PredState` and 
> `BackEdgeState`. Execution doesn't go from `Pred` into the Nth iteration of 
> the loop, it only goes from `Pred` into the first iteration of the loop, e.g. 
> the predecessor for the 4th iteration of the loop is only the back-edge from 
> the 3rd iteration of the loop, not `Pred`.
> 
> Let me know if this makes sense.

The analysis state of the dataflow analysis supposed to overestimate all of the 
paths. Consider the following loop and imagine we want to calculate intervals 
for integer variables:

```
int i = 0;
while (...) {
  [[program point A]];
  ++i;
}
```

During the analysis of this loop, the value `i ==0` flows to `[[program point 
A]]`. This is the motivation to join the state from the back edge and from 
PredState. As far as I understand, you want to achieve this by copying 
PredState to the BackEdgeState before the first iteration. But this also means 
that we will use the widening operator to combine PredState with the state 
after N iterations instead of the regular join operation. I am not entirely 
sure whether these two approaches always produce the same results. 




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131646

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

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

[Gulfem Savrun Yeniceri via Phabricator via cfe-commits]

gulfem added a comment.

Can somebody please clarify this? Is the following code results `undefined` 
behavior?

  enum E1 {e11=-4, e12=4};
  E1 x2b = static_cast(8);

`constexpr E1 x2 = static_cast(8)` seems like `undefined`, so 
`-Wenum-constexpr-conversion` is triggered.
We started seeing `-Wenum-constexpr-conversion` in our codebase after 
https://reviews.llvm.org/D131307, but we stopped seeing them after this review.
Is the first example that I show above still undefined, but 
`-Wenum-constexpr-conversion` is not going to warn in such cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131528

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

[PATCH] D129608: [Clang][OpenMP] Fix segmentation fault when data field is used in is_device_pt.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9080
+DeclComponentLists.emplace_back(
+MCL, OMPC_MAP_to, OMPC_MAP_MODIFIER_unknown, /*IsImpicit = */ true,
+nullptr, nullptr);

Shall it be `OMP_MAP_LITERAL`? Previously we emitted `OMP_MAP_LITERAL` for such 
items (line 9062)



Comment at: clang/test/OpenMP/target_is_device_ptr_codegen.cpp:260
+// CK2-DAG: store [[ST]]* [[THIS1]], [[ST]]** [[CBP0]]
+// CK2-DAG: store double** %a, double*** [[TMP3:%.+]]
 #pragma omp target is_device_ptr(a)

```
// CK2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to double**
// CK2-DAG: store double** [[A]], double*** [[CP0]]
```
?



Comment at: clang/test/OpenMP/target_is_device_ptr_codegen.cpp:278-279
 // CK2-DAG: [[CBP0:%.+]] = bitcast i8** [[BP0]] to [[ST]]**
-// CK2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to [[ST]]**
-// CK2-DAG: store [[ST]]* [[VAR0:%.+]], [[ST]]** [[CBP0]]
-// CK2-DAG: store [[ST]]* [[VAR0]], [[ST]]** [[CP0]]
+// CK2-DAG:  store %struct.ST* [[THIS1]], %struct.ST** [[CBP0]]
+// CK2-DAG: store double*** [[B]], double [[TMP30:%.+]]
 #pragma omp target is_device_ptr(b)

Format and checks.



Comment at: clang/test/OpenMP/target_is_device_ptr_codegen.cpp:299
+// CK2-DAG:  store %struct.ST* [[THIS1]], %struct.ST** [[CBP0]]
+// CK2-DAG: store double** [[A8]], double*** [[TMP64:%.+]]
 #pragma omp target is_device_ptr(a, b)

same


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129608

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

[PATCH] D131646: [clang][dataflow] Restructure loops to call widen on back edges

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua marked an inline comment as done.
li.zhe.hua added inline comments.



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:168-169
+// back edge block. (That is, all paths from the entry block to the back edge
+// block must go through `Block`.) It also means that there are only two
+// predecessors; the other is along the path from the entry block to `Block`.
+static const CFGBlock *findBackEdge(const CFGBlock *Block) {

xazax.hun wrote:
> Is this also true when we have multiple `continue` statements in the loop?
Yes. The end of the loop, and each of the `continue` statements, target the 
back edge block. They all get funneled through that back edge to `Block`, such 
that `Block` only has two predecessors. However, I haven't verified this in the 
CFG code, only by not finding a counterexample.



Comment at: 
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:228-234
+  // If we are at the start of a loop, we will have two precessors, but we 
don't
+  // want to join these two predecessors. Instead, we want to take the back 
edge
+  // block (i.e. the result of the previous iteration) and use that directly as
+  // the input block.
+  //
+  // For the first iteration of loop, the "zeroth" iteration state is set up by
+  // `prepareBackEdges`.

xazax.hun wrote:
> Could you elaborate on this? Let's consider this loop:
> 
> ```
>  Pred
>|
>v
>  LoopHeader <---BackEdge
> ```
> 
> Do we ignore the state coming from `Pred` on purpose? Is that sound?
> 
> I would expect the analysis to always compute `join(PredState, 
> BackEdgeState)`, and I would expect the widening to happen between the 
> previous iteration of `BackEdgeState` and the current iteration of 
> `BackEdgeState`. So, I wonder if we already invoke the widening operator 
> along back edges, wouldn't the regular logic work just fine? Do I miss 
> something?
> 
> Do we ignore the state coming from `Pred` on purpose? Is that sound?

We don't, and this is what the comment

```
  // For the first iteration of loop, the "zeroth" iteration state is set up by
  // `prepareBackEdges`.
```
failed to explain. After transferring `PredState`, we copy `PredState` into 
`BackEdgeState`, which is done in `prepareBackEdges`.

> I would expect the analysis to always compute `join(PredState, BackEdgeState)`

I'm not sure I see that we should always join `PredState` and `BackEdgeState`. 
Execution doesn't go from `Pred` into the Nth iteration of the loop, it only 
goes from `Pred` into the first iteration of the loop, e.g. the predecessor for 
the 4th iteration of the loop is only the back-edge from the 3rd iteration of 
the loop, not `Pred`.

Let me know if this makes sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131646

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

[PATCH] D129608: [Clang][OpenMP] Fix segmentation fault when data field is used in is_device_pt.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9076
+// DeclComponentLists for generating components info.
+auto It = DevPointersMap.find(VD);
+if (It != DevPointersMap.end())

ABataev wrote:
> VD is still nullptr here, why do we need to lookup for it?
VD is nullptr, it represent of this pointer.  And it is DevPointersMap.  Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129608

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

[PATCH] D129694: [OPENMP] Make declare target static global externally visible

[Sunil Shrestha via Phabricator via cfe-commits]

ssquare08 added a comment.

In D129694#3708297 , @jhuber6 wrote:

> In D129694#3708214 , @ssquare08 
> wrote:
>
>> If that's the preference I can make changes as suggested. You mentioned CUDA 
>> and HIP mangle the declaration directly. To me it looks like they mangle it 
>> on host and device separately. Is that not correct? If so, can you point me 
>> to the source you are referring to?
>
> You're right, they mangle them separately like in 
> https://godbolt.org/z/r6hG4brqx, this is most likely because they already had 
> separate "device side" names. For OpenMP we currently just use the same name 
> for the variable on the host and device side like in 
> https://godbolt.org/z/eaGo9qsW3 where we just use the same kernel names. 
> Thinking again, I'm still wondering if there's any utility in keeping the 
> names separate. Correct me if I'm wrong, but the host-side variable should be 
> able to remain internal so this mangled device name shouldn't show up in the 
> final executable. In that case the only benefit is slightly nicer IR, which 
> I'm not super concerned with.

Yes, the host-side variable should be able to remain internal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129694

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

[PATCH] D129608: [Clang][OpenMP] Fix segmentation fault when data field is used in is_device_pt.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9076
+// DeclComponentLists for generating components info.
+auto It = DevPointersMap.find(VD);
+if (It != DevPointersMap.end())

VD is still nullptr here, why do we need to lookup for it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129608

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

[PATCH] D131708: [RISCV] Change how mtune aliases are implemented.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: jrtc27, asb, luismarques, kito-cheng.
Herald added subscribers: sunshaoce, VincentWu, luke957, jeroen.dobbelaere, 
StephenFan, vkmr, frasercrmck, evandro, apazos, sameer.abuasal, s.egerton, Jim, 
benna, psnobl, jocewei, PkmX, the_o, brucehoult, MartinMosbeck, rogfer01, 
edward-jones, zzheng, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, 
hiraditya, arichardson.
Herald added a project: All.
craig.topper requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, eopXD, MaskRay.
Herald added projects: clang, LLVM.

The previous implementation translated from names like sifive-7-series
to sifive-7-rv32 or sifive-7-rv64. This also required sifive-7-rv32
and sifive-7-rv64 to be valid CPU names. As those are not real
CPUs it doesn't make sense to accept them in -mcpu.

This patch does away with the translation and adds sifive-7-series
directly to RISCV.td. Removing sifive-7-rv32 and sifive-7-rv64.
sifive-7-series is only allowed in -mtune. I've done the same for "rocket".

To prevent -mcpu=sifive-7-series or -mcpu=rocket being used with llc,
I've added a Feature32Bit to all rv32 CPUs. And made it an error to
have an rv32 triple without Feature32Bit. sifive-7-series and rocket
do not have Feature32Bit or Feature64Bit set so the user would need
to provide -mattr=+32bit or -mattr=+64bit along with the -mcpu to
avoid the error.

SiFive no longer names their newer products with 3, 5, or 7 series.
Instead we have p200 series, x200 series, p500 series, and p600 series.
Following the previous behavior would require a sifive-p500-rv32 and
sifive-p500-rv64 in order to support -mtune=sifive-p500-series. There
is currently no p500 product, but it could start getting confusing if
there was in the future.

I'm open to hearing alternatives for how to achieve my main goal
of removing sifive-7-rv32/rv64 as a CPU name.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131708

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/riscv-cpus.c
  clang/test/Misc/target-invalid-cpu-note.c
  llvm/include/llvm/Support/RISCVTargetParser.def
  llvm/include/llvm/Support/TargetParser.h
  llvm/lib/Support/TargetParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/Transforms/LoopUnroll/RISCV/unroll.ll

Index: llvm/test/Transforms/LoopUnroll/RISCV/unroll.ll
===
--- llvm/test/Transforms/LoopUnroll/RISCV/unroll.ll
+++ llvm/test/Transforms/LoopUnroll/RISCV/unroll.ll
@@ -1,5 +1,5 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt %s -S -mtriple=riscv64 -loop-unroll -mcpu=sifive-7-rv64 | FileCheck %s
+; RUN: opt %s -S -mtriple=riscv64 -loop-unroll -mcpu=sifive-s76 | FileCheck %s
 
 define dso_local void @saxpy(float %a, float* %x, float* %y) {
 ; CHECK-LABEL: @saxpy(
Index: llvm/lib/Target/RISCV/RISCVSubtarget.h
===
--- llvm/lib/Target/RISCV/RISCVSubtarget.h
+++ llvm/lib/Target/RISCV/RISCVSubtarget.h
@@ -90,6 +90,7 @@
   bool HasStdExtZicboz = false;
   bool HasStdExtZicbop = false;
   bool HasStdExtZmmul = false;
+  bool HasRV32 = false;
   bool HasRV64 = false;
   bool IsRV32E = false;
   bool EnableLinkerRelax = false;
Index: llvm/lib/Target/RISCV/RISCV.td
===
--- llvm/lib/Target/RISCV/RISCV.td
+++ llvm/lib/Target/RISCV/RISCV.td
@@ -434,6 +434,10 @@
 AssemblerPredicate<(all_of FeatureStdExtZicbop),
 "'Zicbop' (Cache-Block Prefetch Instructions)">;
 
+// Feature32Bit exists to mark CPUs that support RV32 to distinquish them from
+// tuning CPU names.
+def Feature32Bit
+: SubtargetFeature<"32bit", "HasRV32", "true", "Implements RV32">;
 def Feature64Bit
 : SubtargetFeature<"64bit", "HasRV64", "true", "Implements RV64">;
 def IsRV64 : Predicate<"Subtarget->is64Bit()">,
@@ -514,42 +518,47 @@
 // RISC-V processors supported.
 //===--===//
 
-def : ProcessorModel<"generic-rv32", NoSchedModel, []>;
+def : ProcessorModel<"generic-rv32", NoSchedModel, [Feature32Bit]>;
 def : ProcessorModel<"generic-rv64", NoSchedModel, [Feature64Bit]>;
 // Support generic for compatibility with other targets. The triple will be used
 // to change to the appropriate rv32/rv64 version.
 def : ProcessorModel<"generic", NoSchedModel, []>;
 
-def : ProcessorModel<"rocket-rv32", RocketModel, []>;
+def : ProcessorModel<"rocket-rv32", RocketModel, [Feature32Bit]>;
 def : ProcessorModel<"rocket-rv64", RocketModel, [Feature64Bit]>;
+def : ProcessorModel<"rocket", RocketModel, []>;
 
-def : ProcessorModel<"sifive-7-rv32", SiFive7Model, [],
- [TuneSiFive7]>;
-def : 

[PATCH] D129608: [Clang][OpenMP] Fix segmentation fault when data field is used in is_device_pt.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9073
 SmallVector DeclComponentLists;
+if (DevPointersMap.count(VD)) {
+  // For member fields list in is_device_ptr, store it in

ABataev wrote:
> If VD is nullptr, why do we need this check?
Yes.  You are right.  It is unnecessary change.  I removed.  Thanks.  Jennifer 



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:9080
+  DeclComponentLists.emplace_back(MCL, OMPC_MAP_to,
+  OMPC_MAP_MODIFIER_unknown, true,
+  nullptr, nullptr);

ABataev wrote:
> comment with the name of parameter, associated with `true` value.
Sorry.  Added.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129608

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

[PATCH] D129608: [Clang][OpenMP] Fix segmentation fault when data field is used in is_device_pt.

[Jennifer Yu via Phabricator via cfe-commits]

jyu2 updated this revision to Diff 451939.
jyu2 added a comment.

Thanks Alexey!  Address the comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129608

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_is_device_ptr_codegen.cpp
  openmp/libomptarget/test/mapping/is_device_ptr.cpp

Index: openmp/libomptarget/test/mapping/is_device_ptr.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/is_device_ptr.cpp
@@ -0,0 +1,32 @@
+// RUN: %libomptarget-compilexx-run-and-check-generic
+
+#include 
+#include 
+#include 
+
+struct view {
+  const int size = 10;
+  int *data_host;
+  int *data_device;
+  void foo() {
+std::size_t bytes = size * sizeof(int);
+const int host_id = omp_get_initial_device();
+const int device_id = omp_get_default_device();
+data_host = (int *)malloc(bytes);
+data_device = (int *)omp_target_alloc(bytes, device_id);
+#pragma omp target teams distribute parallel for is_device_ptr(data_device)
+for (int i = 0; i < size; ++i)
+  data_device[i] = i;
+omp_target_memcpy(data_host, data_device, bytes, 0, 0, host_id, device_id);
+for (int i = 0; i < size; ++i)
+  assert(data_host[i] == i);
+  }
+};
+
+int main() {
+  view a;
+  a.foo();
+  // CHECK: PASSED
+  printf("PASSED\n");
+}
+
Index: clang/test/OpenMP/target_is_device_ptr_codegen.cpp
===
--- clang/test/OpenMP/target_is_device_ptr_codegen.cpp
+++ clang/test/OpenMP/target_is_device_ptr_codegen.cpp
@@ -252,12 +252,12 @@
 // CK2-DAG: [[BPGEP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
 // CK2-DAG: [[PGEP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
+// CK2-DAG: [[A:%.*]] = getelementptr inbounds [[STRUCT_ST:%.*]], %struct.ST* [[THIS1:%.+]], i32 0, i32 0
 // CK2-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[CBP0:%.+]] = bitcast i8** [[BP0]] to [[ST]]**
-// CK2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to [[ST]]**
-// CK2-DAG: store [[ST]]* [[VAR0:%.+]], [[ST]]** [[CBP0]]
-// CK2-DAG: store [[ST]]* [[VAR0]], [[ST]]** [[CP0]]
+// CK2-DAG: store [[ST]]* [[THIS1]], [[ST]]** [[CBP0]]
+// CK2-DAG: store double** %a, double*** [[TMP3:%.+]]
 #pragma omp target is_device_ptr(a)
 {
   a++;
@@ -271,12 +271,12 @@
 // CK2-DAG: [[BPGEP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
 // CK2-DAG: [[PGEP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
+// CK2-DAG: [[B:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* [[THIS1]], i32 0, i32 1
 // CK2-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[CBP0:%.+]] = bitcast i8** [[BP0]] to [[ST]]**
-// CK2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to [[ST]]**
-// CK2-DAG: store [[ST]]* [[VAR0:%.+]], [[ST]]** [[CBP0]]
-// CK2-DAG: store [[ST]]* [[VAR0]], [[ST]]** [[CP0]]
+// CK2-DAG:  store %struct.ST* [[THIS1]], %struct.ST** [[CBP0]]
+// CK2-DAG: store double*** [[B]], double [[TMP30:%.+]]
 #pragma omp target is_device_ptr(b)
 {
   b++;
@@ -290,12 +290,13 @@
 // CK2-DAG: [[BPGEP]] = getelementptr inbounds {{.+}}[[BP:%[^,]+]]
 // CK2-DAG: [[PGEP]] = getelementptr inbounds {{.+}}[[P:%[^,]+]]
 
+// CK2-DAG: [[A8:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* [[THIS1]], i32 0, i32 0
+// CK2-DAG: [[B9:%.*]] = getelementptr inbounds [[STRUCT_ST]], %struct.ST* [[THIS1]], i32 0, i32 1
 // CK2-DAG: [[BP0:%.+]] = getelementptr inbounds {{.+}}[[BP]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[P0:%.+]] = getelementptr inbounds {{.+}}[[P]], i{{.+}} 0, i{{.+}} 0
 // CK2-DAG: [[CBP0:%.+]] = bitcast i8** [[BP0]] to [[ST]]**
-// CK2-DAG: [[CP0:%.+]] = bitcast i8** [[P0]] to [[ST]]**
-// CK2-DAG: store [[ST]]* [[VAR0:%.+]], [[ST]]** [[CBP0]]
-// CK2-DAG: store [[ST]]* [[VAR0]], [[ST]]** [[CP0]]
+// CK2-DAG:  store %struct.ST* [[THIS1]], %struct.ST** [[CBP0]]
+// CK2-DAG: store double** [[A8]], double*** [[TMP64:%.+]]
 #pragma omp target is_device_ptr(a, b)
 {
   a++;
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -9052,7 +9052,7 @@
 // If this declaration appears in a is_device_ptr clause we just have to
 // pass the pointer by value. If it is a reference to a declaration, we just
 // pass its value.
-if (DevPointersMap.count(VD)) {
+if (VD && DevPointersMap.count(VD)) {
   CombinedInfo.Exprs.push_back(VD);
   CombinedInfo.BasePointers.emplace_back(Arg, VD);
   CombinedInfo.Pointers.push_back(Arg);
@@ -9071,6 +9071,14 @@
OpenMPMapClauseKind, ArrayRef, bool,
   

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Matheus Izvekov via Phabricator via cfe-commits]

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

In D112374#3716944 , @alexfh wrote:

> One more problem related to this patch: it changes the behavior of 
> __PRETTY_FUNCTION__: https://gcc.godbolt.org/z/Mvnj9j74E. There may be 
> dependencies upon the specific format of expansions of this macro: tests, log 
> processing tools, libraries like ctti, and probably other things. It would be 
> better to retain the format of function names expanded from this macro.

Well it looks to me that is a bug fix.

We even match GCC now: https://gcc.godbolt.org/z/WT93WdE7e

Ie we are printing the function type as-written correctly now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D131707: [analyzer]{NFC} Cache the result of getLocationType in TypedValueRegion

[Denys Petrov via Phabricator via cfe-commits]

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

Fix FIXME. Produce `QualType` once in `getLocationType` at the first call. Then 
cache the result and return it for the next calls.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131707

Files:
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -529,6 +529,8 @@
 
 /// TypedValueRegion - An abstract class representing regions having a typed 
value.
 class TypedValueRegion : public TypedRegion {
+  mutable QualType CachedLocationType;
+
   void anchor() override;
 
 protected:
@@ -540,12 +542,13 @@
   virtual QualType getValueType() const = 0;
 
   QualType getLocationType() const override {
-// FIXME: We can possibly optimize this later to cache this value.
-QualType T = getValueType();
-ASTContext  = getContext();
-if (T->getAs())
-  return ctx.getObjCObjectPointerType(T);
-return ctx.getPointerType(getValueType());
+if (CachedLocationType.isNull()) {
+  QualType T = getValueType();
+  ASTContext  = getContext();
+  CachedLocationType = T->isObjCObjectType() ? 
C.getObjCObjectPointerType(T)
+ : C.getPointerType(T);
+}
+return CachedLocationType;
   }
 
   QualType getDesugaredValueType(ASTContext ) const {


Index: clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
===
--- clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
+++ clang/include/clang/StaticAnalyzer/Core/PathSensitive/MemRegion.h
@@ -529,6 +529,8 @@
 
 /// TypedValueRegion - An abstract class representing regions having a typed value.
 class TypedValueRegion : public TypedRegion {
+  mutable QualType CachedLocationType;
+
   void anchor() override;
 
 protected:
@@ -540,12 +542,13 @@
   virtual QualType getValueType() const = 0;
 
   QualType getLocationType() const override {
-// FIXME: We can possibly optimize this later to cache this value.
-QualType T = getValueType();
-ASTContext  = getContext();
-if (T->getAs())
-  return ctx.getObjCObjectPointerType(T);
-return ctx.getPointerType(getValueType());
+if (CachedLocationType.isNull()) {
+  QualType T = getValueType();
+  ASTContext  = getContext();
+  CachedLocationType = T->isObjCObjectType() ? C.getObjCObjectPointerType(T)
+ : C.getPointerType(T);
+}
+return CachedLocationType;
   }
 
   QualType getDesugaredValueType(ASTContext ) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131677: [clang][RISCV] Fix incorrect ABI lowering for inherited structs under hard-float ABIs

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10986
+bool Ret = detectFPCCEligibleStructHelper(
+B.getType(), CurOff, Field1Ty, Field1Off, Field2Ty, Field2Off);
+if (!Ret)

craig.topper wrote:
> jrtc27 wrote:
> > jrtc27 wrote:
> > > With multiple inheritance this offset won't make sense, and it gets 
> > > particularly fun with diamond virtual inheritance. This is a minefield.
> > > 
> > > And what about non-standard layout classes in general, like the simple 
> > > ones that just have a vtable, not just ones that have virtual inheritance 
> > > somwhere?
> > (you at least need `uint64_t BaseOffInBits = Layout.getBaseClassOffset()` 
> > and to add `getContext().toCharUnitsFromBits(BaseOffInBits)` like the field 
> > case, then there's the question of non-standard layout classes)
> If it has a vtable wouldn't it have failed the `getRecordArgABI` check 
> earlier? I think the copy constructor would not be trivial in that case.
Quite possibly, I don't know all the details, just enough to be wary of 
ignoring it entirely


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

https://reviews.llvm.org/D131677

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

[PATCH] D131590: Fixed page title for abseil-no-internal-dependencies check documentation

[Vladimir Plyashkun via Phabricator via cfe-commits]

vladimir.plyashkun added a comment.

In D131590#3716471 , @njames93 wrote:

> In D131590#3716358 , 
> @vladimir.plyashkun wrote:
>
>> Sure it's up to you, it's my github  
>> account , but i don't think it's linked to JetBrains...
>
> Which email address would you like me to use then?

Let's leave "vladimir.plyash...@jetbrains.com"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131590

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

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/lib/Parse/ParseExprCXX.cpp:3783
 
-  if (Arity && Args.size() != Arity) {
-Diag(EndLoc, diag::err_type_trait_arity)

Before this patch, we check the `Arity` here, so using type traits with 
unexpected number of arguments does not crash clang

See https://godbolt.org/z/afGxvqdEh



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5433
 SourceLocation RParenLoc) {
+  if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
+return ExprError();

Moves to here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

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

[PATCH] D131677: [clang][RISCV] Fix incorrect ABI lowering for inherited structs under hard-float ABIs

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10986
+bool Ret = detectFPCCEligibleStructHelper(
+B.getType(), CurOff, Field1Ty, Field1Off, Field2Ty, Field2Off);
+if (!Ret)

jrtc27 wrote:
> jrtc27 wrote:
> > With multiple inheritance this offset won't make sense, and it gets 
> > particularly fun with diamond virtual inheritance. This is a minefield.
> > 
> > And what about non-standard layout classes in general, like the simple ones 
> > that just have a vtable, not just ones that have virtual inheritance 
> > somwhere?
> (you at least need `uint64_t BaseOffInBits = Layout.getBaseClassOffset()` 
> and to add `getContext().toCharUnitsFromBits(BaseOffInBits)` like the field 
> case, then there's the question of non-standard layout classes)
If it has a vtable wouldn't it have failed the `getRecordArgABI` check earlier? 
I think the copy constructor would not be trivial in that case.


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

https://reviews.llvm.org/D131677

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

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 451932.
inclyc added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

Files:
  clang/include/clang/Basic/TypeTraits.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/TypeTraits.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp

Index: clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+// Shouldn't crash here
+// Reported by https://github.com/llvm/llvm-project/issues/57008
+template bool b = __is_constructible(Ts...); // expected-error{{type trait requires 1 or more arguments}}
+bool x = b<>; // expected-note{{in instantiation of variable template specialization}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5430,6 +5430,8 @@
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
+  if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
+return ExprError();
   QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4581,6 +4581,21 @@
   } while (!T.isNull() && T->isVariablyModifiedType());
 }
 
+bool Sema::CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N) {
+  if (Arity && N != Arity) {
+Diag(Loc, diag::err_type_trait_arity)
+<< Arity << 0 << (Arity > 1) << (int)N << SourceRange(Loc);
+return false;
+  }
+
+  if (!Arity && N == 0) {
+Diag(Loc, diag::err_type_trait_arity)
+<< 1 << 1 << 1 << (int)N << SourceRange(Loc);
+return false;
+  }
+  return true;
+}
+
 /// Build a sizeof or alignof expression given a type operand.
 ExprResult
 Sema::CreateUnaryExprOrTypeTraitExpr(TypeSourceInfo *TInfo,
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3724,14 +3724,6 @@
   }
 }
 
-static unsigned TypeTraitArity(tok::TokenKind kind) {
-  switch (kind) {
-default: llvm_unreachable("Not a known type trait");
-#define TYPE_TRAIT(N,Spelling,K) case tok::kw_##Spelling: return N;
-#include "clang/Basic/TokenKinds.def"
-  }
-}
-
 /// Parse the built-in type-trait pseudo-functions that allow
 /// implementation of the TR1/C++11 type traits templates.
 ///
@@ -3745,7 +3737,6 @@
 ///
 ExprResult Parser::ParseTypeTrait() {
   tok::TokenKind Kind = Tok.getKind();
-  unsigned Arity = TypeTraitArity(Kind);
 
   SourceLocation Loc = ConsumeToken();
 
@@ -3780,18 +3771,6 @@
 
   SourceLocation EndLoc = Parens.getCloseLocation();
 
-  if (Arity && Args.size() != Arity) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << Arity << 0 << (Arity > 1) << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
-  if (!Arity && Args.empty()) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << 1 << 1 << 1 << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
   return Actions.ActOnTypeTrait(TypeTraitFromTokKind(Kind), Loc, Args, EndLoc);
 }
 
Index: clang/lib/Basic/TypeTraits.cpp
===
--- clang/lib/Basic/TypeTraits.cpp
+++ clang/lib/Basic/TypeTraits.cpp
@@ -55,6 +55,15 @@
 #include "clang/Basic/TokenKinds.def"
 };
 
+static constexpr const unsigned TypeTraitArities[] = {
+#define TYPE_TRAIT_1(Spelling, Name, Key) 1,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_2(Spelling, Name, Key) 2,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};
+
 const char *clang::getTraitName(TypeTrait T) {
   assert(T <= TT_Last && "invalid enum value!");
   return TypeTraitNames[T];
@@ -84,3 +93,8 @@
   assert(T <= UETT_Last && "invalid enum value!");
   return UnaryExprOrTypeTraitSpellings[T];
 }
+
+unsigned clang::getTypeTraitArity(TypeTrait T) {
+  assert(T <= TT_Last && "invalid enum value!");
+  return TypeTraitArities[T];
+}
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5552,6 +5552,8 @@
   bool isQualifiedMemberAccess(Expr *E);
   QualType 

[clang] adcd4b1 - [analyzer] [NFC] Fix comments into more regular form.

[Denys Petrov via cfe-commits]

Author: Denys Petrov
Date: 2022-08-11T21:28:23+03:00
New Revision: adcd4b1c0bd43c89e579bf07daff7ffb02848012

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

LOG: [analyzer] [NFC] Fix comments into more regular form.

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/MemRegion.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp 
b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
index 81c11099e93f0..bb64cbc4b71c4 100644
--- a/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ b/clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -1286,8 +1286,8 @@ bool MemRegion::hasGlobalsOrParametersStorage() const {
   return isa(getMemorySpace());
 }
 
-// getBaseRegion strips away all elements and fields, and get the base region
-// of them.
+// Strips away all elements and fields.
+// Returns the base region of them.
 const MemRegion *MemRegion::getBaseRegion() const {
   const MemRegion *R = this;
   while (true) {
@@ -1307,8 +1307,7 @@ const MemRegion *MemRegion::getBaseRegion() const {
   return R;
 }
 
-// getgetMostDerivedObjectRegion gets the region of the root class of a C++
-// class hierarchy.
+// Returns the region of the root class of a C++ class hierarchy.
 const MemRegion *MemRegion::getMostDerivedObjectRegion() const {
   const MemRegion *R = this;
   while (const auto *BR = dyn_cast(R))



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

[PATCH] D112374: [clang] Implement ElaboratedType sugaring for types written bare

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

One more problem related to this patch: it changes the behavior of 
__PRETTY_FUNCTION__: https://gcc.godbolt.org/z/Mvnj9j74E. There may be 
dependencies upon the specific format of expansions of this macro: tests, log 
processing tools, libraries like ctti, and probably other things. It would be 
better to retain the format of function names expanded from this macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 451931.
inclyc added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

Files:
  clang/include/clang/Basic/TypeTraits.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/TypeTraits.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp

Index: clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/type-trait-eval-crash-issue-57008.cpp
@@ -0,0 +1,6 @@
+// RUN: %clang_cc1 -fsyntax-only %s -verify
+
+// Shouldn't crash here
+// Reported by https://github.com/llvm/llvm-project/issues/57008
+template bool b = __is_constructible(Ts...); // expected-error{{type trait requires 1 or more arguments}}
+bool x = b<>; // expected-note{{in instantiation of variable template specialization}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -5430,6 +5430,8 @@
 ExprResult Sema::BuildTypeTrait(TypeTrait Kind, SourceLocation KWLoc,
 ArrayRef Args,
 SourceLocation RParenLoc) {
+  if (!CheckTypeTraitArity(getTypeTraitArity(Kind), KWLoc, Args.size()))
+return ExprError();
   QualType ResultType = Context.getLogicalOperationType();
 
   if (Kind <= UTT_Last && !CheckUnaryTypeTraitTypeCompleteness(
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -4581,6 +4581,21 @@
   } while (!T.isNull() && T->isVariablyModifiedType());
 }
 
+bool Sema::CheckTypeTraitArity(unsigned Arity, SourceLocation Loc, size_t N) {
+  if (Arity && N != Arity) {
+Diag(Loc, diag::err_type_trait_arity)
+<< Arity << 0 << (Arity > 1) << (int)N << SourceRange(Loc);
+return false;
+  }
+
+  if (!Arity && N == 0) {
+Diag(Loc, diag::err_type_trait_arity)
+<< 1 << 1 << 1 << (int)N << SourceRange(Loc);
+return false;
+  }
+  return true;
+}
+
 /// Build a sizeof or alignof expression given a type operand.
 ExprResult
 Sema::CreateUnaryExprOrTypeTraitExpr(TypeSourceInfo *TInfo,
Index: clang/lib/Parse/ParseExprCXX.cpp
===
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -3724,14 +3724,6 @@
   }
 }
 
-static unsigned TypeTraitArity(tok::TokenKind kind) {
-  switch (kind) {
-default: llvm_unreachable("Not a known type trait");
-#define TYPE_TRAIT(N,Spelling,K) case tok::kw_##Spelling: return N;
-#include "clang/Basic/TokenKinds.def"
-  }
-}
-
 /// Parse the built-in type-trait pseudo-functions that allow
 /// implementation of the TR1/C++11 type traits templates.
 ///
@@ -3745,7 +3737,6 @@
 ///
 ExprResult Parser::ParseTypeTrait() {
   tok::TokenKind Kind = Tok.getKind();
-  unsigned Arity = TypeTraitArity(Kind);
 
   SourceLocation Loc = ConsumeToken();
 
@@ -3780,18 +3771,6 @@
 
   SourceLocation EndLoc = Parens.getCloseLocation();
 
-  if (Arity && Args.size() != Arity) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << Arity << 0 << (Arity > 1) << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
-  if (!Arity && Args.empty()) {
-Diag(EndLoc, diag::err_type_trait_arity)
-  << 1 << 1 << 1 << (int)Args.size() << SourceRange(Loc);
-return ExprError();
-  }
-
   return Actions.ActOnTypeTrait(TypeTraitFromTokKind(Kind), Loc, Args, EndLoc);
 }
 
Index: clang/lib/Basic/TypeTraits.cpp
===
--- clang/lib/Basic/TypeTraits.cpp
+++ clang/lib/Basic/TypeTraits.cpp
@@ -55,6 +55,15 @@
 #include "clang/Basic/TokenKinds.def"
 };
 
+static constexpr const unsigned TypeTraitArities[] = {
+#define TYPE_TRAIT_1(Spelling, Name, Key) 1,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_2(Spelling, Name, Key) 2,
+#include "clang/Basic/TokenKinds.def"
+#define TYPE_TRAIT_N(Spelling, Name, Key) 0,
+#include "clang/Basic/TokenKinds.def"
+};
+
 const char *clang::getTraitName(TypeTrait T) {
   assert(T <= TT_Last && "invalid enum value!");
   return TypeTraitNames[T];
@@ -84,3 +93,8 @@
   assert(T <= UETT_Last && "invalid enum value!");
   return UnaryExprOrTypeTraitSpellings[T];
 }
+
+unsigned clang::getTypeTraitArity(TypeTrait T) {
+  assert(T <= TT_Last && "invalid enum value!");
+  return TypeTraitArities[T];
+}
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5552,6 +5552,8 @@
   bool isQualifiedMemberAccess(Expr *E);
   QualType 

[PATCH] D131706: [clangd][unittests][IncludeCleaner] Don't call findReferencedFiles() if the result is not used

[Aleksandr Platonov via Phabricator via cfe-commits]

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

IncludeCleaner.RecursiveInclusion and IncludeCleaner.IWYUPragmaExport tests 
don't check referenced files list, so we don't need to call 
findReferencedFiles() there.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131706

Files:
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -571,9 +571,6 @@
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
@@ -596,9 +593,6 @@
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   // FIXME: This is not correct: foo.h is unused but is not diagnosed as such
   // because we ignore headers with IWYU export pragmas for now.


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -571,9 +571,6 @@
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
 }
@@ -596,9 +593,6 @@
   )cpp");
   ParsedAST AST = TU.build();
 
-  auto ReferencedFiles = findReferencedFiles(
-  findReferencedLocations(AST), AST.getIncludeStructure(),
-  AST.getCanonicalIncludes(), AST.getSourceManager());
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
   // FIXME: This is not correct: foo.h is unused but is not diagnosed as such
   // because we ignore headers with IWYU export pragmas for now.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131677: [clang][RISCV] Fix incorrect ABI lowering for inherited structs under hard-float ABIs

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:10986
+bool Ret = detectFPCCEligibleStructHelper(
+B.getType(), CurOff, Field1Ty, Field1Off, Field2Ty, Field2Off);
+if (!Ret)

jrtc27 wrote:
> With multiple inheritance this offset won't make sense, and it gets 
> particularly fun with diamond virtual inheritance. This is a minefield.
> 
> And what about non-standard layout classes in general, like the simple ones 
> that just have a vtable, not just ones that have virtual inheritance somwhere?
(you at least need `uint64_t BaseOffInBits = Layout.getBaseClassOffset()` and 
to add `getContext().toCharUnitsFromBits(BaseOffInBits)` like the field case, 
then there's the question of non-standard layout classes)


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

https://reviews.llvm.org/D131677

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