[PATCH] D141666: [RISCV] Proper support of extensions Zicsr and Zifencei

[Elena Lepilkina via Phabricator via cfe-commits]

eklepilkina updated this revision to Diff 490056.
eklepilkina added a comment.

- Updated I extension verson


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141666

Files:
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/BareMetal.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/test/Driver/baremetal.cpp
  clang/test/Driver/riscv-default-features.c
  clang/test/Driver/riscv-gnutools.c
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVISelLowering.cpp
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/CodeGen/RISCV/get-register-noreserve.ll
  llvm/test/CodeGen/RISCV/readcyclecounter.ll
  llvm/test/CodeGen/RISCV/rvv/rvv-stack-align.mir
  llvm/test/CodeGen/RISCV/rvv/scalar-stack-align.ll
  llvm/test/CodeGen/RISCV/rvv/vscale-power-of-two.ll
  llvm/test/CodeGen/RISCV/rvv/zvlsseg-zero-vl.ll
  llvm/test/CodeGen/RISCV/vlenb.ll
  llvm/test/MC/RISCV/Ztso.s
  llvm/test/MC/RISCV/csr-aliases.s
  llvm/test/MC/RISCV/deprecated-csr-names.s
  llvm/test/MC/RISCV/hypervisor-csr-names.s
  llvm/test/MC/RISCV/machine-csr-names.s
  llvm/test/MC/RISCV/rv32-hypervisor-csr-names.s
  llvm/test/MC/RISCV/rv32-machine-csr-names.s
  llvm/test/MC/RISCV/rv32-supervisor-csr-names.s
  llvm/test/MC/RISCV/rv32-user-csr-names.s
  llvm/test/MC/RISCV/rv32e-valid.s
  llvm/test/MC/RISCV/rv32i-aliases-valid.s
  llvm/test/MC/RISCV/rv32i-invalid.s
  llvm/test/MC/RISCV/rv32i-valid.s
  llvm/test/MC/RISCV/rv64-machine-csr-names.s
  llvm/test/MC/RISCV/rv64-user-csr-names.s
  llvm/test/MC/RISCV/rvf-user-csr-names.s
  llvm/test/MC/RISCV/rvi-aliases-valid.s
  llvm/test/MC/RISCV/rvk-user-csr-name.s
  llvm/test/MC/RISCV/supervisor-csr-names.s
  llvm/test/MC/RISCV/user-csr-names.s

Index: llvm/test/MC/RISCV/user-csr-names.s
===
--- llvm/test/MC/RISCV/user-csr-names.s
+++ llvm/test/MC/RISCV/user-csr-names.s
@@ -1,13 +1,13 @@
-# RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -show-encoding \
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zicsr -riscv-no-aliases -show-encoding \
 # RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
-# RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
-# RUN: | llvm-objdump -d - \
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zicsr < %s \
+# RUN: | llvm-objdump -d --mattr=+zicsr - \
 # RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
 #
-# RUN: llvm-mc %s -triple=riscv64 -riscv-no-aliases -show-encoding \
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+zicsr -riscv-no-aliases -show-encoding \
 # RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
-# RUN: llvm-mc -filetype=obj -triple riscv64 < %s \
-# RUN: | llvm-objdump -d - \
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+zicsr < %s \
+# RUN: | llvm-objdump -d --mattr=+zicsr - \
 # RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
 
 ##
Index: llvm/test/MC/RISCV/supervisor-csr-names.s
===
--- llvm/test/MC/RISCV/supervisor-csr-names.s
+++ llvm/test/MC/RISCV/supervisor-csr-names.s
@@ -1,13 +1,13 @@
-# RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -show-encoding \
+# RUN: llvm-mc %s -triple=riscv32 -mattr=+zicsr -riscv-no-aliases -show-encoding \
 # RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
-# RUN: llvm-mc -filetype=obj -triple riscv32 < %s \
-# RUN: | llvm-objdump -d - \
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zicsr < %s \
+# RUN: | llvm-objdump -d --mattr=+zicsr - \
 # RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
 #
-# RUN: llvm-mc %s -triple=riscv64 -riscv-no-aliases -show-encoding \
+# RUN: llvm-mc %s -triple=riscv64 -mattr=+zicsr -riscv-no-aliases -show-encoding \
 # RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
-# RUN: llvm-mc -filetype=obj -triple riscv64 < %s \
-# RUN: | llvm-objdump -d - \
+# RUN: llvm-mc -filetype=obj -triple riscv64 -mattr=+zicsr < %s \
+# RUN: | llvm-objdump -d --mattr=+zicsr - \
 # RUN: | FileCheck -check-prefix=CHECK-INST-ALIAS %s
 
 ##
Index: llvm/test/MC/RISCV/rvk-user-csr-name.s
===
--- llvm/test/MC/RISCV/rvk-user-csr-name.s
+++ llvm/test/MC/RISCV/rvk-user-csr-name.s
@@ -1,13 +1,13 @@
-# RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -mattr=+f -show-encoding \
+# RUN: llvm-mc %s -triple=riscv32 -riscv-no-aliases -mattr=+zicsr,+f -show-encoding \
 # RUN: | FileCheck -check-prefixes=CHECK-INST,CHECK-ENC %s
-# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zkr < %s \
-# RUN: | llvm-objdump -d --mattr=+zkr - \
+# RUN: llvm-mc -filetype=obj -triple riscv32 -mattr=+zkr,+zicsr < %s \
+# RUN: | llvm-objdump -d --mattr=+zkr,+zicsr - \
 # RUN: | File

[PATCH] D140875: [clangd] prototype: Implement unused include warnings with include-cleaner library.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

regarding testing, i think updating all tests that we have in 
`IncludeCleanerTests` that call `computeUnusedIncludes` to also call the new 
function that'll use include-cleaner library should be enough.




Comment at: clang-tools-extra/clangd/CMakeLists.txt:5
 
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../include-cleaner/include)
+

can you move this near the include_directories for clang-tidy below (near line 
63)



Comment at: clang-tools-extra/clangd/Config.h:91
 
-  enum UnusedIncludesPolicy { Strict, None };
+  enum UnusedIncludesPolicy { Strict, None, Experiment };
   /// Controls warnings and errors when parsing code.

can you also add a comment saying that "`Experiment` tries to provide the same 
behaviour as `Strict` but using include-cleaner"



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:471
+
+// FIXME: this map should probably be in IncludeStructure.
+llvm::StringMap> BySpelling;

agreed, let's change `StdlibHeaders` in `IncludeStructure` to actually be a 
`BySpelling` mapping, it should be no-op for existing implementation as we're 
only checking for stdlib headers already (and there's no other usage)



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:479
+}
+// FIXME: !!this is a hacky way to collect macro references.
+std::vector Macros;

this might behave slightly different than what we have in `RecordedPP`, and 
rest of the applications of include-cleaner will be using that. can we expose 
the pieces in RecordedPP that collects MacroRefs as a separate handler and 
attach that collector (combined with the skipped range collection inside 
`CollectMainFileMacros` and also still converting to `MainFileMacros` in the 
end (as we can't store sourcelocations/identifierinfos from preamble)?

afterwards we can use the names stored in there to get back to 
`IdentifierInfo`s and Ranges stored to get back to `SourceLocation`s. WDYT?



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:487
+Macros.push_back({Tok.location(),
+  include_cleaner::Macro{/*Name=*/nullptr, DefLoc},
+  include_cleaner::RefType::Explicit});

you can get the `IdentifierInfo` using `Name` inside `Macro` and PP.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:512
+});
+std::vector Unused;
+for (const auto &I : Includes.MainFileIncludes) {

let's use `getUnused` directly here, with an empty set of `PublicHeaders`.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:539
   const Config &Cfg = Config::current();
-  if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
+  if (Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::None ||
   Cfg.Diagnostics.SuppressAll ||

can you keep `computeUnusedIncludes` the same and introduce a new function 
that'll call `include_cleaner`? afterwards we can just do a switch on policy 
here and call the relevant function.



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:609
+  Config::UnusedIncludesPolicy::Experiment)
+Pragmas.record(*Clang);
   // Copy over the macros in the preamble region of the main file, and combine

why do we need to collect pragmas in main file? i think we already have 
necessary information available via `IncludeStructure` (it stores keeps and 
exports, and we don't care about anything else in the main file AFAICT). so 
let's just use the PragmaIncludes we're getting from the Preamble directly? 
without even making a copy and returning a reference from the `Preamble` 
instead in `ParsedAST::getPragmaIncludes`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140875

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

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 490062.
iains added a comment.

rebased, address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141908

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CXX/module/module.import/p6.cpp


Index: clang/test/CXX/module/module.import/p6.cpp
===
--- clang/test/CXX/module/module.import/p6.cpp
+++ clang/test/CXX/module/module.import/p6.cpp
@@ -28,3 +28,11 @@
 static const int value = 43; 
 };
 
+void deleted_fn_ok (void) = delete;
+
+struct S {
+   ~S() noexcept(false) = default;
+private:
+  S(S&);
+};
+S::S(S&) = default;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -15254,9 +15254,12 @@
   }
 
   // C++ [module.import/6] external definitions are not permitted in header
-  // units.
+  // units.  Deleted and Defaulted functions are implicitly inline (but the
+  // inline state is not set at this point, so check the BodyKind explicitly).
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
-  FD->getFormalLinkage() == Linkage::ExternalLinkage && !FD->isInlined()) {
+  FD->getFormalLinkage() == Linkage::ExternalLinkage &&
+  !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
+  BodyKind != FnBodyKind::Default && !FD->isInlined()) {
 Diag(FD->getLocation(), diag::err_extern_def_in_header_unit);
 FD->setInvalidDecl();
   }


Index: clang/test/CXX/module/module.import/p6.cpp
===
--- clang/test/CXX/module/module.import/p6.cpp
+++ clang/test/CXX/module/module.import/p6.cpp
@@ -28,3 +28,11 @@
 static const int value = 43; 
 };
 
+void deleted_fn_ok (void) = delete;
+
+struct S {
+   ~S() noexcept(false) = default;
+private:
+  S(S&);
+};
+S::S(S&) = default;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -15254,9 +15254,12 @@
   }
 
   // C++ [module.import/6] external definitions are not permitted in header
-  // units.
+  // units.  Deleted and Defaulted functions are implicitly inline (but the
+  // inline state is not set at this point, so check the BodyKind explicitly).
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
-  FD->getFormalLinkage() == Linkage::ExternalLinkage && !FD->isInlined()) {
+  FD->getFormalLinkage() == Linkage::ExternalLinkage &&
+  !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
+  BodyKind != FnBodyKind::Default && !FD->isInlined()) {
 Diag(FD->getLocation(), diag::err_extern_def_in_header_unit);
 FD->setInvalidDecl();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:15258
+  // units.  Deleted and Defaulted functions are implicitly inline (but the
+  // inline state is not set at this point, so check the BodyKind explicitly).
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&

ChuanqiXu wrote:
> I prefer to add a FIXME here to say that we need to find a better place for 
> the check to eliminate the unnecessary check for `BodyKind `. The current 
> check for `BodyKind` looks a little bit hacky to me.
When the patch was originally done, this was found to be a good place to do the 
check (i.e. less duplication of testing and to avoid duplication of 
diagnostics) so I do not think I agree that there is a FIXME to move it.

BodyKind is already used elsewhere in this function for similar purposes - it 
does not look hacky to me.



Comment at: clang/lib/Sema/SemaDecl.cpp:15261
+  FD->getFormalLinkage() == Linkage::ExternalLinkage &&
+  !FD->isInvalidDecl() && BodyKind == FnBodyKind::Other &&
+  !FD->isInlined()) {

ChuanqiXu wrote:
> It looks like we need to check `FD->isThisDeclarationADefinition()` too.
> 
> And personally, I prefer to check BodyKind explicitly. Otherwise the readers 
> need to checkout the definition of `FnBodyKind` to understand the code. 
> It looks like we need to check `FD->isThisDeclarationADefinition()` too.

This is an unnecessary test, it will always return true at this point.

> And personally, I prefer to check BodyKind explicitly. Otherwise the readers 
> need to checkout the definition of `FnBodyKind` to understand the code. 

You prefer two tests  instead of one?
OK, I guess


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141908

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

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

[Chuanqi Xu via Phabricator via cfe-commits]

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

LGTM basically. I still feel we need a FIXME there. But I don't want to block 
this for this reason especially we need to land this before the branch.




Comment at: clang/lib/Sema/SemaDecl.cpp:15258
+  // units.  Deleted and Defaulted functions are implicitly inline (but the
+  // inline state is not set at this point, so check the BodyKind explicitly).
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&

iains wrote:
> ChuanqiXu wrote:
> > I prefer to add a FIXME here to say that we need to find a better place for 
> > the check to eliminate the unnecessary check for `BodyKind `. The current 
> > check for `BodyKind` looks a little bit hacky to me.
> When the patch was originally done, this was found to be a good place to do 
> the check (i.e. less duplication of testing and to avoid duplication of 
> diagnostics) so I do not think I agree that there is a FIXME to move it.
> 
> BodyKind is already used elsewhere in this function for similar purposes - it 
> does not look hacky to me.
It looks hacky to me since we shouldn't care if it is deleted or defaulted here 
and it should be enough to check `FD->isInlied()`.  And I don't see similar 
usage of `BodyKind ` in this function.



Comment at: clang/lib/Sema/SemaDecl.cpp:15261
+  FD->getFormalLinkage() == Linkage::ExternalLinkage &&
+  !FD->isInvalidDecl() && BodyKind == FnBodyKind::Other &&
+  !FD->isInlined()) {

iains wrote:
> ChuanqiXu wrote:
> > It looks like we need to check `FD->isThisDeclarationADefinition()` too.
> > 
> > And personally, I prefer to check BodyKind explicitly. Otherwise the 
> > readers need to checkout the definition of `FnBodyKind` to understand the 
> > code. 
> > It looks like we need to check `FD->isThisDeclarationADefinition()` too.
> 
> This is an unnecessary test, it will always return true at this point.
> 
> > And personally, I prefer to check BodyKind explicitly. Otherwise the 
> > readers need to checkout the definition of `FnBodyKind` to understand the 
> > code. 
> 
> You prefer two tests  instead of one?
> OK, I guess
> This is an unnecessary test, it will always return true at this point.

Oh, I found it now. It may be better to have an assertion 
`assert(FD->isThisDeclarationADefinition())`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141908

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

[PATCH] D141577: [4/15][Clang][RISCV][NFC] Remove unnecessary logic under RVVIntrinsic::computeBuiltinTypes

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

eopXD added inline comments.



Comment at: clang/lib/Support/RISCVVIntrinsicUtils.cpp:921
 PolicyAttrs.TailPolicy = Policy::PolicyType::Agnostic; // TAMA
-  if (PolicyAttrs.isUnspecified()) {
-if (!IsMasked) {

craig.topper wrote:
> I guess removing this negates a comment I left on the rename to Unspecified?
Yes you are correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141577

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

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

[Iain Sandoe via Phabricator via cfe-commits]

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

In D141908#4061409 , @ChuanqiXu wrote:

> LGTM basically. I still feel we need a FIXME there. But I don't want to block 
> this for this reason especially we need to land this before the branch.

Well.. we have time for another iteration, I will add the assert...




Comment at: clang/lib/Sema/SemaDecl.cpp:15258
+  // units.  Deleted and Defaulted functions are implicitly inline (but the
+  // inline state is not set at this point, so check the BodyKind explicitly).
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > I prefer to add a FIXME here to say that we need to find a better place 
> > > for the check to eliminate the unnecessary check for `BodyKind `. The 
> > > current check for `BodyKind` looks a little bit hacky to me.
> > When the patch was originally done, this was found to be a good place to do 
> > the check (i.e. less duplication of testing and to avoid duplication of 
> > diagnostics) so I do not think I agree that there is a FIXME to move it.
> > 
> > BodyKind is already used elsewhere in this function for similar purposes - 
> > it does not look hacky to me.
> It looks hacky to me since we shouldn't care if it is deleted or defaulted 
> here and it should be enough to check `FD->isInlied()`.  And I don't see 
> similar usage of `BodyKind ` in this function.
> It looks hacky to me since we shouldn't care if it is deleted or defaulted 
> here and it should be enough to check `FD->isInlied()`. 

that means checking much later and in muliple places, I think - but if you want 
to make a follow-on patch, I will be happy to review.

> And I don't see similar usage of `BodyKind ` in this function.

line 15208?





Comment at: clang/lib/Sema/SemaDecl.cpp:15261
+  FD->getFormalLinkage() == Linkage::ExternalLinkage &&
+  !FD->isInvalidDecl() && BodyKind == FnBodyKind::Other &&
+  !FD->isInlined()) {

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > It looks like we need to check `FD->isThisDeclarationADefinition()` too.
> > > 
> > > And personally, I prefer to check BodyKind explicitly. Otherwise the 
> > > readers need to checkout the definition of `FnBodyKind` to understand the 
> > > code. 
> > > It looks like we need to check `FD->isThisDeclarationADefinition()` too.
> > 
> > This is an unnecessary test, it will always return true at this point.
> > 
> > > And personally, I prefer to check BodyKind explicitly. Otherwise the 
> > > readers need to checkout the definition of `FnBodyKind` to understand the 
> > > code. 
> > 
> > You prefer two tests  instead of one?
> > OK, I guess
> > This is an unnecessary test, it will always return true at this point.
> 
> Oh, I found it now. It may be better to have an assertion 
> `assert(FD->isThisDeclarationADefinition())`.
yeah, I was thinking maybe to do that (it is kind of documenting that it is 
always true - perhaps a comment would be better?)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141908

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

[PATCH] D133574: [C2x] reject type definitions in offsetof

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a subscriber: aeubanks.
mstorsjo added a comment.

For clarity in the review - the relanded commit ended up reverted by @aeubanks 
in 39da55e8f548a11f7dadefa73ea73d809a5f1729 
. The 
relanded commit triggers failed asserts like this:

  $ echo 'void c() { __builtin_offsetof(struct {int b;}, b); }' | bin/clang 
-cc1 -emit-llvm -o /dev/null - -x c
  clang: ../../clang/lib/AST/RecordLayoutBuilder.cpp:3286: const 
clang::ASTRecordLayout& clang::ASTContext::getASTRecordLayout(const 
clang::RecordDecl*) const: Assertion `!D->isInvalidDecl() && "Cannot get layout 
of invalid decl!"' failed.

I also ran into a second issue with the reapplied version of the commit:

  $ echo 'int i = __builtin_offsetof(struct {int b;}, b);' | bin/clang -cc1 
-emit-llvm -o /dev/null - -x c
  :1:9: error: initializer element is not a compile-time constant
  int i = __builtin_offsetof(struct {int b;}, b);
  ^~
  1 error generated.

(This used to compile successfully before.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

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

[PATCH] D139704: [clang][RISCV] Added target attributes to runtime functions

[Elena Lepilkina via Phabricator via cfe-commits]

eklepilkina added a comment.
Herald added a subscriber: luke.

Gently ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139704

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

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

> Well.. we have time for another iteration,

I am going to take a vacation for the Chinese New Year since tomorrow to 
February. So I am a little bit hurried : )




Comment at: clang/lib/Sema/SemaDecl.cpp:15258
+  // units.  Deleted and Defaulted functions are implicitly inline (but the
+  // inline state is not set at this point, so check the BodyKind explicitly).
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&

iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > ChuanqiXu wrote:
> > > > I prefer to add a FIXME here to say that we need to find a better place 
> > > > for the check to eliminate the unnecessary check for `BodyKind `. The 
> > > > current check for `BodyKind` looks a little bit hacky to me.
> > > When the patch was originally done, this was found to be a good place to 
> > > do the check (i.e. less duplication of testing and to avoid duplication 
> > > of diagnostics) so I do not think I agree that there is a FIXME to move 
> > > it.
> > > 
> > > BodyKind is already used elsewhere in this function for similar purposes 
> > > - it does not look hacky to me.
> > It looks hacky to me since we shouldn't care if it is deleted or defaulted 
> > here and it should be enough to check `FD->isInlied()`.  And I don't see 
> > similar usage of `BodyKind ` in this function.
> > It looks hacky to me since we shouldn't care if it is deleted or defaulted 
> > here and it should be enough to check `FD->isInlied()`. 
> 
> that means checking much later and in muliple places, I think - but if you 
> want to make a follow-on patch, I will be happy to review.
> 
> > And I don't see similar usage of `BodyKind ` in this function.
> 
> line 15208?
> 
> 
> line 15208?

line 15208 checks the wording `unless the function is deleted (C++ specifc, C++ 
[dcl.fct.def.general]p2)`. So it is used to check if this is a delete function, 
which looks fine. I mean it would be best if we can check `FD->isInlined()` 
only.

> that means checking much later and in muliple places, I think - but if you 
> want to make a follow-on patch, I will be happy to review.

I think I won't work on this. But it would be meaningful for future developers. 
And it looks not impossible to refactor it. (I'm not asking you to do the 
change)



Comment at: clang/lib/Sema/SemaDecl.cpp:15261
+  FD->getFormalLinkage() == Linkage::ExternalLinkage &&
+  !FD->isInvalidDecl() && BodyKind == FnBodyKind::Other &&
+  !FD->isInlined()) {

iains wrote:
> ChuanqiXu wrote:
> > iains wrote:
> > > ChuanqiXu wrote:
> > > > It looks like we need to check `FD->isThisDeclarationADefinition()` too.
> > > > 
> > > > And personally, I prefer to check BodyKind explicitly. Otherwise the 
> > > > readers need to checkout the definition of `FnBodyKind` to understand 
> > > > the code. 
> > > > It looks like we need to check `FD->isThisDeclarationADefinition()` too.
> > > 
> > > This is an unnecessary test, it will always return true at this point.
> > > 
> > > > And personally, I prefer to check BodyKind explicitly. Otherwise the 
> > > > readers need to checkout the definition of `FnBodyKind` to understand 
> > > > the code. 
> > > 
> > > You prefer two tests  instead of one?
> > > OK, I guess
> > > This is an unnecessary test, it will always return true at this point.
> > 
> > Oh, I found it now. It may be better to have an assertion 
> > `assert(FD->isThisDeclarationADefinition())`.
> yeah, I was thinking maybe to do that (it is kind of documenting that it is 
> always true - perhaps a comment would be better?)
> 
I always prefer assertion than comments since  some people may not like to read 
the comments. And the assertion may be useful. For example, if someday we want 
to move the codes to another place but the FD is not always a definition. Then 
the assertion can save us a lot of time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141908

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

[PATCH] D119708: [clang][lex] Remove `PPCallbacks::FileNotFound()`

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D119708#4060336 , @jansvoboda11 
wrote:

> In D119708#4059254 , @Hahnfeld 
> wrote:
>
>> Hello, sorry for the late heads-up, but this functionality is used by ROOT: 
>> https://github.com/root-project/root/blob/f58cccf5ce7fd67894c7fd9e9e74d3f37bc1acba/core/metacling/src/TClingCallbacks.cxx#L282
>>  Any chance of bringing this back?
>
> Hi. Your downstream callback never fills in `RecoveryPath` and always returns 
> `false`, meaning it will never reach the unsafe block of code I had issue 
> with: `HeaderInfo.AddSearchPath(DL, isAngled)`.
> Technically, we could bring back a safe/trimmed-down version of this callback 
> that doesn't take the out-parameter and returns `void`. But I don't think 
> it's the right call, since that would essentially re-introduce dead code to 
> upstream.
> This is probably best kept downstream, unless there are other downstream 
> users.

+1 for bringing back that piece of code because it seems an interesting event 
to listen to anyways. Would you be more comfortable if we provide also a test 
case and relevant comments explaining the use-case?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119708

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

[PATCH] D141855: [include-mapping] Parse zombie_names.html into a removed symbols map.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

mostly good, just nits.




Comment at: clang/tools/include-mapping/gen_std.py:31
// Generate C++ symbols
python3 gen_std.py -cppreference cppreference/reference -language=cpp > 
StdSymbolMap.inc
// Generate C symbols

nit: update the doc.



Comment at: clang/tools/include-mapping/gen_std.py:94
 parse_pages = [(page_root, "index.html", None)]
-
+  elif args.symbols == 'cppremoved':
+page_root = os.path.join(args.cppreference, "en", "cpp")

nit: can we add a `_` between `cpp` and `removed`. And maybe move it after the 
if `args.symbols == 'cpp'` branch as they are `cpp` related.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141855

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

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 490079.
iains marked 5 inline comments as done.
iains added a comment.

address review commments, add an assert and a FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141908

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CXX/module/module.import/p6.cpp


Index: clang/test/CXX/module/module.import/p6.cpp
===
--- clang/test/CXX/module/module.import/p6.cpp
+++ clang/test/CXX/module/module.import/p6.cpp
@@ -28,3 +28,11 @@
 static const int value = 43; 
 };
 
+void deleted_fn_ok (void) = delete;
+
+struct S {
+   ~S() noexcept(false) = default;
+private:
+  S(S&);
+};
+S::S(S&) = default;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -15254,9 +15254,15 @@
   }
 
   // C++ [module.import/6] external definitions are not permitted in header
-  // units.
+  // units.  Deleted and Defaulted functions are implicitly inline (but the
+  // inline state is not set at this point, so check the BodyKind explicitly).
+  // FIXME: Consider an alternate location for the test where the inlined()
+  // state is complete.
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
-  FD->getFormalLinkage() == Linkage::ExternalLinkage && !FD->isInlined()) {
+  FD->getFormalLinkage() == Linkage::ExternalLinkage &&
+  !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
+  BodyKind != FnBodyKind::Default && !FD->isInlined()) {
+assert(FD->isThisDeclarationADefinition());
 Diag(FD->getLocation(), diag::err_extern_def_in_header_unit);
 FD->setInvalidDecl();
   }


Index: clang/test/CXX/module/module.import/p6.cpp
===
--- clang/test/CXX/module/module.import/p6.cpp
+++ clang/test/CXX/module/module.import/p6.cpp
@@ -28,3 +28,11 @@
 static const int value = 43; 
 };
 
+void deleted_fn_ok (void) = delete;
+
+struct S {
+   ~S() noexcept(false) = default;
+private:
+  S(S&);
+};
+S::S(S&) = default;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -15254,9 +15254,15 @@
   }
 
   // C++ [module.import/6] external definitions are not permitted in header
-  // units.
+  // units.  Deleted and Defaulted functions are implicitly inline (but the
+  // inline state is not set at this point, so check the BodyKind explicitly).
+  // FIXME: Consider an alternate location for the test where the inlined()
+  // state is complete.
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
-  FD->getFormalLinkage() == Linkage::ExternalLinkage && !FD->isInlined()) {
+  FD->getFormalLinkage() == Linkage::ExternalLinkage &&
+  !FD->isInvalidDecl() && BodyKind != FnBodyKind::Delete &&
+  BodyKind != FnBodyKind::Default && !FD->isInlined()) {
+assert(FD->isThisDeclarationADefinition());
 Diag(FD->getLocation(), diag::err_extern_def_in_header_unit);
 FD->setInvalidDecl();
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141908: [C++20][Modules] Handle defaulted and deleted functions in header units.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D141908#4061451 , @ChuanqiXu wrote:

>> Well.. we have time for another iteration,
>
> I am going to take a vacation for the Chinese New Year since tomorrow to 
> February. So I am a little bit hurried : )

(I added the FIXME) Have a good holiday!




Comment at: clang/lib/Sema/SemaDecl.cpp:15258
+  // units.  Deleted and Defaulted functions are implicitly inline (but the
+  // inline state is not set at this point, so check the BodyKind explicitly).
   if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > iains wrote:
> > > > ChuanqiXu wrote:
> > > > > I prefer to add a FIXME here to say that we need to find a better 
> > > > > place for the check to eliminate the unnecessary check for `BodyKind 
> > > > > `. The current check for `BodyKind` looks a little bit hacky to me.
> > > > When the patch was originally done, this was found to be a good place 
> > > > to do the check (i.e. less duplication of testing and to avoid 
> > > > duplication of diagnostics) so I do not think I agree that there is a 
> > > > FIXME to move it.
> > > > 
> > > > BodyKind is already used elsewhere in this function for similar 
> > > > purposes - it does not look hacky to me.
> > > It looks hacky to me since we shouldn't care if it is deleted or 
> > > defaulted here and it should be enough to check `FD->isInlied()`.  And I 
> > > don't see similar usage of `BodyKind ` in this function.
> > > It looks hacky to me since we shouldn't care if it is deleted or 
> > > defaulted here and it should be enough to check `FD->isInlied()`. 
> > 
> > that means checking much later and in muliple places, I think - but if you 
> > want to make a follow-on patch, I will be happy to review.
> > 
> > > And I don't see similar usage of `BodyKind ` in this function.
> > 
> > line 15208?
> > 
> > 
> > line 15208?
> 
> line 15208 checks the wording `unless the function is deleted (C++ specifc, 
> C++ [dcl.fct.def.general]p2)`. So it is used to check if this is a delete 
> function, which looks fine. I mean it would be best if we can check 
> `FD->isInlined()` only.
> 
> > that means checking much later and in muliple places, I think - but if you 
> > want to make a follow-on patch, I will be happy to review.
> 
> I think I won't work on this. But it would be meaningful for future 
> developers. And it looks not impossible to refactor it. (I'm not asking you 
> to do the change)

> > that means checking much later and in muliple places, I think - but if you 
> > want to make a follow-on patch, I will be happy to review.
> 
> I think I won't work on this. But it would be meaningful for future 
> developers. 

I re-checked (to remind myself) .. essentially that state is set by 
SetFunctionBodyKind() which is called much later.  I still think we would most 
likely end up with having to place the diagnostic in multiple places.  However, 
I added the FIXME,

> And it looks not impossible to refactor it. (I'm not asking you to do the 
> change)

such a refactoring looks non-trivial and certainly not suitable for a few days 
before a branch ...



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141908

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

[PATCH] D140972: [flang] Add -fstack-arrays flag

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added inline comments.



Comment at: flang/include/flang/Tools/CLOptions.inc:159
 inline void createDefaultFIROptimizerPassPipeline(
-mlir::PassManager &pm, llvm::OptimizationLevel optLevel = defaultOptLevel) 
{
+mlir::PassManager &pm, bool stackArrays = false, llvm::OptimizationLevel 
optLevel = defaultOptLevel) {
   // simplify the IR

tblah wrote:
> awarzynski wrote:
> > CLANG-FORMAT-ME :) Same for other long lines here.
> It seems clang-format is not run on this file. I have fixed the lines changed 
> in my patch. Should I clang-format the whole file in a separate patch?
+1



Comment at: flang/test/Transforms/stack-arrays.f90:3
 
+! We have to check llvm ir here to force flang to run the whole mlir pipeline
+! this is just to check that -fstack-arrays enables the stack-arrays pass so

tblah wrote:
> awarzynski wrote:
> > Also, I don't quite follow this comment:
> > 
> > >  We have to check llvm ir here to force flang to run the whole mlir 
> > > pipeline
> > 
> > Why is checking LLVM IR going to force Flang to run anything?
> Just running `flang-new -fc1 -emit-fir` outputs the FIR before all of the 
> transformation passes run (which is why I have to pipe the result through 
> `fir-opt` to run the array value copy and stack arrays passes on the first 
> line).
> 
> Outputting LLVM IR requires all of the MLIR transformation passes to be run 
> to transform all of the higher level dialects into the LLVM dialect. Stack 
> arrays is run as part of that same pipeline 
> (`fir::createMLIRToLLVMPassPipeline`). I want to test that `-fstack-arrays` 
> does cause the stack arrays pass to run as part of that pipeline, which is 
> why I am checking the LLVM-IR.
> 
> One alternative would be to print out which passes have run and check that, 
> but I felt this way fits more with the spirit of the other tests.
Thanks for the explanation. So you want to verify this functionality by 
investigating two pipelines:
* source --> FIR
* source --> LLVM IR
You could convey that by using `CHECK-FIR` and `CHECK-LLVM-IR` (or just `FIR` 
and `LLVM-IR`). I "complained", because this is unclear:
> We have to check llvm ir here to force flang to run the whole mlir pipeline
Perhaps:
> In order to verify the whole MLIR pipeline, make the driver generate LLVM IR.



Comment at: flang/test/Transforms/stack-arrays.f90:6
+! only check the first example
+! RUN: %flang_fc1 -emit-llvm -o - -fstack-arrays %s | FileCheck 
--check-prefix=CHECK-LLVM %s
+

tblah wrote:
> awarzynski wrote:
> > 
> Lots of other tests do not follow this convention. It would change the 
> FileCheck comments to look like `! LLVM: array_value_copy_simple`. 
> 
> I think it is good to require `CHECK-FEATURE:` so that any mention of 
> `FEATURE` in comments does not confuse FileCheck. Especially for a name like 
> LLVM which is likely to be written in capitals.
> Lots of other tests do not follow this convention.

I think that we would easily find examples for either approach.

> I think it is good to require CHECK-FEATURE:

Well, `LLVM` is not a feature ;-) Also, most folks working with LLVM are 
familiar with the LIT syntax - the presence of `CHECK` in `CHECK-FEATURE` is 
just unnecessary noise to me. But I am happy either way. But I would appreciate 
replacing with `LLVM` with something a bit more meaningful - perhaps `LLVM-IR`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140972

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

[PATCH] D141768: [11/15][Clang][RISCV][NFC] Remove Policy::PolicyType::Omit

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

eopXD added inline comments.



Comment at: clang/lib/Support/RISCVVIntrinsicUtils.cpp:1032
+if (IsMasked) {
+  if (PolicyAttrs.isTUMAPolicy() && !HasMaskPolicy)
+appendPolicySuffix("_tum");

craig.topper wrote:
> If you hadn't just removed Omit, it somewhat feels like Omit should be part 
> of Policy and this code to pick the policy suffix string should be inside the 
> Policy class. But I'll defer to your judgement.
Good idea, I will encapsulate logics into `Policy`. Considering the next 
patch-set, to simplify the policy suffixes, `IsUnspecified` can be further 
removed. Eventually we can have something like:

```
void RVVIntrinsic::updateNamesAndPolicy(bool IsMasked, bool HasPolicy,
std::string &Name,
std::string &BuiltinName,
std::string &OverloadedName,
Policy &PolicyAttrs) {
  std::string PolicySuffix =
PolicyAttrs.getPolicySuffix(IsMasked, HasPolicy);
  Name += PolicySuffix;
  BuiltinName += PolicySuffix;
  OverloadedName += PolicySuffix;
}
```

I will create a separate revision so we won't be distracted towards the goal of 
this patch-set.

---

On the other hand, here is my thought process when considering the removal of 
`Omit` as a legit refactoring.

Logics to deal with `Omit` occurs under here (`updateNamesAndPolicy`), 
`computeBuiltinTypes`, and `getPolicyAttrs`. The `Omit` state specified to a 
`Policy` implies it has to be corrected to something specific (either Agnostic 
or Undisturbed) before calling `getPolicyAttrs`, which will set the values in 
`riscv_vector_cg` (called under `RISCVVEmitter.cpp`. This blocks the 
possibility of setting `TailPolicy`, `MaskPolicy` as constant members.

I picked `HasTailPolicy` and `HasMaskPolicy` for the predicates because it was 
exactly how `riscv_vector.td` defined the intrinsics. They are necessary as 
conditions now with the current policy suffix naming rules. With the next 
patch-set, `HasTailPolicy` and `HasMaskPolicy` will only be called by 
`getSupportedUnMaskedPolicies` and `getSupportedMaskedPolicies`. This is 
because the policy suffix logics here can be simplified to something like: 
(this does not contradict with the encapsulating topic above)

```
if (PolicyAttrs.isUnspecified()) {
  if (IsMasked) {
Name += "_m";
if (HasPolicy)
  BuiltinName += "_tama";
else
  BuiltinName += "_m";
  } else {
if (HasPolicy)
  BuiltinName += "_ta";
  }
} else {
  if (IsMasked) {
if (PolicyAttrs.isTAMUPolicy())
  appendPolicySuffix("_mu")
else if (PolicyAttrs.isTUMAPolicy())
  appendPolicySuffix("_tum")
else if (PolicyAttrs.isTUMUPolicy())
  appendPolicySuffix("_tumu")
else
  llvm_unreachable("Unhandled policy condition");
  } else {
if (PolicyAttrs.isTUPolicy())
  appendPolicySuffix("_tu");
else
  llvm_unreachable("Unhandled policy condition");
  }
}
```  




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141768

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

[PATCH] D142001: [clang] Use FP options from AST for emitting code for casts

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff created this revision.
sepavloff added reviewers: rjmccall, kpn, aaron.ballman.
Herald added a subscriber: pengfei.
Herald added a project: All.
sepavloff requested review of this revision.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142001

Files:
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/test/CodeGen/X86/avx512dq-builtins-constrained.c
  clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c

Index: clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c
===
--- clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c
+++ clang/test/CodeGen/aarch64-v8.2a-neon-intrinsics-constrained.c
@@ -21,7 +21,7 @@
 
 // Test that the constrained intrinsics are picking up the exception
 // metadata from the AST instead of the global default from the command line.
-// FIXME: All cases of "fpexcept.maytrap" in this test are wrong.
+// Any cases of "fpexcept.maytrap" in this test are clang bugs.
 
 #if EXCEPT
 #pragma float_control(except, on)
@@ -31,7 +31,7 @@
 
 // COMMON-LABEL: test_vsqrt_f16
 // UNCONSTRAINED:  [[SQR:%.*]] = call <4 x half> @llvm.sqrt.v4f16(<4 x half> %a)
-// CONSTRAINED:[[SQR:%.*]] = call <4 x half> @llvm.experimental.constrained.sqrt.v4f16(<4 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+// CONSTRAINED:[[SQR:%.*]] = call <4 x half> @llvm.experimental.constrained.sqrt.v4f16(<4 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.strict")
 // CHECK-ASM:  fsqrt v{{[0-9]+}}.4h, v{{[0-9]+}}.4h
 // COMMONIR:   ret <4 x half> [[SQR]]
 float16x4_t test_vsqrt_f16(float16x4_t a) {
@@ -40,7 +40,7 @@
 
 // COMMON-LABEL: test_vsqrtq_f16
 // UNCONSTRAINED:  [[SQR:%.*]] = call <8 x half> @llvm.sqrt.v8f16(<8 x half> %a)
-// CONSTRAINED:[[SQR:%.*]] = call <8 x half> @llvm.experimental.constrained.sqrt.v8f16(<8 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+// CONSTRAINED:[[SQR:%.*]] = call <8 x half> @llvm.experimental.constrained.sqrt.v8f16(<8 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.strict")
 // CHECK-ASM:  fsqrt v{{[0-9]+}}.8h, v{{[0-9]+}}.8h
 // COMMONIR:   ret <8 x half> [[SQR]]
 float16x8_t test_vsqrtq_f16(float16x8_t a) {
@@ -49,7 +49,7 @@
 
 // COMMON-LABEL: test_vfma_f16
 // UNCONSTRAINED:  [[ADD:%.*]] = call <4 x half> @llvm.fma.v4f16(<4 x half> %b, <4 x half> %c, <4 x half> %a)
-// CONSTRAINED:[[ADD:%.*]] = call <4 x half> @llvm.experimental.constrained.fma.v4f16(<4 x half> %b, <4 x half> %c, <4 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+// CONSTRAINED:[[ADD:%.*]] = call <4 x half> @llvm.experimental.constrained.fma.v4f16(<4 x half> %b, <4 x half> %c, <4 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.strict")
 // CHECK-ASM:  fmla v{{[0-9]+}}.4h, v{{[0-9]+}}.4h, v{{[0-9]+}}.4h
 // COMMONIR:   ret <4 x half> [[ADD]]
 float16x4_t test_vfma_f16(float16x4_t a, float16x4_t b, float16x4_t c) {
@@ -58,7 +58,7 @@
 
 // COMMON-LABEL: test_vfmaq_f16
 // UNCONSTRAINED:  [[ADD:%.*]] = call <8 x half> @llvm.fma.v8f16(<8 x half> %b, <8 x half> %c, <8 x half> %a)
-// CONSTRAINED:[[ADD:%.*]] = call <8 x half> @llvm.experimental.constrained.fma.v8f16(<8 x half> %b, <8 x half> %c, <8 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+// CONSTRAINED:[[ADD:%.*]] = call <8 x half> @llvm.experimental.constrained.fma.v8f16(<8 x half> %b, <8 x half> %c, <8 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.strict")
 // CHECK-ASM:  fmla v{{[0-9]+}}.8h, v{{[0-9]+}}.8h, v{{[0-9]+}}.8h
 // COMMONIR:   ret <8 x half> [[ADD]]
 float16x8_t test_vfmaq_f16(float16x8_t a, float16x8_t b, float16x8_t c) {
@@ -68,7 +68,7 @@
 // COMMON-LABEL: test_vfms_f16
 // COMMONIR:   [[SUB:%.*]] = fneg <4 x half> %b
 // UNCONSTRAINED:  [[ADD:%.*]] = call <4 x half> @llvm.fma.v4f16(<4 x half> [[SUB]], <4 x half> %c, <4 x half> %a)
-// CONSTRAINED:[[ADD:%.*]] = call <4 x half> @llvm.experimental.constrained.fma.v4f16(<4 x half> [[SUB]], <4 x half> %c, <4 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.maytrap")
+// CONSTRAINED:[[ADD:%.*]] = call <4 x half> @llvm.experimental.constrained.fma.v4f16(<4 x half> [[SUB]], <4 x half> %c, <4 x half> %a, metadata !"round.tonearest", metadata !"fpexcept.strict")
 // CHECK-ASM:  fmls v{{[0-9]+}}.4h, v{{[0-9]+}}.4h, v{{[0-9]+}}.4h
 // COMMONIR:   ret <4 x half> [[ADD]]
 float16x4_t test_vfms_f16(float16x4_t a, float16x4_t b, float16x4_t c) {
@@ -78,7 +78,7 @@
 // COMMON-LABEL: test_vfmsq_f16
 // COMMONIR:   [[SUB:%.*]] = fneg <8 x half> %b
 // UNCONSTRAINED:  [[ADD:%.*]] = call <8 x half> @llvm.fma.v8f16(<8 x half> [[SUB]], <8 x half> %c, <8 x half> %a)
-// CONSTRAINED:[[ADD:%.*]] = call <8 x half> @llvm.experimental.constrained.fma.v8f16(<8 x half> [[SUB]], <8 x half> %c, <8 x half> %a, metadata !"round.tonearest", 

[PATCH] D141573: [1/15][Clang][RISCV][NFC] Extract common utility to RISCVVIntrinsicUtils

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

eopXD added inline comments.



Comment at: clang/lib/Support/RISCVVIntrinsicUtils.cpp:977
 
+llvm::SmallVector RVVIntrinsic::getSupportedUnMaskedPolicies() {
+  return {

craig.topper wrote:
> Possible improvement for a different patch:
> 
> Could these policy lists be static const arrays and this function could 
> returned an ArrayRef pointing to the appropriate list instead constructing a 
> SmallVector?
Yes. I will use a flyweight pattern to simplify things here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141573

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

[PATCH] D141765: [FPEnv] Fix complex operations in strictfp mode

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff updated this revision to Diff 490088.
sepavloff added a comment.

Updated comment in test as proposed by Kevin


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141765

Files:
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/test/CodeGen/complex-strictfp.c

Index: clang/test/CodeGen/complex-strictfp.c
===
--- clang/test/CodeGen/complex-strictfp.c
+++ clang/test/CodeGen/complex-strictfp.c
@@ -5,8 +5,8 @@
 // Test that the constrained intrinsics are picking up the exception
 // metadata from the AST instead of the global default from the command line.
 // Include rounding metadata in the testing.
-// FIXME: All cases of "fpexcept.maytrap" in this test are wrong.
-// FIXME: All cases of "round.tonearest" in this test are wrong.
+// Any cases of "fpexcept.maytrap" in this test are clang bugs.
+// Any cases of "round.tonearest" in this test are clang bugs.
 
 #pragma float_control(except, on)
 #pragma STDC FENV_ROUND FE_UPWARD
@@ -15,7 +15,7 @@
 _Complex float cf;
 double D;
 
-// CHECK-LABEL: define {{[^@]+}}@test3a(
+// CHECK-LABEL: @test3a(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[TMP0:%.*]] = load double, ptr @D, align 8
 // CHECK-NEXT:[[CF_REAL:%.*]] = load float, ptr @cf, align 4
@@ -33,7 +33,7 @@
   cf += D;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@test3b(
+// CHECK-LABEL: @test3b(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[CF_REAL:%.*]] = load float, ptr @cf, align 4
 // CHECK-NEXT:[[CF_IMAG:%.*]] = load float, ptr getelementptr inbounds ({ float, float }, ptr @cf, i32 0, i32 1), align 4
@@ -48,7 +48,7 @@
   D += cf;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@test3c(
+// CHECK-LABEL: @test3c(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[G1_REAL:%.*]] = load double, ptr @g1, align 8
 // CHECK-NEXT:[[G1_IMAG:%.*]] = load double, ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
@@ -69,12 +69,12 @@
   cf /= g1;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@test3d(
+// CHECK-LABEL: @test3d(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[G1_REAL:%.*]] = load double, ptr @g1, align 8
 // CHECK-NEXT:[[G1_IMAG:%.*]] = load double, ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
 // CHECK-NEXT:[[TMP0:%.*]] = load double, ptr @D, align 8
-// CHECK-NEXT:[[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[G1_REAL]], double [[TMP0]], metadata !"round.tonearest", metadata !"fpexcept.maytrap") #[[ATTR2]]
+// CHECK-NEXT:[[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[G1_REAL]], double [[TMP0]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:store double [[ADD_R]], ptr @g1, align 8
 // CHECK-NEXT:store double [[G1_IMAG]], ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
 // CHECK-NEXT:ret void
@@ -83,12 +83,12 @@
   g1 = g1 + D;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@test3e(
+// CHECK-LABEL: @test3e(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[TMP0:%.*]] = load double, ptr @D, align 8
 // CHECK-NEXT:[[G1_REAL:%.*]] = load double, ptr @g1, align 8
 // CHECK-NEXT:[[G1_IMAG:%.*]] = load double, ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
-// CHECK-NEXT:[[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[TMP0]], double [[G1_REAL]], metadata !"round.tonearest", metadata !"fpexcept.maytrap") #[[ATTR2]]
+// CHECK-NEXT:[[ADD_R:%.*]] = call double @llvm.experimental.constrained.fadd.f64(double [[TMP0]], double [[G1_REAL]], metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:store double [[ADD_R]], ptr @g1, align 8
 // CHECK-NEXT:store double [[G1_IMAG]], ptr getelementptr inbounds ({ double, double }, ptr @g1, i32 0, i32 1), align 8
 // CHECK-NEXT:ret void
@@ -97,7 +97,7 @@
   g1 = D + g1;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@t1(
+// CHECK-LABEL: @t1(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[CONV:%.*]] = call float @llvm.experimental.constrained.fptrunc.f32.f64(double 4.00e+00, metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:store float [[CONV]], ptr @cf, align 4
@@ -107,7 +107,7 @@
   (__real__ cf) = 4.0;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@t2(
+// CHECK-LABEL: @t2(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[CONV:%.*]] = call float @llvm.experimental.constrained.fptrunc.f32.f64(double 4.00e+00, metadata !"round.upward", metadata !"fpexcept.strict") #[[ATTR2]]
 // CHECK-NEXT:store float [[CONV]], ptr getelementptr inbounds ({ float, float }, ptr @cf, i32 0, i32 1), align 4
@@ -117,7 +117,7 @@
   (__imag__ cf) = 4.0;
 }
 
-// CHECK-LABEL: define {{[^@]+}}@t91(
+// CHECK-LABEL: @t91(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[C:%.*]] = alloca [0 x i8], align 1
 // CHECK-NEXT:br i1 false, l

[PATCH] D141765: [FPEnv] Fix complex operations in strictfp mode

[Serge Pavlov via Phabricator via cfe-commits]

sepavloff added a comment.

In D141765#4059214 , @kpn wrote:

> Are we testing _Complex multiply or subtraction anywhere? I have a vague 
> memory of multiply not working correctly.

There is a test `clang/test/CodeGen/complex-math.c` that checks complex 
operations including subtraction and multiplication. The check for 
multiplication is not thorough however. And only default FP environment is 
checked.

IIRC spec2017 contains a test that uses C99 complex numbers. If there were 
issues, I think they would be reported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141765

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

[PATCH] D140927: [C++20][Modules] Fix named module import diagnostics.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 490099.
iains added a comment.

rebased


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140927

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/Modules/cxx20-import-diagnostics-a.cpp
  clang/test/Modules/cxx20-import-diagnostics-b.cpp

Index: clang/test/Modules/cxx20-import-diagnostics-b.cpp
===
--- /dev/null
+++ clang/test/Modules/cxx20-import-diagnostics-b.cpp
@@ -0,0 +1,61 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface  %t/a.cpp -o %t/a.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface  %t/c.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/c.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface  %t/d.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/d.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface  %t/e.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/e.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface  %t/a-part.cpp \
+// RUN: -o %t/a-part.pcm
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface  %t/f.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/f.pcm -verify
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface  %t/g.cpp \
+// RUN: -fmodule-file=%t/a.pcm -o %t/g.pcm -verify
+
+//--- a.cpp
+export module a;
+
+//--- b.hpp
+import a;
+
+//--- c.cpp
+module;
+#include "b.hpp"
+export module c;
+
+//--- d.cpp
+module;
+import a;
+
+export module d;
+
+//--- e.cpp
+export module e;
+
+module :private;
+import a;
+
+//--- a-part.cpp
+export module a:part;
+
+//--- f.cpp
+module;
+import :part ; // expected-error {{module partition imports cannot be in the global module fragment}}
+
+export module f;
+
+//--- g.cpp
+
+export module g;
+module :private;
+import :part; // expected-error {{module partition imports cannot be in the private module fragment}}
Index: clang/test/Modules/cxx20-import-diagnostics-a.cpp
===
--- clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -95,14 +95,13 @@
 //--- import-diags-tu7.cpp
 
 module;
-// We can only have preprocessor directives here, which permits an include-
-// translated header unit.  However those are identified specifically by the
-// preprocessor; non-preprocessed user code should not contain an 'import' here.
-import B; // expected-error {{module imports cannot be in the global module fragment}}
-
+// We can only have preprocessor directives here, which permits
+// header units (include-translated or not) and named modules.
+import B;
 export module D;
 
 int delta ();
+// expected-no-diagnostics
 
 //--- import-diags-tu8.cpp
 
@@ -112,7 +111,7 @@
 
 module :private;
 
-import B; // expected-error {{module imports cannot be in the private module fragment}}
+import B; // expected-error {{imports must immediately follow the module declaration}}
 
 //--- import-diags-tu9.cpp
 
Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -591,9 +591,6 @@
  (ModuleScopes.back().ModuleInterface ||
   (getLangOpts().CPlusPlusModules &&
ModuleScopes.back().Module->isGlobalModule( {
-assert((!ModuleScopes.back().Module->isGlobalModule() ||
-Mod->Kind == Module::ModuleKind::ModuleHeaderUnit) &&
-   "should only be importing a header unit into the GMF");
 // Re-export the module if the imported module is exported.
 // Note that we don't need to add re-exported module to Imports field
 // since `Exports` implies the module is imported already.
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -750,6 +750,10 @@
 else if (ImportState == Sema::ModuleImportState::ImportAllowed)
   // Non-imports disallow further imports.
   ImportState = Sema::ModuleImportState::ImportFinished;
+else if (ImportState ==
+ Sema::ModuleImportState::PrivateFragmentImportAllowed)
+  // Non-imports disallow further imports.
+  ImportState = Sema::ModuleImportState::PrivateFragmentImportFinished;
   }
   return false;
 }
@@ -2427,7 +2431,9 @@
 SourceLocation PrivateLoc = ConsumeToken();
 DiagnoseAndSkipCXX11Attributes();
 ExpectAndConsumeSemi(diag::err_private_module_fragment_expected_semi);
-ImportState = Sema::ModuleImportState::PrivateFragment;
+ImportState = ImportState == Sema::ModuleImportState::ImportAllowed
+  ? Sema::ModuleImportState::PrivateFragmentImportAllowed
+  : Sema::ModuleImportState::PrivateFragmentImportFinished;

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[J. Ryan Stinnett via Phabricator via cfe-commits]

jryans added a comment.

@calebzulawski This looks ready to land to me! 🙂 Do you have commit access? If 
not, I can land it for you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136315

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

[PATCH] D140972: [flang] Add -fstack-arrays flag

[Tom Eccles via Phabricator via cfe-commits]

tblah updated this revision to Diff 490100.
tblah marked 4 inline comments as done.
tblah added a comment.

Clarified a comment in stack-arrays.f90 and clarified the choice of FileCheck
prefix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140972

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Flang.cpp
  flang/docs/FlangDriver.md
  flang/include/flang/Frontend/CodeGenOptions.def
  flang/include/flang/Tools/CLOptions.inc
  flang/lib/Frontend/CompilerInvocation.cpp
  flang/lib/Frontend/FrontendActions.cpp
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/fast_math.f90
  flang/test/Transforms/stack-arrays.f90

Index: flang/test/Transforms/stack-arrays.f90
===
--- flang/test/Transforms/stack-arrays.f90
+++ flang/test/Transforms/stack-arrays.f90
@@ -1,5 +1,10 @@
 ! RUN: %flang_fc1 -emit-fir %s -o - | fir-opt --array-value-copy | fir-opt --stack-arrays | FileCheck %s
 
+! In order to verify the whole MLIR pipeline, make the driver generate LLVM IR.
+! This is only to check that -fstack-arrays enables the stack-arrays pass so
+! only check the first example
+! RUN: %flang_fc1 -emit-llvm -o - -fstack-arrays %s | FileCheck --check-prefix=LLVM-IR %s
+
 ! check simple array value copy case
 subroutine array_value_copy_simple(arr)
   integer, intent(inout) :: arr(4)
@@ -14,6 +19,15 @@
 ! CHECK: return
 ! CHECK-NEXT: }
 
+! LLVM-IR: array_value_copy_simple
+! LLVM-IR-NOT: malloc
+! LLVM-IR-NOT: free
+! LLVM-IR: alloca [4 x i32]
+! LLVM-IR-NOT: malloc
+! LLVM-IR-NOT: free
+! LLVM-IR: ret void
+! LLVM-IR-NEXT: }
+
 ! check complex array value copy case
 module stuff
   type DerivedWithAllocatable
Index: flang/test/Driver/fast_math.f90
===
--- flang/test/Driver/fast_math.f90
+++ flang/test/Driver/fast_math.f90
@@ -1,25 +1,35 @@
 ! Test for correct forwarding of fast-math flags from the compiler driver to the
 ! frontend driver
 
-! -Ofast => -ffast-math -O3
+! -Ofast => -ffast-math -O3 -fstack-arrays
 ! RUN: %flang -Ofast -fsyntax-only -### %s -o %t 2>&1 \
 ! RUN: | FileCheck --check-prefix=CHECK-OFAST %s
 ! CHECK-OFAST: -fc1
 ! CHECK-OFAST-SAME: -ffast-math
+! CHECK-OFAST-SAME: -fstack-arrays
 ! CHECK-OFAST-SAME: -O3
 
-! TODO: update once -fstack-arays is added
-! RUN: %flang -fstack-arrays -fsyntax-only %s -o %t 2>&1 \
+! RUN: %flang -fstack-arrays -fsyntax-only -### %s -o %t 2>&1 \
 ! RUN: | FileCheck --check-prefix=CHECK-STACK-ARRAYS %s
-! CHECK-STACK-ARRAYS: warning: argument unused during compilation: '-fstack-arrays'
+! CHECK-STACK-ARRAYS: -fc1
+! CHECK-STACK-ARRAYS-SAME: -fstack-arrays
 
-! -Ofast -fno-fast-math => -O3
+! -Ofast -fno-fast-math => -O3 -fstack-arrays
 ! RUN: %flang -Ofast -fno-fast-math -fsyntax-only -### %s -o %t 2>&1 \
 ! RUN: | FileCheck --check-prefix=CHECK-OFAST-NO-FAST %s
 ! CHECK-OFAST-NO-FAST: -fc1
 ! CHECK-OFAST-NO-FAST-NOT: -ffast-math
+! CHECK-OFAST-NO-FAST-SAME: -fstack-arrays
 ! CHECK-OFAST-NO-FAST-SAME: -O3
 
+! -Ofast -fno-stack-arrays -> -O3 -ffast-math
+! RUN: %flang -Ofast -fno-stack-arrays -fsyntax-only -### %s -o %t 2>&1 \
+! RUN: | FileCheck --check-prefix=CHECK-OFAST-NO-SA %s
+! CHECK-OFAST-NO-SA: -fc1
+! CHECK-OFAST-NO-SA-SAME: -ffast-math
+! CHECK-OFAST-NO-SA-NOT: -fstack-arrays
+! CHECK-OFAST-NO-SA-SAME: -O3
+
 ! -ffast-math => -ffast-math
 ! RUN: %flang -ffast-math -fsyntax-only -### %s -o %t 2>&1 \
 ! RUN: | FileCheck --check-prefix=CHECK-FFAST %s
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -45,10 +45,12 @@
 ! HELP-NEXT: -fno-color-diagnostics  Disable colors in diagnostics
 ! HELP-NEXT: -fno-integrated-as  Disable the integrated assembler
 ! HELP-NEXT: -fno-signed-zeros  Allow optimizations that ignore the sign of floating point zeros
+! HELP-NEXT: -fno-stack-arrays  Allocate array temporaries on the heap (default)
 ! HELP-NEXT: -fopenacc  Enable OpenACC
 ! HELP-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! HELP-NEXT: -fpass-plugin= Load pass plugin from a dynamic shared object file (only with new pass manager).
 ! HELP-NEXT: -freciprocal-math  Allow division operations to be reassociated
+! HELP-NEXT: -fstack-arrays Attempt to allocate array temporaries on the stack, no matter their size
 ! HELP-NEXT: -fsyntax-only  Run the preprocessor, parser and semantic analysis stages
 ! HELP-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
 ! HELP-NEXT: -help  Display available options
@@ -130,10 +132,12 @@
 ! HELP-FC1-NEXT: -fno-debug-pass-manager Disables debug printing for the new pass manager
 ! HELP-FC1-NEXT: -fno-r

[clang] 6898d84 - Reland "nullptr returned from ActOnTag() is not a valid result"

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2023-01-18T12:41:44+01:00
New Revision: 6898d8413ff4af205000eab1db3fa900b39c6097

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

LOG: Reland "nullptr returned from ActOnTag() is not a valid result"

The commit was reverted in 346e1c43a11b8af5a818dac321f83f043862c1ec as
part of the f1f0a0d8e8fdd2e534d9423b2e64c6b8aaa53aee revert.

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/Parser/cxx-undeclared-identifier.cpp
clang/test/Parser/recovery.cpp
clang/test/SemaCXX/invalid-template-params.cpp
clang/test/SemaCXX/rdar42746401.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 30c5ea608f7a0..6292da7f04822 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3304,22 +3304,24 @@ class Sema final {
 TUK_Friend   // Friend declaration:  'friend struct foo;'
   };
 
-  Decl *ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
- SourceLocation KWLoc, CXXScopeSpec &SS, IdentifierInfo *Name,
- SourceLocation NameLoc, const ParsedAttributesView &Attr,
- AccessSpecifier AS, SourceLocation ModulePrivateLoc,
- MultiTemplateParamsArg TemplateParameterLists, bool 
&OwnedDecl,
- bool &IsDependent, SourceLocation ScopedEnumKWLoc,
- bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
- bool IsTypeSpecifier, bool IsTemplateParamOrArg,
- SkipBodyInfo *SkipBody = nullptr);
-
-  Decl *ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
-unsigned TagSpec, SourceLocation TagLoc,
-CXXScopeSpec &SS, IdentifierInfo *Name,
-SourceLocation NameLoc,
-const ParsedAttributesView &Attr,
-MultiTemplateParamsArg TempParamLists);
+  DeclResult ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
+  SourceLocation KWLoc, CXXScopeSpec &SS,
+  IdentifierInfo *Name, SourceLocation NameLoc,
+  const ParsedAttributesView &Attr, AccessSpecifier AS,
+  SourceLocation ModulePrivateLoc,
+  MultiTemplateParamsArg TemplateParameterLists,
+  bool &OwnedDecl, bool &IsDependent,
+  SourceLocation ScopedEnumKWLoc,
+  bool ScopedEnumUsesClassTag, TypeResult UnderlyingType,
+  bool IsTypeSpecifier, bool IsTemplateParamOrArg,
+  SkipBodyInfo *SkipBody = nullptr);
+
+  DeclResult ActOnTemplatedFriendTag(Scope *S, SourceLocation FriendLoc,
+ unsigned TagSpec, SourceLocation TagLoc,
+ CXXScopeSpec &SS, IdentifierInfo *Name,
+ SourceLocation NameLoc,
+ const ParsedAttributesView &Attr,
+ MultiTemplateParamsArg TempParamLists);
 
   TypeResult ActOnDependentTag(Scope *S,
unsigned TagSpec,

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 56fe9c3ac7bac..56e8038d0d849 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -4972,7 +4972,7 @@ void Parser::ParseEnumSpecifier(SourceLocation StartLoc, 
DeclSpec &DS,
   DSC == DeclSpecContext::DSC_type_specifier,
   DSC == DeclSpecContext::DSC_template_param ||
   DSC == DeclSpecContext::DSC_template_type_arg,
-  &SkipBody);
+  &SkipBody).get();
 
   if (SkipBody.ShouldSkip) {
 assert(TUK == Sema::TUK_Definition && "can only skip a definition");

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index baadaf3210ee4..af20c70718cc4 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -16583,17 +16583,17 @@ static bool isAcceptableTagRedeclContext(Sema &S, 
DeclContext *OldDC,
 ///
 /// \param SkipBody If non-null, will be set to indicate if the caller should
 /// skip the definition of this tag and treat it as if it were a declaration.
-Decl *Sema::ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
- SourceLocation KWLoc, CXXScopeSpec &SS,
- IdentifierInfo *Name, SourceLocation NameLoc,
- const ParsedAttributesView &Attrs, AccessSpecifier AS,
- SourceLocation ModulePrivateLoc,
- Mult

[PATCH] D141580: [clang] Don't consider a nullptr returned from ActOnTag() as a valid result in ParseClassSpecifier.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D141580#4061205 , @aeubanks wrote:

> apologies for reverting this, but 
> https://reviews.llvm.org/rGf1f0a0d8e8fdd2e534d9423b2e64c6b8aaa53aee is broken 
> and doesn't revert cleanly without this also being reverted, could you reland?

I relanded it in 6898d8413ff4af205000eab1db3fa900b39c6097 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141580

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

[PATCH] D142007: [NFC] Fix "form/from" typos

[Piotr Fusik via Phabricator via cfe-commits]

pfusik created this revision.
Herald added subscribers: Moerafaat, zero9178, steakhal, bzcheeseman, sdasgup3, 
wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, 
Kayjukh, grosul1, martong, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, 
antiagainst, shauheen, rriddle, mehdi_amini, hiraditya, arichardson.
Herald added a reviewer: NoQ.
Herald added a reviewer: ributzka.
Herald added a project: All.
pfusik requested review of this revision.
Herald added a reviewer: nicolasvasilache.
Herald added subscribers: llvm-commits, libcxx-commits, lldb-commits, 
cfe-commits, stephenneuendorffer, nicolasvasilache.
Herald added projects: clang, LLDB, libc++, MLIR, LLVM.
Herald added a reviewer: libc++.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142007

Files:
  clang/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
  libcxx/include/__format/extended_grapheme_cluster_table.h
  libcxx/utils/generate_extended_grapheme_cluster_table.py
  lldb/include/lldb/Host/File.h
  lldb/source/Plugins/CMakeLists.txt
  lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h
  llvm/lib/Analysis/IVDescriptors.cpp
  llvm/lib/CodeGen/CodeGenPrepare.cpp
  llvm/lib/Target/AArch64/AArch64ISelDAGToDAG.cpp
  llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
  llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
  llvm/test/DebugInfo/ARM/PR26163.ll
  llvm/tools/llvm-exegesis/lib/Clustering.cpp
  llvm/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
  llvm/unittests/FuzzMutate/RandomIRBuilderTest.cpp
  mlir/lib/Dialect/Transform/IR/TransformDialect.cpp

Index: mlir/lib/Dialect/Transform/IR/TransformDialect.cpp
===
--- mlir/lib/Dialect/Transform/IR/TransformDialect.cpp
+++ mlir/lib/Dialect/Transform/IR/TransformDialect.cpp
@@ -74,7 +74,7 @@
 
 void transform::TransformDialect::mergeInPDLMatchHooks(
 llvm::StringMap &&constraintFns) {
-  // Steal the constraint functions form the given map.
+  // Steal the constraint functions from the given map.
   for (auto &it : constraintFns)
 pdlMatchHooks.registerConstraintFunction(it.getKey(), std::move(it.second));
 }
Index: llvm/unittests/FuzzMutate/RandomIRBuilderTest.cpp
===
--- llvm/unittests/FuzzMutate/RandomIRBuilderTest.cpp
+++ llvm/unittests/FuzzMutate/RandomIRBuilderTest.cpp
@@ -130,7 +130,7 @@
 }
 
 TEST(RandomIRBuilderTest, ShuffleVectorSink) {
-  // Check that we will never use shuffle vector mask as a sink form the
+  // Check that we will never use shuffle vector mask as a sink from the
   // unrelated operation.
 
   LLVMContext Ctx;
Index: llvm/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
===
--- llvm/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
+++ llvm/unittests/ExecutionEngine/MCJIT/MCJITCAPITest.cpp
@@ -6,7 +6,7 @@
 //
 //===--===//
 //
-// This test suite verifies basic MCJIT functionality when invoked form the C
+// This test suite verifies basic MCJIT functionality when invoked from the C
 // API.
 //
 //===--===//
Index: llvm/tools/llvm-exegesis/lib/Clustering.cpp
===
--- llvm/tools/llvm-exegesis/lib/Clustering.cpp
+++ llvm/tools/llvm-exegesis/lib/Clustering.cpp
@@ -314,7 +314,7 @@
   // Actually append to-be-moved points to the new cluster.
   UnstableCluster.PointIndices.insert(UnstableCluster.PointIndices.end(),
   it, OldCluster.PointIndices.end());
-  // And finally, remove "to-be-moved" points form the old cluster.
+  // And finally, remove "to-be-moved" points from the old cluster.
   OldCluster.PointIndices.erase(it, OldCluster.PointIndices.end());
   // Now, the old cluster may end up being empty, but let's just keep it
   // in whatever state it ended up. Purging empty clusters isn't worth it.
Index: llvm/test/DebugInfo/ARM/PR26163.ll
===
--- llvm/test/DebugInfo/ARM/PR26163.ll
+++ llvm/test/DebugInfo/ARM/PR26163.ll
@@ -17,7 +17,7 @@
 ; CHECK-NEXT: DW_AT_location (DW_OP_lit0, DW_OP_stack_value, DW_OP_piece 0x4)
 ; CHECK-NEXT: DW_AT_abstract_origin
 
-; Created form the following test case (PR26163) with
+; Created from the following test case (PR26163) with
 ; clang -cc1 -triple armv4t--freebsd11.0-gnueabi -emit-obj -debug-info-kind=standalone -O2 -x c test.c
 ;
 ; typedef	unsigned int	size_t;
Index: llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
===
--- llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
+++ llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
@@ -722,7 +722,7 @@
 
 /// Updates the operand at Idx in ins

[PATCH] D140915: [clangd] Fix getQueryScopes for using-directive with inline namespace

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:646
   // "" for global namespace, "ns::" for normal namespace.
+  std::vector QueryScopes;
+  // Like QueryScopes, but also contains inline namespaces.

could you actually change the comments above to showcase an inline namespace 
and keep this as `AccessibleScopes`.

Then introduce `QueryScopes` with a comment saying that `This is an 
overestimate of AccessibleScopes, e.g. it ignores inline namespaces, to fetch 
more relevant symbols from index`.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:653
 
+  // Construct scopes to use for code completion. The results are deduplicated.
+  std::vector scopesForCodeCompletion() {

`Scopes that are accessible from current context. Used for dropping unnecessary 
namespecifiers.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:654
+  // Construct scopes to use for code completion. The results are deduplicated.
+  std::vector scopesForCodeCompletion() {
+std::set Results;

s/scopesForCodeCompletion/scopesForQualificaiton



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:676
 // (e.g. enclosing namespace).
-std::pair, bool>
+std::tuple, std::vector, bool>
 getQueryScopes(CodeCompletionContext &CCContext, const Sema &CCSema,

can we have a dedicated struct for return type here? especially the "enclosing 
namespace first" logic seem to be getting out of control. so what about 
updating `SpecificedScope` to have two new fields `std::optional 
EnclosingNamespace;` and a `bool AllowAllScopes;`? that way we can just call 
getQueryScopes/getAccessibleScopes on needed places and make sure they're 
putting the EnclosingNamespace (if it exists) in front.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:712
+std::vector EnclosingAtFrontForCompletion;
 std::string EnclosingScope = printNamespaceScope(*CCSema.CurContext);
+EnclosingAtFrontForIndex.push_back(EnclosingScope);

note that this is actually going to skip inline namespaces (and you're using 
that in the returned set)



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1660
 // First scope is the (modified) enclosing scope.
 QueryScopes = Scopes.scopesForIndexQuery();
 ScopeProximity.emplace(QueryScopes);

we should be setting `AccessibleScopes` too



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:674
+else if (const auto *ND = dyn_cast(Context)) {
+  if (ND->isInlineNamespace())
+Scopes.AccessibleScopes.push_back(printQualifiedName(*ND) + "::");

tom-anders wrote:
> kadircet wrote:
> > tom-anders wrote:
> > > kadircet wrote:
> > > > tom-anders wrote:
> > > > > kadircet wrote:
> > > > > > since we know that the `Context` is a `NamespaceDecl` it should be 
> > > > > > safe to use `printQualifiedName` always. any reason for the extra 
> > > > > > branching here (apart from minimizing the change to behaviour)? if 
> > > > > > not I think we can get rid of the special casing.
> > > > > Unfortunately, this fails CompletionTest.EnclosingScopeComesFirst and 
> > > > > CompletionTest.NoDuplicatedQueryScopes, I think because of anonymous 
> > > > > namespaces (where printNameSpaceScope would return an empty string, 
> > > > > but (printQualifiedName(*ND) + "::" does not). 
> > > > i see. taking a closer look at this `getQueryScopes` is used for two 
> > > > things:
> > > > - The scopes to query with fuzzyfind requests, hence this should use 
> > > > the same "serialization" as symbolcollector (which only strips anon 
> > > > namespaces today, but initially it were to strip both anon & inline 
> > > > namespaces. it got changed inside clang without clangd tests catching 
> > > > it).
> > > > - The shortening of the fully qualified name in 
> > > > `CodeCompletionBuilder`. Not having inline namespaces spelled in the 
> > > > available namespaces implies getting wrong qualifiers (such as the bug 
> > > > you're fixing).
> > > > 
> > > > so considering the requirements here:
> > > > - when querying index, we actually want to hide inline namespaces (as 
> > > > `ns::hidden::Foo` should be a viable alternative even if only `ns::` is 
> > > > accessible). so we should actually fix `printQualifiedName` to set 
> > > > `SuppressInlineNamespace` in printing policy to restore the old 
> > > > behaviour (and keep using `printNamespaceScope` here).
> > > > - inside `CodeCompletionBuilder`, we shouldn't use the same scopes we 
> > > > use during index queries. we should use the visible namespaces while 
> > > > preserving inline namespace information and only ignoring the anonymous 
> > > > namespaces.
> > > > 
> > > > hence can we have 2 separate scopes in `CodeCompleteFlow` instead?
> > > > One called `QueryScopes`, which has the behavior we have t

[clang] 0aaeb25 - [include-mapping] Fix gen_std.py test

[Viktoriia Bakalova via cfe-commits]

Author: Viktoriia Bakalova
Date: 2023-01-18T13:07:41Z
New Revision: 0aaeb25525ecbc667a9029a4be0d87085392954a

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

LOG: [include-mapping] Fix gen_std.py test

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

Added: 


Modified: 
clang/tools/include-mapping/test.py

Removed: 




diff  --git a/clang/tools/include-mapping/test.py 
b/clang/tools/include-mapping/test.py
index 9fad952b2e97c..507e462e75273 100755
--- a/clang/tools/include-mapping/test.py
+++ b/clang/tools/include-mapping/test.py
@@ -24,11 +24,11 @@ def testParseIndexPage(self):
 
 actual = _ParseIndexPage(html)
 expected = [
-  ("abs", "abs.html", True),
-  ("abs", "complex/abs.html", True),
-  ("acos", "acos.html", False),
-  ("acosh", "acosh.html", False),
-  ("as_bytes", "as_bytes.html", False),
+  ("abs", "abs.html", 'int'),
+  ("abs", "complex/abs.html", 'std::complex'),
+  ("acos", "acos.html", None),
+  ("acosh", "acosh.html", None),
+  ("as_bytes", "as_bytes.html", None),
 ]
 self.assertEqual(len(actual), len(expected))
 for i in range(0, len(actual)):



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

[PATCH] D141944: [include-mapping] Fix gen_std.py test

[Viktoriia Bakalova via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG0aaeb25525ec: [include-mapping] Fix gen_std.py test 
(authored by VitaNuo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141944

Files:
  clang/tools/include-mapping/test.py


Index: clang/tools/include-mapping/test.py
===
--- clang/tools/include-mapping/test.py
+++ clang/tools/include-mapping/test.py
@@ -24,11 +24,11 @@
 
 actual = _ParseIndexPage(html)
 expected = [
-  ("abs", "abs.html", True),
-  ("abs", "complex/abs.html", True),
-  ("acos", "acos.html", False),
-  ("acosh", "acosh.html", False),
-  ("as_bytes", "as_bytes.html", False),
+  ("abs", "abs.html", 'int'),
+  ("abs", "complex/abs.html", 'std::complex'),
+  ("acos", "acos.html", None),
+  ("acosh", "acosh.html", None),
+  ("as_bytes", "as_bytes.html", None),
 ]
 self.assertEqual(len(actual), len(expected))
 for i in range(0, len(actual)):


Index: clang/tools/include-mapping/test.py
===
--- clang/tools/include-mapping/test.py
+++ clang/tools/include-mapping/test.py
@@ -24,11 +24,11 @@
 
 actual = _ParseIndexPage(html)
 expected = [
-  ("abs", "abs.html", True),
-  ("abs", "complex/abs.html", True),
-  ("acos", "acos.html", False),
-  ("acosh", "acosh.html", False),
-  ("as_bytes", "as_bytes.html", False),
+  ("abs", "abs.html", 'int'),
+  ("abs", "complex/abs.html", 'std::complex'),
+  ("acos", "acos.html", None),
+  ("acosh", "acosh.html", None),
+  ("as_bytes", "as_bytes.html", None),
 ]
 self.assertEqual(len(actual), len(expected))
 for i in range(0, len(actual)):
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140972: [flang] Add -fstack-arrays flag

[Andrzej Warzynski via Phabricator via cfe-commits]

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

LGTM, thanks!




Comment at: flang/test/Transforms/stack-arrays.f90:22-29
+! LLVM-IR: array_value_copy_simple
+! LLVM-IR-NOT: malloc
+! LLVM-IR-NOT: free
+! LLVM-IR: alloca [4 x i32]
+! LLVM-IR-NOT: malloc
+! LLVM-IR-NOT: free
+! LLVM-IR: ret void

Hopefully I got this right, it's been a while :) 
https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140972

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

[PATCH] D141855: [include-mapping] Parse zombie_names.html into a removed symbols map.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 490117.
VitaNuo marked an inline comment as done.
VitaNuo added a comment.

Minor adjustments in accordance with review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141855

Files:
  clang/tools/include-mapping/gen_std.py


Index: clang/tools/include-mapping/gen_std.py
===
--- clang/tools/include-mapping/gen_std.py
+++ clang/tools/include-mapping/gen_std.py
@@ -28,9 +28,11 @@
  get a "cppreference/reference" directory.
   4. Run the command:
// Generate C++ symbols
-   python3 gen_std.py -cppreference cppreference/reference -language=cpp > 
StdSymbolMap.inc
+   python3 gen_std.py -cppreference cppreference/reference -symbols=cpp > 
StdSymbolMap.inc
+   // Generate C++ removed symbols
+   python3 gen_std.py -cppreference cppreference/reference 
-symbols=cpp_removed > RemovedSymbolMap.inc
// Generate C symbols
-   python3 gen_std.py -cppreference cppreference/reference -language=c > 
CSymbolMap.inc
+   python3 gen_std.py -cppreference cppreference/reference -symbols=c > 
CSymbolMap.inc
 """
 
 
@@ -60,16 +62,16 @@
   help='path to the cppreference offline HTML directory',
   required=True
  )
-  parser.add_argument('-language',
+  parser.add_argument('-symbols',
   default='cpp',
-  help='Generate c or cpp symbols',
-  required=True)
+  help='Generate c or cpp (removed) symbols. One of {cpp, 
c, cpp_removed}.',
+  required=True) 
   return parser.parse_args()
 
 
 def main():
   args = ParseArg()
-  if args.language == 'cpp':
+  if args.symbols == 'cpp':
 page_root = os.path.join(args.cppreference, "en", "cpp")
 symbol_index_root = os.path.join(page_root, "symbol_index")
 parse_pages =  [
@@ -87,22 +89,26 @@
   (symbol_index_root, "regex_constants.html", "std::regex_constants::"),
   (symbol_index_root, "this_thread.html", "std::this_thread::"),
 ]
-  elif args.language == 'c':
+  elif args.symbols == 'cpp_removed':
+page_root = os.path.join(args.cppreference, "en", "cpp")
+symbol_index_root = os.path.join(page_root, "symbol_index")
+parse_pages = [(symbol_index_root, "zombie_names.html", "std::")]
+  elif args.symbols == 'c':
 page_root = os.path.join(args.cppreference, "en", "c")
 symbol_index_root = page_root
-parse_pages = [(page_root, "index.html", None)]
-
+parse_pages = [(page_root, "index.html", None)]  
+
   if not os.path.exists(symbol_index_root):
 exit("Path %s doesn't exist!" % symbol_index_root)
 
   symbols = cppreference_parser.GetSymbols(parse_pages)
-
+  
   # We don't have version information from the unzipped offline HTML files.
   # so we use the modified time of the symbol_index.html as the version.
   index_page_path = os.path.join(page_root, "index.html")
   cppreference_modified_date = datetime.datetime.fromtimestamp(
 os.stat(index_page_path).st_mtime).strftime('%Y-%m-%d')
-  print(CODE_PREFIX % (args.language.upper(), cppreference_modified_date))
+  print(CODE_PREFIX % (args.symbols.upper(), cppreference_modified_date))
   for symbol in symbols:
 if len(symbol.headers) == 1:
   # SYMBOL(unqualified_name, namespace, header)


Index: clang/tools/include-mapping/gen_std.py
===
--- clang/tools/include-mapping/gen_std.py
+++ clang/tools/include-mapping/gen_std.py
@@ -28,9 +28,11 @@
  get a "cppreference/reference" directory.
   4. Run the command:
// Generate C++ symbols
-   python3 gen_std.py -cppreference cppreference/reference -language=cpp > StdSymbolMap.inc
+   python3 gen_std.py -cppreference cppreference/reference -symbols=cpp > StdSymbolMap.inc
+   // Generate C++ removed symbols
+   python3 gen_std.py -cppreference cppreference/reference -symbols=cpp_removed > RemovedSymbolMap.inc
// Generate C symbols
-   python3 gen_std.py -cppreference cppreference/reference -language=c > CSymbolMap.inc
+   python3 gen_std.py -cppreference cppreference/reference -symbols=c > CSymbolMap.inc
 """
 
 
@@ -60,16 +62,16 @@
   help='path to the cppreference offline HTML directory',
   required=True
  )
-  parser.add_argument('-language',
+  parser.add_argument('-symbols',
   default='cpp',
-  help='Generate c or cpp symbols',
-  required=True)
+  help='Generate c or cpp (removed) symbols. One of {cpp, c, cpp_removed}.',
+  required=True) 
   return parser.parse_args()
 
 
 def main():
   args = ParseArg()
-  if args.language == 'cpp':
+  if args.symbols == 'cpp':

[PATCH] D141855: [include-mapping] Parse zombie_names.html into a removed symbols map.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Thanks for the comments!




Comment at: clang/tools/include-mapping/gen_std.py:31
// Generate C++ symbols
python3 gen_std.py -cppreference cppreference/reference -language=cpp > 
StdSymbolMap.inc
// Generate C symbols

hokein wrote:
> nit: update the doc.
Ah thanks for the reminder!



Comment at: clang/tools/include-mapping/gen_std.py:94
 parse_pages = [(page_root, "index.html", None)]
-
+  elif args.symbols == 'cppremoved':
+page_root = os.path.join(args.cppreference, "en", "cpp")

hokein wrote:
> nit: can we add a `_` between `cpp` and `removed`. And maybe move it after 
> the if `args.symbols == 'cpp'` branch as they are `cpp` related.
Ok, sure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141855

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Erich Keane via Phabricator via cfe-commits]

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

In D141954#4060647 , @philnik wrote:

> In D141954#4060246 , @erichkeane 
> wrote:
>
>> In D141954#4060244 , @philnik 
>> wrote:
>>
>>> In D141954#4060212 , @erichkeane 
>>> wrote:
>>>
 @ldionne @Mordante  The libcxx failure isn't clear what it is complaining 
 about, but I thought this fix might end up breaking a libcxx test, since 
 it would be easy to fall into the trap of having an implicit conversion 
 here.

 Any chance you could prod at that test a little and see whose fault it is?

 ALSO: NOTE TO SELF: need release note!
>>>
>>> I think the problem is that `__type_traits/common_reference.h` doesn't 
>>> re-export `__type_traits/common_type.h` in the modulemap. It //should// be 
>>> enough to add `export common_type` to `module common_reference` 
>>> (`libcxx/include/module.modulemap.in:1399`). See `module is_arithmetic` for 
>>> an example. This is just a guess though, this stuff can be very weird.
>>
>> Oh man, I hope the others know what you're talking about :)  I don't really 
>> know how I caused this though :/
>
> I've applied your patch locally and this diff fixes the issue:
>
>   diff --git a/libcxx/include/module.modulemap.in 
> b/libcxx/include/module.modulemap.in
>   index 5d4cf53aa334..30209b353d6b 100644
>   --- a/libcxx/include/module.modulemap.in
>   +++ b/libcxx/include/module.modulemap.in
>   @@ -747,7 +747,10 @@ module std [system] {
>  module assignable { private header 
> "__concepts/assignable.h" }
>  module boolean_testable   { private header 
> "__concepts/boolean_testable.h" }
>  module class_or_enum  { private header 
> "__concepts/class_or_enum.h" }
>   -  module common_reference_with  { private header 
> "__concepts/common_reference_with.h" }
>   +  module common_reference_with  {
>   +private header "__concepts/common_reference_with.h"
>   +export type_traits.common_reference
>   +  }
>  module common_with{ private header 
> "__concepts/common_with.h" }
>  module constructible  { private header 
> "__concepts/constructible.h" }
>  module convertible_to { private header 
> "__concepts/convertible_to.h" }
>
> I don't know why your patch changes the behaviour of clang here, but I 
> wouldn't worry too much about it. There is a lot of weirdness around modules 
> and dependent type names resulting in pretty much useless error messages. I 
> hope the work on C++ modules reduces this weirdness, but I don't really know 
> how that stuff works.

Thanks!  I'll apply that patch here and see if it makes the bot happy.


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

https://reviews.llvm.org/D141954

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 490120.
erichkeane added a reviewer: libc++.
erichkeane added a comment.

Add module map fix, add nit from Shafik.


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

https://reviews.llvm.org/D141954

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaConcept.cpp
  
clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
  libcxx/include/module.modulemap.in

Index: libcxx/include/module.modulemap.in
===
--- libcxx/include/module.modulemap.in
+++ libcxx/include/module.modulemap.in
@@ -747,7 +747,10 @@
   module assignable { private header "__concepts/assignable.h" }
   module boolean_testable   { private header "__concepts/boolean_testable.h" }
   module class_or_enum  { private header "__concepts/class_or_enum.h" }
-  module common_reference_with  { private header "__concepts/common_reference_with.h" }
+  module common_reference_with  { 
+private header "__concepts/common_reference_with.h" 
+export type_traits.common_reference
+  }
   module common_with{ private header "__concepts/common_with.h" }
   module constructible  { private header "__concepts/constructible.h" }
   module convertible_to { private header "__concepts/convertible_to.h" }
Index: clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
===
--- /dev/null
+++ clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++20 -x c++ -verify %s
+
+template concept C =
+sizeof(T) == 4 && !true;  // requires atomic constraints sizeof(T) == 4 and !true
+
+template struct S {
+  constexpr operator bool() const { return true; }
+};
+
+// expected-error@+3{{atomic constraint must be of type 'bool' (found 'S')}}
+// expected-note@#FINST{{while checking constraint satisfaction}}
+// expected-note@#FINST{{in instantiation of function template specialization}}
+template requires (S{})
+void f(T);
+void f(int);
+
+// Ensure this applies to operator && as well.
+// expected-error@+3{{atomic constraint must be of type 'bool' (found 'S')}}
+// expected-note@#F2INST{{while checking constraint satisfaction}}
+// expected-note@#F2INST{{in instantiation of function template specialization}}
+template requires (S{} && true)
+void f2(T);
+void f2(int);
+
+void g() {
+  f(0); // #FINST
+  f2(0); // #F2INST
+}  
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -329,14 +329,7 @@
   Sema::SFINAETrap Trap(S);
   SubstitutedExpression =
   S.SubstConstraintExpr(const_cast(AtomicExpr), MLTAL);
-  // Substitution might have stripped off a contextual conversion to
-  // bool if this is the operand of an '&&' or '||'. For example, we
-  // might lose an lvalue-to-rvalue conversion here. If so, put it back
-  // before we try to evaluate.
-  if (SubstitutedExpression.isUsable() &&
-  !SubstitutedExpression.isInvalid())
-SubstitutedExpression =
-S.PerformContextuallyConvertToBool(SubstitutedExpression.get());
+
   if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) {
 // C++2a [temp.constr.atomic]p1
 //   ...If substitution results in an invalid type or expression, the
@@ -370,8 +363,24 @@
   }
 }
 
+// [temp.constr.atomic]p3: To determine if an atomic constraint is
+// satisfied, the parameter mapping and template arguments are first
+// substituted into its expression.  If substitution results in an
+// invalid type or expression, the constraint is not satisfied.
+// Otherwise, the lvalue-to-rvalue conversion is performed if necessary,
+// and E shall be a constant expression of type bool.
+//
+// Perform the L to R Value conversion if necessary. We do so for all
+// non-PRValue categories, else we fail to extend the lifetime of
+// temporaries, and that fails the constant expression check.
+if (!SubstitutedExpression.get()->isPRValue())
+  SubstitutedExpression = ImplicitCastExpr::Create(
+  S.Context, SubstitutedExpression.get()->getType(),
+  CK_LValueToRValue, SubstitutedExpression.get(),
+  /*BasePath=*/nullptr, VK_PRValue, FPOptionsOverride());
+
 if (!S.CheckConstraintExpression(SubstitutedExpression.get()))
-  return ExprError();
+  return ExprError(); // convert to bool here?
 
 return SubstitutedExpression;
   });
Index: clang/docs/ReleaseN

[clang] e7300e7 - Diagnose extensions in 'offsetof'

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-01-18T08:51:21-05:00
New Revision: e7300e75b51a7e7d4e81975b4be7a6c65f9a8286

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

LOG: Diagnose extensions in 'offsetof'

https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2350.htm made very
clear that it is an UB having type definitions with in offsetof.
Clang supports defining a type as the first argument as a conforming
extension due to how many projects use the construct in C99 and earlier
to calculate the alignment of a type. GCC also supports defining a type
as the first argument.

This adds extension warnings and documentation for the functionality
Clang explicitly supports.

Fixes #57065
Reverts the revert of 39da55e8f548a11f7dadefa73ea73d809a5f1729

Co-authored-by: Yingchi Long 
Co-authored-by: Aaron Ballman 

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

Added: 
clang/test/C/C2x/n2350.c

Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Parse/RAIIObjectsForParser.h
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Parse/ParseExpr.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/test/C/drs/dr4xx.c
clang/test/CXX/drs/dr4xx.cpp
clang/test/CodeGen/offsetof.c
clang/test/Parser/declarators.c
clang/test/Sema/offsetof.c
clang/test/SemaCXX/offsetof.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index 9b39dc23e0a5c..dbec1cd7c1966 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2360,6 +2360,50 @@ evaluated, so any side effects of the expression will be 
discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 
+``__builtin_offsetof``
+--
+
+``__builtin_offsetof`` is used to implement the ``offsetof`` macro, which
+calculates the offset (in bytes) to a given member of the given type.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_offsetof(type-name, member-designator)
+
+**Example of Use**:
+
+.. code-block:: c++
+
+  struct S {
+char c;
+int i;
+struct T {
+  float f[2];
+} t;
+  };
+
+  const int offset_to_i = __builtin_offsetof(struct S, i);
+  const int ext1 = __builtin_offsetof(struct U { int i; }, i); // C extension
+  const int ext2 = __builtin_offsetof(struct S, t.f[1]); // C & C++ extension
+
+**Description**:
+
+This builtin is usable in an integer constant expression which returns a value
+of type ``size_t``. The value returned is the offset in bytes to the subobject
+designated by the member-designator from the beginning of an object of type
+``type-name``. Clang extends the required standard functionality in a few ways:
+
+* In C language modes, the first argument may be the definition of a new type.
+  Any type declared this way is scoped to the nearest scope containing the call
+  to the builtin.
+* The second argument may be a member-designator designated by a series of
+  member access expressions using the dot (``.``) operator or array subscript
+  expressions.
+
+Query for this feature with ``__has_builtin(__builtin_offsetof)``.
+
 ``__builtin_call_with_static_chain``
 
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index b912f77887f9a..288904b023863 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -504,7 +504,8 @@ Non-comprehensive list of changes in this release
 - Clang can now generate a PCH when using ``-fdelayed-template-parsing`` for
   code with templates containing loop hint pragmas, OpenMP pragmas, and
   ``#pragma unused``.
-
+- Now diagnoses use of a member access expression or array subscript expression
+  within ``__builtin_offsetof`` and ``offsetof`` as being a Clang extension.
 
 New Compiler Flags
 --
@@ -684,6 +685,12 @@ C2x Feature Support
   va_end(list);
 }
 
+- Diagnose type definitions in the ``type`` argument of ``__builtin_offsetof``
+  as a conforming C extension according to
+  `WG14 N2350 `_.
+  Also documents the builtin appropriately. Note, a type definition in C++
+  continues to be rejected.
+
 C++ Language Changes in Clang
 -
 - Implemented `DR692 `_, `DR1395 
`_,

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Also, I forgot I added the release notes in that last patch :)


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

https://reviews.llvm.org/D141954

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

[PATCH] D133574: [C2x] reject type definitions in offsetof

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D133574#4061428 , @mstorsjo wrote:

> For clarity in the review - the relanded commit ended up reverted by 
> @aeubanks in 39da55e8f548a11f7dadefa73ea73d809a5f1729 
> . The 
> relanded commit triggers failed asserts like this:
>
>   $ echo 'void c() { __builtin_offsetof(struct {int b;}, b); }' | bin/clang 
> -cc1 -emit-llvm -o /dev/null - -x c
>   clang: ../../clang/lib/AST/RecordLayoutBuilder.cpp:3286: const 
> clang::ASTRecordLayout& clang::ASTContext::getASTRecordLayout(const 
> clang::RecordDecl*) const: Assertion `!D->isInvalidDecl() && "Cannot get 
> layout of invalid decl!"' failed.
>
> I also ran into a second issue with the reapplied version of the commit:
>
>   $ echo 'int i = __builtin_offsetof(struct {int b;}, b);' | bin/clang -cc1 
> -emit-llvm -o /dev/null - -x c
>   :1:9: error: initializer element is not a compile-time constant
>   int i = __builtin_offsetof(struct {int b;}, b);
>   ^~
>   1 error generated.
>
> (This used to compile successfully before.)

Sorry for the trouble, there was a declaration being marked as invalid despite 
us *warning* on it rather than erroring on it. I've corrected it and re-landed 
in e7300e75b51a7e7d4e81975b4be7a6c65f9a8286 
 with 
additional test coverage for both of the scenarios discovered. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133574

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

[PATCH] D136315: [clang][Darwin] Try to guess the SDK root with xcrun when unspecified

[Caleb Zulawski via Phabricator via cfe-commits]

calebzulawski added a comment.

@jryans I do not, I would appreciate that. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136315

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

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang/lib/AST/Interp/Pointer.h:63-64
 private:
   static constexpr unsigned PastEndMark = (unsigned)-1;
   static constexpr unsigned RootPtrMark = (unsigned)-1;
 

aaron.ballman wrote:
> tbaeder wrote:
> > shafik wrote:
> > > or `-1u`  
> > Alright, but those were not introduced in this patch.
> Preference for `~0u` so the types all match up nicely, fine to do in a 
> post-commit NFC change.
Don't forget to hit this one in post-commit.


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

https://reviews.llvm.org/D135750

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Speaking of: I guess the libcxx failure is NOT caused by me?  
https://reviews.llvm.org/D141992

I see this on a different review, despite this not being committed.


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

https://reviews.llvm.org/D141954

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

[PATCH] D137070: [clang][Interp] Support destructors

[Aaron Ballman via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1900-1902
+/// When calling this, we have a pointer of the local-to-destroy
+/// on the stack.
+/// Emit destruction of record types (or arrays of record types).

As a FIXME: you should also handle virtual destructors at some point, whenever 
you get around to handling virtual functions in general.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1915-1916
+
+if (const CXXDestructorDecl *Dtor = ElemRecord->getDestructor();
+Dtor && !Dtor->isTrivial()) {
+  for (size_t I = 0, E = Desc->getNumElems(); I != E; ++I) {

`isTrivial()` only works once the class has been fully built up by Sema IIRC; 
we should have a test case for that situation.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1917
+Dtor && !Dtor->isTrivial()) {
+  for (size_t I = 0, E = Desc->getNumElems(); I != E; ++I) {
+if (!this->emitConstUint64(I, SourceInfo{}))

This looks like it will destroy the array elements in order instead of in 
reverse order -- need test coverage for that. See 
https://eel.is/c++draft/class.dtor#13.sentence-5



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1934
+const Descriptor *D = Field.Desc;
+if (!D->isPrimitive() && !D->isPrimitiveArray()) {
+  if (!this->emitDupPtr(SourceInfo{}))

Won't this also destroy static data members? (Needs a test case for that.)

Also, what if the record is a union and not a structure? We only want to 
destroy the active member in that case, not all of the variant members, right? 
(Also needs a test case.)

See http://eel.is/c++draft/class.dtor#13



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1958-1963
+  for (const Record::Base &Base : llvm::reverse(R->bases())) {
+if (!this->emitGetPtrBase(Base.Offset, SourceInfo{}))
+  return false;
+if (!this->emitRecordDestruction(Base.Desc))
+  return false;
+  }

I don't think we handle virtual functions yet so I doubt we handle virtual 
bases, but that's something that might need a FIXME comment.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:345
 
-  ~LocalScope() override { this->emitDestruction(); }
+  virtual ~LocalScope() override { this->emitDestruction(); }
 

No need to add the `virtual` here as the `override` already signals that (can't 
override a non-virtual function).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137070

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

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D138958#4060633 , @lebedev.ri 
wrote:

> Ok, if we must not unconditionally emit the memory attributes, then let's not.
> Please stamp? :)

FWIW, I don't think we should land any of this until after the Clang 16 branch 
has happened; I think we need more time for this to bake before we release it 
to the wild.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Nikolas Klauser via Phabricator via cfe-commits]

philnik added a comment.

In D141954#4061962 , @erichkeane 
wrote:

> Speaking of: I guess the libcxx failure is NOT caused by me?  
> https://reviews.llvm.org/D141992
>
> I see this on a different review, despite this not being committed.

That's not great. Seems like some recently landed patch broke this. Do you mind 
committing the libc++-part of this patch to get the Clang CI green again? 
(Assuming the libc++ CI is green)


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

https://reviews.llvm.org/D141954

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D141954#4062029 , @philnik wrote:

> In D141954#4061962 , @erichkeane 
> wrote:
>
>> Speaking of: I guess the libcxx failure is NOT caused by me?  
>> https://reviews.llvm.org/D141992
>>
>> I see this on a different review, despite this not being committed.
>
> That's not great. Seems like some recently landed patch broke this. Do you 
> mind committing the libc++-part of this patch to get the Clang CI green 
> again? (Assuming the libc++ CI is green)

If this passes, I might just do the module-map patch as a separate commit to 
fix the CI, else the libcxx history is going to be awful strange :)   I'm 
probably a bit from having this review approved anyway, as it is still 'night 
time' for most of my reviewers :)


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

https://reviews.llvm.org/D141954

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

cfe-commits@lists.llvm.org

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:181
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+

how does this differentiate from 

`MyType & val2;`

is that a binary operator? I don't think so?


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

https://reviews.llvm.org/D141959

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

[PATCH] D142014: [clangd] fix wrong CalleeArgInfo in the hover

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry created this revision.
v1nh1shungry added reviewers: tom-anders, nridge.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
v1nh1shungry requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

  void foobar(int);
  int main() {
foobar(1 + 2);
 ^
  }

Currently the CalleeArgInfo will be "Passed by reference", which should
be "Passed by value".

Fixes https://github.com/clangd/clangd/issues/1467


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142014

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

Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -934,6 +934,23 @@
  HI.CalleeArgInfo->Type = "const int &";
  HI.CallPassType = HoverInfo::PassType{PassMode::ConstRef, false};
}},
+  {
+  R"cpp(
+int add(int lhs, int rhs);
+int main() {
+  add(1 [[^+]] 2, 3);
+}
+)cpp",
+  [](HoverInfo &HI) {
+HI.Name = "expression";
+HI.Kind = index::SymbolKind::Unknown;
+HI.Type = "int";
+HI.Value = "3";
+HI.CalleeArgInfo.emplace();
+HI.CalleeArgInfo->Name = "lhs";
+HI.CalleeArgInfo->Type = "int";
+HI.CallPassType = HoverInfo::PassType{PassMode::Value, false};
+  }},
   {// Extra info for method call.
R"cpp(
   class C {
@@ -1673,8 +1690,8 @@
   }},
   {
   R"cpp(// Function definition via using declaration
-namespace ns { 
-  void foo(); 
+namespace ns {
+  void foo();
 }
 int main() {
   using ns::foo;
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -952,6 +952,15 @@
   }
 }
 
+HoverInfo::PassType::PassMode getPassMode(QualType QT) {
+  if (QT->isReferenceType()) {
+if (QT->getPointeeType().isConstQualified())
+  return HoverInfo::PassType::ConstRef;
+return HoverInfo::PassType::Ref;
+  }
+  return HoverInfo::PassType::Value;
+}
+
 // If N is passed as argument to a function, fill HI.CalleeArgInfo with
 // information about that argument.
 void maybeAddCalleeArgInfo(const SelectionTree::Node *N, HoverInfo &HI,
@@ -972,14 +981,18 @@
   if (!FD || FD->isOverloadedOperator() || FD->isVariadic())
 return;
 
+  QualType ParmType;
+
   // Find argument index for N.
   for (unsigned I = 0; I < CE->getNumArgs() && I < FD->getNumParams(); ++I) {
 if (CE->getArg(I) != OuterNode.ASTNode.get())
   continue;
 
 // Extract matching argument from function declaration.
-if (const ParmVarDecl *PVD = FD->getParamDecl(I))
+if (const ParmVarDecl *PVD = FD->getParamDecl(I)) {
   HI.CalleeArgInfo.emplace(toHoverInfoParam(PVD, PP));
+  ParmType = PVD->getType();
+}
 break;
   }
   if (!HI.CalleeArgInfo)
@@ -989,15 +1002,7 @@
   // [const-]reference. For this we need to walk up the AST from the arg itself
   // to CallExpr and check all implicit casts, constructor calls, etc.
   HoverInfo::PassType PassType;
-  if (const auto *E = N->ASTNode.get()) {
-if (E->getType().isConstQualified())
-  PassType.PassBy = HoverInfo::PassType::ConstRef;
-
-// No implicit node, literal passed by value
-if (isLiteral(E) && N->Parent == OuterNode.Parent)
-  PassType.PassBy = HoverInfo::PassType::Value;
-  }
-
+  PassType.PassBy = getPassMode(ParmType);
   for (auto *CastNode = N->Parent;
CastNode != OuterNode.Parent && !PassType.Converted;
CastNode = CastNode->Parent) {
@@ -1006,22 +1011,13 @@
   case CK_NoOp:
   case CK_DerivedToBase:
   case CK_UncheckedDerivedToBase:
-// If it was a reference before, it's still a reference.
-if (PassType.PassBy != HoverInfo::PassType::Value)
-  PassType.PassBy = ImplicitCast->getType().isConstQualified()
-? HoverInfo::PassType::ConstRef
-: HoverInfo::PassType::Ref;
-break;
   case CK_LValueToRValue:
   case CK_ArrayToPointerDecay:
   case CK_FunctionToPointerDecay:
   case CK_NullToPointer:
   case CK_NullToMemberPointer:
-// No longer a reference, but we do not show this as type conversion.
-PassType.PassBy = HoverInfo::PassType::Value;
 break;
   default:
-PassType.PassBy = HoverInfo::PassType::Value;
 PassType.Converted = true;
 break;
   }
@@ -1029,16 +1025,16 @@
CastNode->ASTNode.get()) {
   // We want t

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ok, so. I looked really hard, and essentially i'm not sure we can just change 
those attributes from implying `readonly`/`readnone`.
Things kinda just fall apart




Comment at: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl:20
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone 
attribute.
+// FIXME: we don't, though.
 // CHECK-LABEL: @test_const_attr

I've looked, and i really don't understand how D64319 works.
It seems like the AST is then serialized into a header?
Because just adding a new attribute without spelling does not solve the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958

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

cfe-commits@lists.llvm.org

[David K Turner via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Format/TokenAnnotatorTest.cpp:181
+  ASSERT_EQ(Tokens.size(), 5u) << Tokens;
+  EXPECT_TOKEN(Tokens[1], tok::amp, TT_BinaryOperator);
+

MyDeveloperDay wrote:
> how does this differentiate from 
> 
> `MyType & val2;`
> 
> is that a binary operator? I don't think so?
The heuristic I'm using is that [symbol] & [symbol]; outside a class/struct 
declaration is more likely an instance of operator& than an uninitialized 
reference.  Tests on line 206 and 215 demonstrate how the two cases differ.


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

https://reviews.llvm.org/D141959

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

[PATCH] D141929: Add support for clang-cl's option /fexcess-precision.

[Michael D Toguchi via Phabricator via cfe-commits]

mdtoguchi added inline comments.



Comment at: clang/include/clang/Driver/Options.td:6654
+def _SLASH_fexcess_precision_EQ : CLJoined<"fexcess-precision=">,
+  Alias;
+

To expose to clang-cl, can we just make the original option a `CoreOption` 
instead of adding an alias of the same name here?


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

https://reviews.llvm.org/D141929

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

[PATCH] D142016: [Clang][RISCV] Simplify RVV intrinsic policy suffix

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

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

This patch works towards the simplification proposal [0] of Nick
Knight. After this patch, we have reduced the hierarchy of intrinsics
from two sets (non-policy and policy) into a single set, with a general
assumption that policy behavior is agnostic unless specified.

[0] https://gist.github.com/nick-knight/6cb0b74b351a25323dfb1821d3a269b9

Pull Request: riscv-non-isa/rvv-intrinsic-doc#186.

Depends on D141796 .`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142016

Files:
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  clang/lib/Support/RISCVVIntrinsicUtils.cpp
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vaaddu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vadc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vand.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vasubu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vcompress.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vdivu.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfabs.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfclass.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfcvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfmadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfmax.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfmerge.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfmin.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfmsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfmv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfncvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfneg.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfnmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfnmadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfnmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfnmsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfrdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfrec7.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfredmax.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfredmin.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfredosum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfredusum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfrsqrt7.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfrsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfsgnj.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfsgnjn.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfsgnjx.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-overloaded/vfslide1down.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/policy/non-over

[PATCH] D140875: [clangd] prototype: Implement unused include warnings with include-cleaner library.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 490137.
hokein marked 4 inline comments as done.
hokein added a comment.

address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140875

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
  clang-tools-extra/include-cleaner/lib/Record.cpp

Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -150,7 +150,7 @@
   RecordPragma(const CompilerInstance &CI, PragmaIncludes *Out)
   : SM(CI.getSourceManager()),
 HeaderInfo(CI.getPreprocessor().getHeaderSearchInfo()), Out(Out),
-UniqueStrings(Arena) {}
+UniqueStrings(Out->Arena) {}
 
   void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -176,7 +176,6 @@
   std::unique(It.getSecond().begin(), It.getSecond().end()),
   It.getSecond().end());
 }
-Out->Arena = std::move(Arena);
   }
 
   void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok,
@@ -307,7 +306,6 @@
   const SourceManager &SM;
   HeaderSearch &HeaderInfo;
   PragmaIncludes *Out;
-  llvm::BumpPtrAllocator Arena;
   /// Intern table for strings. Contents are on the arena.
   llvm::StringSaver UniqueStrings;
 
@@ -336,6 +334,27 @@
   std::vector KeepStack;
 };
 
+PragmaIncludes::PragmaIncludes(const PragmaIncludes & In) {
+  *this = In;
+}
+
+PragmaIncludes &PragmaIncludes::operator=(const PragmaIncludes &In) {
+  ShouldKeep = In.ShouldKeep;
+  NonSelfContainedFiles = In.NonSelfContainedFiles;
+  Arena.Reset();
+  llvm::UniqueStringSaver Strings(Arena);
+  IWYUPublic.clear();
+  for (const auto& It : In.IWYUPublic)
+IWYUPublic.try_emplace(It.first, Strings.save(It.second));
+  IWYUExportBy.clear();
+  for (const auto& UIDAndExporters : In.IWYUExportBy) {
+const auto& [UID, Exporters] = UIDAndExporters;
+for (StringRef ExporterHeader : Exporters)
+   IWYUExportBy.try_emplace(UID).first->second.push_back(ExporterHeader);
+  }
+  return *this;
+ }
+
 void PragmaIncludes::record(const CompilerInstance &CI) {
   auto Record = std::make_unique(CI, this);
   CI.getPreprocessor().addCommentHandler(Record.get());
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
===
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -48,6 +48,13 @@
 /// defines the symbol.
 class PragmaIncludes {
 public:
+  PragmaIncludes() = default;
+  PragmaIncludes(PragmaIncludes &&) = default;
+  PragmaIncludes &operator=(PragmaIncludes &&) = default;
+
+  PragmaIncludes(const PragmaIncludes &);
+  PragmaIncludes &operator=(const PragmaIncludes &);
+
   /// Installs an analysing PPCallback and CommentHandler and populates results
   /// to the structure.
   void record(const CompilerInstance &CI);
Index: clang-tools-extra/clangd/Preamble.h
===
--- clang-tools-extra/clangd/Preamble.h
+++ clang-tools-extra/clangd/Preamble.h
@@ -27,6 +27,7 @@
 #include "Diagnostics.h"
 #include "FS.h"
 #include "Headers.h"
+#include "clang-include-cleaner/Record.h"
 #include "index/CanonicalIncludes.h"
 #include "support/Path.h"
 #include "clang/Frontend/CompilerInvocation.h"
@@ -57,6 +58,8 @@
   // Processes like code completions and go-to-definitions will need #include
   // information, and their compile action skips preamble range.
   IncludeStructure Includes;
+
+  include_cleaner::PragmaIncludes Pragmas;
   // Macros defined in the preamble section of the main file.
   // Users care about headers vs main-file, not preamble vs non-preamble.
   // These should be treated as main-file entities e.g. for code completion.
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -11,6 +11,7 @@
 #include "Config.h"
 #include "Headers.h"
 #include "SourceCode.h"
+#include "clang-include-cleaner/Record.h"
 #include "support/Logger.h"
 #include "support/ThreadsafeFS.h"
 #include "support/Trace.h"
@@ -77,6 +78,9 @@
 
   std::vector takeMarks() { return std::move(Marks); }
 
+  include_cleaner::PragmaIncludes takePragmaIncludes() {
+return std::move(Pragmas

[PATCH] D140875: [clangd] prototype: Implement unused include warnings with include-cleaner library.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:471
+
+// FIXME: this map should probably be in IncludeStructure.
+llvm::StringMap> BySpelling;

kadircet wrote:
> agreed, let's change `StdlibHeaders` in `IncludeStructure` to actually be a 
> `BySpelling` mapping, it should be no-op for existing implementation as we're 
> only checking for stdlib headers already (and there's no other usage)
sure, I will address it in a followup patch.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:479
+}
+// FIXME: !!this is a hacky way to collect macro references.
+std::vector Macros;

kadircet wrote:
> this might behave slightly different than what we have in `RecordedPP`, and 
> rest of the applications of include-cleaner will be using that. can we expose 
> the pieces in RecordedPP that collects MacroRefs as a separate handler and 
> attach that collector (combined with the skipped range collection inside 
> `CollectMainFileMacros` and also still converting to `MainFileMacros` in the 
> end (as we can't store sourcelocations/identifierinfos from preamble)?
> 
> afterwards we can use the names stored in there to get back to 
> `IdentifierInfo`s and Ranges stored to get back to `SourceLocation`s. WDYT?
Yeah, this is a better solution, but I'm not sure whether we should do this 
before the release cut, it has a side effect of changing the 
find-macro-reference behavior in clangd. It requires some design/implement work.

I agree that the current solution is hacky, and eventually will be replaced, 
but it follows the existing `findReferencedMacros`, so it is not that bad. I 
tend to land this version before the release cut. What do you think?



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:487
+Macros.push_back({Tok.location(),
+  include_cleaner::Macro{/*Name=*/nullptr, DefLoc},
+  include_cleaner::RefType::Explicit});

kadircet wrote:
> you can get the `IdentifierInfo` using `Name` inside `Macro` and PP.
oh, right. Done.



Comment at: clang-tools-extra/clangd/ParsedAST.cpp:609
+  Config::UnusedIncludesPolicy::Experiment)
+Pragmas.record(*Clang);
   // Copy over the macros in the preamble region of the main file, and combine

kadircet wrote:
> why do we need to collect pragmas in main file? i think we already have 
> necessary information available via `IncludeStructure` (it stores keeps and 
> exports, and we don't care about anything else in the main file AFAICT). so 
> let's just use the PragmaIncludes we're getting from the Preamble directly? 
> without even making a copy and returning a reference from the `Preamble` 
> instead in `ParsedAST::getPragmaIncludes`
> i think we already have necessary information available via IncludeStructure 
> (it stores keeps and exports, and we don't care about anything else in the 
> main file AFAICT)

The IncludeStructure doesn't have a full support for IWYU export pragma, it 
only tracks the headers that have the export pragma.

My understand of the end goal is to use the `PragmaInclude` to handle every 
IWYU-related things, and we can remove all these IWYU bits in the 
`IncludeStructure`,  clangd IWYU pragma handling 
[code](https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clangd/Headers.cpp#L127-L166).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140875

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

[PATCH] D142016: [Clang][RISCV] Simplify RVV intrinsic policy suffix

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

eopXD added inline comments.



Comment at: clang/lib/Support/RISCVVIntrinsicUtils.cpp:875
 
-  updateNamesAndPolicy(IsMasked, hasPolicy(), HasTailPolicy, HasMaskPolicy,
-   Name, BuiltinName, OverloadedName, PolicyAttrs);

@kito-cheng I think the removal of `HasTailPolicy` and `HasMaskPolicy` here and 
under `SemaRISCVVectorLookup.cpp`, `RISCVVEmitter.cpp` should resolve your 
concern on these two attributes blending into the `Policy` object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142016

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

[PATCH] D142007: [NFC] Fix "form/from" typos

[Louis Dionne via Phabricator via cfe-commits]

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

LGTM, thanks! I don't think you need to wait for other owners to approve before 
landing this, this is pretty clearly an improvement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142007

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

[PATCH] D142022: [Clang][OpenMP] Fix handling of -mcode-object-version for OpenMP

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam created this revision.
saiislam added reviewers: jhuber6, yaxunl.
Herald added subscribers: kosarev, kerbowa, guansong, tpr, jvesely.
Herald added a project: All.
saiislam requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a reviewer: jdoerfert.
Herald added a project: clang.

Code object version flag for AMDGPU was not being honored by the
driver. It was required to be passed as derived arg so that correct
bitcode library can be linked.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142022

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
  clang/lib/Driver/ToolChains/Clang.cpp


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7113,7 +7113,7 @@
   }
 
   if (Triple.isAMDGPU()) {
-handleAMDGPUCodeObjectVersionOptions(D, Args, CmdArgs);
+handleAMDGPUCodeObjectVersionOptions(D, C.getArgs(), CmdArgs);
 
 Args.addOptInFlag(CmdArgs, options::OPT_munsafe_fp_atomics,
   options::OPT_mno_unsafe_fp_atomics);
@@ -8087,7 +8087,8 @@
   }
 
   if (Triple.isAMDGPU())
-handleAMDGPUCodeObjectVersionOptions(D, Args, CmdArgs, /*IsCC1As=*/true);
+handleAMDGPUCodeObjectVersionOptions(D, C.getArgs(), CmdArgs,
+ /*IsCC1As=*/true);
 
   assert(Input.isFilename() && "Invalid input.");
   CmdArgs.push_back(Input.getFilename());
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
@@ -26,8 +26,8 @@
 : public ROCMToolChain {
 public:
   AMDGPUOpenMPToolChain(const Driver &D, const llvm::Triple &Triple,
-const ToolChain &HostTC,
-const llvm::opt::ArgList &Args);
+const ToolChain &HostTC, const llvm::opt::ArgList 
&Args,
+const llvm::opt::DerivedArgList &DerivedArgs);
 
   const llvm::Triple *getAuxTriple() const override {
 return &HostTC.getTriple();
@@ -58,6 +58,7 @@
   getDeviceLibs(const llvm::opt::ArgList &Args) const override;
 
   const ToolChain &HostTC;
+  const llvm::opt::DerivedArgList &DerivedArgs;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -58,8 +58,9 @@
 AMDGPUOpenMPToolChain::AMDGPUOpenMPToolChain(const Driver &D,
  const llvm::Triple &Triple,
  const ToolChain &HostTC,
- const ArgList &Args)
-: ROCMToolChain(D, Triple, Args), HostTC(HostTC) {
+ const ArgList &Args,
+ const DerivedArgList &DerivedArgs)
+: ROCMToolChain(D, Triple, Args), HostTC(HostTC), DerivedArgs(DerivedArgs) 
{
   // Lookup binaries into the driver directory, this is used to
   // discover the clang-offload-bundler executable.
   getProgramPaths().push_back(getDriver().Dir);
@@ -190,7 +191,7 @@
   getTriple(), Args.getLastArgValue(options::OPT_march_EQ));
 
   SmallVector BCLibs;
-  for (auto BCLib : getCommonDeviceLibNames(Args, GpuArch.str(),
+  for (auto BCLib : getCommonDeviceLibNames(DerivedArgs, GpuArch.str(),
 /*IsOpenMP=*/true))
 BCLibs.emplace_back(BCLib);
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -872,7 +872,7 @@
   }
   if (AMDTriple) {
 auto TempTC = std::make_unique(
-*this, *AMDTriple, *HostTC, C.getInputArgs());
+*this, *AMDTriple, *HostTC, C.getInputArgs(), C.getArgs());
 for (StringRef Arch : getOffloadArchs(
  C, C.getArgs(), Action::OFK_OpenMP, &*TempTC, true))
   Archs.insert(Arch);
@@ -943,7 +943,7 @@
   *this, TT, *HostTC, C.getInputArgs());
 else if (TT.isAMDGCN())
   DeviceTC = std::make_unique(
-  *this, TT, *HostTC, C.getInputArgs());
+  *this, TT, *HostTC, C.getInputArgs(), C.getArgs());
 else
   assert(DeviceTC && "Device toolchain not defined.");
   }


Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -7113,7 +7113,7 @@
   }
 
   if (Triple.

[PATCH] D142007: [NFC] Fix "form/from" typos

[Piotr Fusik via Phabricator via cfe-commits]

pfusik added a comment.

In D142007#4062211 , @ldionne wrote:

> LGTM, thanks! I don't think you need to wait for other owners to approve 
> before landing this, this is pretty clearly an improvement.

I have no commit access, could you please commit this for me?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142007

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

[PATCH] D139730: [OpenMP][DeviceRTL][AMDGPU] Support code object version 5

[Saiyedul Islam via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7085
   if (Triple.isAMDGPU()) {
-handleAMDGPUCodeObjectVersionOptions(D, Args, CmdArgs);
+handleAMDGPUCodeObjectVersionOptions(D, C.getArgs(), CmdArgs,
+ /*IsCC1As=*/true);

yaxunl wrote:
> Any reason you need the original args? This will bypass the driver 
> translation, which should not in normal cases.
We need derived args to look for mcode-object-version. I have created a 
separate review for this change. Please have a look at D142022


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139730

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

[PATCH] D141961: [clang][lex] Pass hash location to more PPCallbacks methods

[Kyle Edwards via Phabricator via cfe-commits]

KyleFromKitware updated this revision to Diff 490160.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141961

Files:
  clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllMacros.cpp
  clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllMacros.h
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h
  clang-tools-extra/clang-tidy/bugprone/MacroParenthesesCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.cpp
  clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/clang-tidy/readability/DuplicateIncludeCheck.cpp
  clang-tools-extra/clang-tidy/readability/ElseAfterReturnCheck.cpp
  clang-tools-extra/clang-tidy/readability/RedundantPreprocessorCheck.cpp
  clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clangd/CollectMacros.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/include-cleaner/lib/Record.cpp
  clang-tools-extra/modularize/PreprocessorTracker.cpp
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/pp-trace/PPCallbacksTracker.h
  clang/include/clang/Lex/PPCallbacks.h
  clang/include/clang/Lex/PPConditionalDirectiveRecord.h
  clang/include/clang/Lex/PreprocessingRecord.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/CodeGen/MacroPPCallbacks.cpp
  clang/lib/CodeGen/MacroPPCallbacks.h
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/Frontend/ASTUnit.cpp
  clang/lib/Frontend/PrintPreprocessedOutput.cpp
  clang/lib/Frontend/Rewrite/InclusionRewriter.cpp
  clang/lib/Index/IndexingAction.cpp
  clang/lib/Lex/PPConditionalDirectiveRecord.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/PreprocessingRecord.cpp
  clang/tools/libclang/Indexing.cpp
  clang/unittests/Basic/SourceManagerTest.cpp
  clang/unittests/Lex/PPCallbacksTest.cpp

Index: clang/unittests/Lex/PPCallbacksTest.cpp
===
--- clang/unittests/Lex/PPCallbacksTest.cpp
+++ clang/unittests/Lex/PPCallbacksTest.cpp
@@ -75,13 +75,15 @@
 
   std::vector Results;
 
-  void If(SourceLocation Loc, SourceRange ConditionRange,
+  void If(SourceLocation HashLoc, SourceLocation Loc,
+  SourceRange ConditionRange,
   ConditionValueKind ConditionValue) override {
 Results.emplace_back(ConditionRange, ConditionValue);
   }
 
-  void Elif(SourceLocation Loc, SourceRange ConditionRange,
-ConditionValueKind ConditionValue, SourceLocation IfLoc) override {
+  void Elif(SourceLocation HashLoc, SourceLocation Loc,
+SourceRange ConditionRange, ConditionValueKind ConditionValue,
+SourceLocation IfLoc) override {
 Results.emplace_back(ConditionRange, ConditionValue);
   }
 };
Index: clang/unittests/Basic/SourceManagerTest.cpp
===
--- clang/unittests/Basic/SourceManagerTest.cpp
+++ clang/unittests/Basic/SourceManagerTest.cpp
@@ -510,15 +510,15 @@
 public:
   explicit MacroTracker(std::vector &Macros) : Macros(Macros) { }
 
-  void MacroDefined(const Token &MacroNameTok,
+  void MacroDefined(SourceLocation HashLoc, const Token &MacroNameTok,
 const MacroDirective *MD) override {
 Macros.push_back(MacroAction(MD->getLocation(),
  MacroNameTok.getIdentifierInfo()->getName(),
  MacroAction::kDefinition));
   }
-  void MacroUndefined(const Token &MacroNameTok,
+  void MacroUndefined(SourceLocation HashLoc, const Token &MacroNameTok,
   const MacroDefinition &MD,
-  const MacroDirective  *UD) override {
+  const MacroDirective *UD) override {
 Macros.push_back(
 MacroAction(UD ? UD->getLocation() : SourceLocation(),
 MacroNameTok.getIdentifierInfo()->getName(),
Index: clang/tools/libclang/Indexing.cpp
===
--- clang/tools/libclang/Indexing.cpp
+++ clang/tools/libclang/Indexing.cpp
@@ -272,11 +272,12 @@
   }
 
   /// MacroDefined - This hook is called whenever a macro definition is seen.
-  void MacroDefined(const Token &Id, const MacroDirective *MD) override {}
+  void MacroDefined(SourceLocation HashLoc, const Token &Id,
+const MacroDirective *MD) override {}
 
   /// MacroUndefined - This hook is called whenever a macro #undef is seen.
   /// MI is released immediately following this callback.
-  void MacroUndefined(const Token &MacroNameTok,
+  void MacroUndefined(SourceLocation HashLoc, const Token &MacroNameTok,
   const MacroDefinition &MD,
   const MacroDirective *U

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh added inline comments.



Comment at: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl:20
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone 
attribute.
+// FIXME: we don't, though.
 // CHECK-LABEL: @test_const_attr

lebedev.ri wrote:
> I've looked, and i really don't understand how D64319 works.
> It seems like the AST is then serialized into a header?
> Because just adding a new attribute without spelling does not solve the issue.
We're not setting the `NoUnwind` attribute for OpenCL (yet).  The following 
quick-and-dirty patch appears to fix this test for your patch (but will cause 
other tests to fail).  If you think it's time to add `NoUnwind` now, I can try 
putting up a review for that.

```
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 276d91fa2758..1ea3c11fbe03 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -1972,7 +1972,7 @@ void 
CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
   // TODO: NoUnwind attribute should be added for other GPU modes OpenCL, HIP,
   // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
   // code.
-  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
+  if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) || 
getLangOpts().OpenCL) {
 // Exceptions aren't supported in CUDA device code.
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958

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

[PATCH] D138958: [clang] Better UX for Clang’s unwind-affecting attributes

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.



Comment at: clang/test/CodeGenOpenCL/fdeclare-opencl-builtins.cl:20
 // Test that Attr.Const from OpenCLBuiltins.td is lowered to a readnone 
attribute.
+// FIXME: we don't, though.
 // CHECK-LABEL: @test_const_attr

svenvh wrote:
> lebedev.ri wrote:
> > I've looked, and i really don't understand how D64319 works.
> > It seems like the AST is then serialized into a header?
> > Because just adding a new attribute without spelling does not solve the 
> > issue.
> We're not setting the `NoUnwind` attribute for OpenCL (yet).  The following 
> quick-and-dirty patch appears to fix this test for your patch (but will cause 
> other tests to fail).  If you think it's time to add `NoUnwind` now, I can 
> try putting up a review for that.
> 
> ```
> diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
> index 276d91fa2758..1ea3c11fbe03 100644
> --- a/clang/lib/CodeGen/CGCall.cpp
> +++ b/clang/lib/CodeGen/CGCall.cpp
> @@ -1972,7 +1972,7 @@ void 
> CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
>// TODO: NoUnwind attribute should be added for other GPU modes OpenCL, 
> HIP,
>// SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
>// code.
> -  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
> +  if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) || 
> getLangOpts().OpenCL) {
>  // Exceptions aren't supported in CUDA device code.
>  FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
>}
> ```
Yeah, that would be my guess as to how you'd fix this it, yes.
I'd suggest posting such a patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138958

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

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rG865094746e2a: [clang][Interp] Track initialization state of 
local variables (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135750

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeStmtGen.cpp
  clang/lib/AST/Interp/Context.cpp
  clang/lib/AST/Interp/Descriptor.cpp
  clang/lib/AST/Interp/Descriptor.h
  clang/lib/AST/Interp/EvalEmitter.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBlock.h
  clang/lib/AST/Interp/InterpFrame.cpp
  clang/lib/AST/Interp/InterpFrame.h
  clang/lib/AST/Interp/Pointer.cpp
  clang/lib/AST/Interp/Pointer.h
  clang/lib/AST/Interp/Program.cpp
  clang/lib/AST/Interp/Program.h
  clang/test/AST/Interp/cxx20.cpp
  clang/test/AST/Interp/literals.cpp
  clang/test/AST/Interp/loops.cpp

Index: clang/test/AST/Interp/loops.cpp
===
--- clang/test/AST/Interp/loops.cpp
+++ clang/test/AST/Interp/loops.cpp
@@ -5,6 +5,7 @@
 
 // ref-no-diagnostics
 // expected-no-diagnostics
+// expected-cpp20-no-diagnostics
 
 namespace WhileLoop {
   constexpr int f() {
@@ -165,8 +166,6 @@
   static_assert(f5(true) == 8, "");
   static_assert(f5(false) == 5, "");
 
-  /// FIXME: This should be accepted in C++20 but is currently being rejected
-  ///   because the variable declaration doesn't have an initializier.
 #if __cplusplus >= 202002L
   constexpr int f6() {
 int i;
@@ -176,7 +175,7 @@
 } while (true);
 return i;
   }
-  static_assert(f6() == 5, ""); // expected-cpp20-error {{not an integral constant}}
+  static_assert(f6() == 5, "");
 #endif
 
 #if 0
Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -407,8 +407,7 @@
 return 1;
   }
   static_assert(uninit(), ""); // ref-error {{not an integral constant expression}} \
-   // ref-note {{in call to 'uninit()'}} \
-   // expected-error {{not an integral constant expression}}
+   // ref-note {{in call to 'uninit()'}}
 
   constexpr int OverFlow() { // ref-error {{never produces a constant expression}}
 int a = INT_MAX;
Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -56,33 +56,51 @@
 }
 static_assert(pointerAssign2() == 12, "");
 
-
 constexpr int unInitLocal() {
   int a;
-  return a; // ref-note{{read of uninitialized object}}
+  return a; // ref-note {{read of uninitialized object}} \
+// expected-note {{read of object outside its lifetime}}
+// FIXME: ^^^ Wrong diagnostic.
 }
-static_assert(unInitLocal() == 0, ""); // expected-error {{not an integral constant expression}} \
-   // ref-error {{not an integral constant expression}} \
-   // ref-note {{in call to 'unInitLocal()'}}
-
-/// TODO: The example above is correctly rejected by the new constexpr
-///   interpreter, but for the wrong reasons. We don't reject it because
-///   it is an uninitialized read, we reject it simply because
-///   the local variable does not have an initializer.
-///
-///   The code below should be accepted but is also being rejected
-///   right now.
-#if 0
+static_assert(unInitLocal() == 0, ""); // ref-error {{not an integral constant expression}} \
+   // ref-note {{in call to 'unInitLocal()'}} \
+   // expected-error {{not an integral constant expression}} \
+   // expected-note {{in call to 'unInitLocal()'}} \
+
 constexpr int initializedLocal() {
   int a;
-  int b;
-
   a = 20;
   return a;
 }
 static_assert(initializedLocal() == 20);
 
-/// Similar here, but the uninitialized local is passed as a function parameter.
+constexpr int initializedLocal2() {
+  int a[2];
+  return *a; // expected-note {{read of object outside its lifetime}} \
+ // ref-note {{read of uninitialized object is not allowed in a constant expression}}
+}
+static_assert(initializedLocal2() == 20); // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to}} \
+  // ref-error {{not an integral constant expression}} \
+  // ref-note {{in call to}}
+
+
+struct Int { int a; };
+constexpr int initializedLocal3() {
+  Int i;
+  return i.a; // expected-note {{read of object outside its lifetime}} \
+  // ref-note {{read of uninitialized object is not all

[PATCH] D141862: [clang][driver][AIX] Add OpenMP runtime if -fopenmp specified

[Xing Xue via Phabricator via cfe-commits]

xingxue updated this revision to Diff 490167.
xingxue added a comment.

Addressed comments.

- added test scenarios for option `fopenmp` in `clang/test/Driver/aix-ld.c`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141862

Files:
  clang/lib/Driver/ToolChains/AIX.cpp
  clang/test/Driver/aix-ld.c

Index: clang/test/Driver/aix-ld.c
===
--- clang/test/Driver/aix-ld.c
+++ clang/test/Driver/aix-ld.c
@@ -1016,3 +1016,76 @@
 // CHECK-LD64-SHARED-EXPFULL: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc64.a"
 // CHECK-LD64-SHARED-EXPFULL: "-lm"
 // CHECK-LD64-SHARED-EXPFULL: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0. -fopenmp to use default OpenMP runtime libomp.
+// RUN: %clang %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-fopenmp \
+// RUN:   | FileCheck --check-prefix=CHECK-FOPENMP-OMP %s
+// CHECK-FOPENMP-OMP-NOT: warning:
+// CHECK-FOPENMP-OMP: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-FOPENMP-OMP: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-FOPENMP-OMP: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-FOPENMP-OMP: "{{.*}}ld{{(.exe)?}}"
+// CHECK-FOPENMP-OMP-NOT: "-bnso"
+// CHECK-FOPENMP-OMP: "-b32"
+// CHECK-FOPENMP-OMP: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-FOPENMP-OMP: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-FOPENMP-OMP: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-FOPENMP-OMP-NOT: "-lc++"
+// CHECK-FOPENMP-OMP-NOT: "-lc++abi"
+// CHECK-FOPENMP-OMP: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-FOPENMP-OMP-NOT: "--as-needed"
+// CHECK-FOPENMP-OMP: "-lunwind"
+// CHECK-FOPENMP-OMP-NOT: "--no-as-needed"
+// CHECK-FOPENMP-OMP-NOT: "-lm"
+// CHECK-FOPENMP-OMP: "-lomp"
+// CHECK-FOPENMP-OMP: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0. -fopenmp=libomp to specify libomp explicitly.
+// RUN: %clang %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-fopenmp=libomp \
+// RUN:   | FileCheck --check-prefix=CHECK-FOPENMP-OMP %s
+
+// Check powerpc-ibm-aix7.1.0.0. -fopenmp=libgomp to specify libgomp explicitly.
+// RUN: %clang %s -### 2>&1 \
+// RUN:-resource-dir=%S/Inputs/resource_dir \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:--unwindlib=libunwind \
+// RUN:-fopenmp=libgomp \
+// RUN:   | FileCheck --check-prefix=CHECK-FOPENMP-GOMP %s
+// CHECK-FOPENMP-GOMP-NOT: warning:
+// CHECK-FOPENMP-GOMP: "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0"
+// CHECK-FOPENMP-GOMP: "-resource-dir" "[[RESOURCE_DIR:[^"]+]]"
+// CHECK-FOPENMP-GOMP: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-FOPENMP-GOMP: "{{.*}}ld{{(.exe)?}}"
+// CHECK-FOPENMP-GOMP-NOT: "-bnso"
+// CHECK-FOPENMP-GOMP: "-b32"
+// CHECK-FOPENMP-GOMP: "-bpT:0x1000" "-bpD:0x2000"
+// CHECK-FOPENMP-GOMP: "[[SYSROOT]]/usr/lib{{/|}}crt0.o"
+// CHECK-FOPENMP-GOMP: "[[SYSROOT]]/usr/lib{{/|}}crti.o"
+// CHECK-FOPENMP-GOMP-NOT: "-lc++"
+// CHECK-FOPENMP-GOMP-NOT: "-lc++abi"
+// CHECK-FOPENMP-GOMP: "[[RESOURCE_DIR]]{{/|}}lib{{/|}}aix{{/|}}libclang_rt.builtins-powerpc.a"
+// CHECK-FOPENMP-GOMP-NOT: "--as-needed"
+// CHECK-FOPENMP-GOMP: "-lunwind"
+// CHECK-FOPENMP-GOMP-NOT: "--no-as-needed"
+// CHECK-FOPENMP-GOMP-NOT: "-lm"
+// CHECK-FOPENMP-GOMP: "-lgomp"
+// CHECK-FOPENMP-GOMP: "-lc"
+
+// Check powerpc-ibm-aix7.1.0.0, 32-bit. -fopenmp=libfoo results an error.
+// RUN: %clang %s 2>&1 -### \
+// RUN:--target=powerpc-ibm-aix7.1.0.0 \
+// RUN:--sysroot %S/Inputs/aix_ppc_tree \
+// RUN:-fopenmp=libfoo \
+// RUN:   | FileCheck --check-prefix=CHECK-FOPENMP-FOO %s
+// CHECK-FOPENMP-FOO: error: unsupported argument 'libfoo' to option '-fopenmp='
Index: clang/lib/Driver/ToolChains/AIX.cpp
===
--- clang/lib/Driver/ToolChains/AIX.cpp
+++ clang/lib/Driver/ToolChains/AIX.cpp
@@ -243,6 +243,25 @@
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
 AddRunTimeLibs(ToolChain, D, CmdArgs, Args);
 
+// Add OpenMP runtime if -fopenmp is specified.
+if (Args.hasFlag(options::OPT_fopenmp, options::OPT_fopenmp_EQ,
+ options::OPT_fno_openmp, false)) {
+  switch (ToolChain.getDriver().getOpenMPRuntime(Args)) {
+  case Driver::OMPRT_OMP:
+CmdArgs.push_back("-lomp");
+break;
+  case Driver::OMPRT_IOMP5:
+Cmd

[clang] 8650947 - [clang][Interp] Track initialization state of local variables

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-01-18T16:55:02+01:00
New Revision: 865094746e2ad759aee52df2a291420d4fdd1e02

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

LOG: [clang][Interp] Track initialization state of local variables

Use an InlineDescriptor per local variable to track whether locals
have been initialized or not. This way we can support uninitialized
local variables in constexpr functions.

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

Added: 


Modified: 
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Context.cpp
clang/lib/AST/Interp/Descriptor.cpp
clang/lib/AST/Interp/Descriptor.h
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/Interp.h
clang/lib/AST/Interp/InterpBlock.h
clang/lib/AST/Interp/InterpFrame.cpp
clang/lib/AST/Interp/InterpFrame.h
clang/lib/AST/Interp/Pointer.cpp
clang/lib/AST/Interp/Pointer.h
clang/lib/AST/Interp/Program.cpp
clang/lib/AST/Interp/Program.h
clang/test/AST/Interp/cxx20.cpp
clang/test/AST/Interp/literals.cpp
clang/test/AST/Interp/loops.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 0bbab0cc06655..f4bf162a1454c 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -802,7 +802,11 @@ unsigned 
ByteCodeExprGen::allocateLocalPrimitive(DeclTy &&Src,
   PrimType Ty,
   bool IsConst,
   bool IsExtended) {
-  Descriptor *D = P.createDescriptor(Src, Ty, IsConst, Src.is());
+  // FIXME: There are cases where Src.is() is wrong, e.g.
+  //   (int){12} in C. Consider using Expr::isTemporaryObject() instead
+  //   or isa().
+  Descriptor *D = P.createDescriptor(Src, Ty, Descriptor::InlineDescMD, 
IsConst,
+ Src.is());
   Scope::Local Local = this->createLocal(D);
   if (auto *VD = dyn_cast_or_null(Src.dyn_cast()))
 Locals.insert({VD, Local});
@@ -831,7 +835,8 @@ ByteCodeExprGen::allocateLocal(DeclTy &&Src, bool 
IsExtended) {
   }
 
   Descriptor *D = P.createDescriptor(
-  Src, Ty.getTypePtr(), Ty.isConstQualified(), IsTemporary, false, Init);
+  Src, Ty.getTypePtr(), Descriptor::InlineDescMD, Ty.isConstQualified(),
+  IsTemporary, /*IsMutable=*/false, Init);
   if (!D)
 return {};
 

diff  --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp 
b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index 44997857f6a59..60506a759f45c 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -402,24 +402,26 @@ bool ByteCodeStmtGen::visitVarDecl(const VarDecl 
*VD) {
   if (std::optional T = this->classify(VD->getType())) {
 const Expr *Init = VD->getInit();
 
-if (!Init)
-  return false;
-
 unsigned Offset =
 this->allocateLocalPrimitive(VD, *T, VD->getType().isConstQualified());
 // Compile the initializer in its own scope.
-{
+if (Init) {
   ExprScope Scope(this);
   if (!this->visit(Init))
 return false;
+
+  return this->emitSetLocal(*T, Offset, VD);
 }
-// Set the value.
-return this->emitSetLocal(*T, Offset, VD);
+return true;
   }
 
   // Composite types - allocate storage and initialize it.
-  if (std::optional Offset = this->allocateLocal(VD))
+  if (std::optional Offset = this->allocateLocal(VD)) {
+if (!VD->getInit())
+  return true;
+
 return this->visitLocalInitializer(VD->getInit(), *Offset);
+  }
 
   return this->bail(VD);
 }

diff  --git a/clang/lib/AST/Interp/Context.cpp 
b/clang/lib/AST/Interp/Context.cpp
index 6fd645b87d22b..16471242f3282 100644
--- a/clang/lib/AST/Interp/Context.cpp
+++ b/clang/lib/AST/Interp/Context.cpp
@@ -125,7 +125,7 @@ unsigned Context::getCharBit() const {
 
 bool Context::Run(State &Parent, Function *Func, APValue &Result) {
   InterpState State(Parent, *P, Stk, *this);
-  State.Current = new InterpFrame(State, Func, nullptr, {}, {});
+  State.Current = new InterpFrame(State, Func, /*Caller=*/nullptr, {});
   if (Interpret(State, Result))
 return true;
   Stk.clear();

diff  --git a/clang/lib/AST/Interp/Descriptor.cpp 
b/clang/lib/AST/Interp/Descriptor.cpp
index f645063acdd01..5575fc1e2a6e7 100644
--- a/clang/lib/AST/Interp/Descriptor.cpp
+++ b/clang/lib/AST/Interp/Descriptor.cpp
@@ -191,19 +191,22 @@ static BlockMoveFn getMoveArrayPrim(PrimType Type) {
   COMPOSITE_TYPE_SWITCH(Type, return moveArrayTy, return nullptr);
 }
 
-Descriptor::Descriptor(const DeclTy &D, PrimType Type, bool IsConst,
-   b

[PATCH] D142026: Optimize OptTable::findNearest implementation and usage

[serge via Phabricator via cfe-commits]

serge-sans-paille created this revision.
serge-sans-paille added reviewers: aaron.ballman, nikic.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
serge-sans-paille requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

When used to find an exact match, some extra context can be used to
totally cut some computations.

This saves 1% of the instruction count when pre processing sqlite3.c
through

valgrind --tool=callgrind ./bin/clang -E sqlite3.c -o/dev/null


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142026

Files:
  clang/lib/Driver/Driver.cpp
  llvm/include/llvm/Option/OptTable.h
  llvm/lib/Option/OptTable.cpp


Index: llvm/lib/Option/OptTable.cpp
===
--- llvm/lib/Option/OptTable.cpp
+++ llvm/lib/Option/OptTable.cpp
@@ -227,12 +227,13 @@
 
 unsigned OptTable::findNearest(StringRef Option, std::string &NearestString,
unsigned FlagsToInclude, unsigned 
FlagsToExclude,
-   unsigned MinimumLength) const {
+   unsigned MinimumLength,
+   unsigned MinimumDistance) const {
   assert(!Option.empty());
 
   // Consider each [option prefix + option name] pair as a candidate, finding
   // the closest match.
-  unsigned BestDistance = UINT_MAX;
+  unsigned BestDistance = MinimumDistance + 1;
   SmallString<16> Candidate;
   SmallString<16> NormalizedName;
 
@@ -276,6 +277,14 @@
 // appropriate one. For example, if a user asks for "--helm", suggest
 // "--help" over "-help".
 for (auto CandidatePrefix : CandidateInfo.Prefixes) {
+  // If Candidate and NormalizedName have more than 'BestDistance'
+  // characters of difference, no need to compute the edit distance, it's
+  // going to be greater than BestDistance. Don't bother computing 
Candidate
+  // at all.
+  if (std::abs((signed)(CandidatePrefix.size() + CandidateName.size()) -
+   (signed)NormalizedName.size()) > (signed)BestDistance) {
+continue;
+  }
   Candidate = CandidatePrefix;
   Candidate += CandidateName;
   unsigned Distance = StringRef(Candidate).edit_distance(
Index: llvm/include/llvm/Option/OptTable.h
===
--- llvm/include/llvm/Option/OptTable.h
+++ llvm/include/llvm/Option/OptTable.h
@@ -175,11 +175,21 @@
   /// \param [in] MinimumLength - Don't find options shorter than this length.
   /// For example, a minimum length of 3 prevents "-x" from being considered
   /// near to "-S".
+  /// \param [in] MinimumDistance - Don't find options whose distance is 
greater
+  /// than this value.
   ///
   /// \return The edit distance of the nearest string found.
   unsigned findNearest(StringRef Option, std::string &NearestString,
unsigned FlagsToInclude = 0, unsigned FlagsToExclude = 
0,
-   unsigned MinimumLength = 4) const;
+   unsigned MinimumLength = 4,
+   unsigned MinimumDistance = UINT_MAX - 1) const;
+
+  bool findExact(StringRef Option, std::string &ExactString,
+ unsigned FlagsToInclude = 0,
+ unsigned FlagsToExclude = 0) const {
+return findNearest(Option, ExactString, FlagsToInclude, FlagsToExclude,
+   Option.size(), 0) == 0;
+  }
 
   /// Parse a single argument; returning the new argument and
   /// updating Index.
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -313,8 +313,8 @@
 std::string Nearest;
 if (getOpts().findNearest(ArgString, Nearest, IncludedFlagsBitmask,
   ExcludedFlagsBitmask) > 1) {
-  if (getOpts().findNearest(ArgString, Nearest, options::CC1Option) == 0 &&
-  !IsCLMode()) {
+  if (!IsCLMode() &&
+  getOpts().findExact(ArgString, Nearest, options::CC1Option)) {
 DiagID = diag::err_drv_unknown_argument_with_suggestion;
 Diags.Report(DiagID) << ArgString << "-Xclang " + Nearest;
   } else {
@@ -339,8 +339,8 @@
 // Warn on joined arguments that are similar to a long argument.
 std::string ArgString = ArgStrings[A->getIndex()];
 std::string Nearest;
-if (getOpts().findNearest("-" + ArgString, Nearest, IncludedFlagsBitmask,
-  ExcludedFlagsBitmask) == 0)
+if (getOpts().findExact("-" + ArgString, Nearest, IncludedFlagsBitmask,
+ExcludedFlagsBitmask))
   Diags.Report(diag::warn_drv_potentially_misspelled_joined_argument)
   << A->getAsString(Args) << Nearest;
   }
@@ -2472,7 +2472,7 @@
 getIncludeExcludeOptionFlagMasks(IsCLMode());
 std::string Nearest;
 

[PATCH] D142022: [Clang][OpenMP] Fix handling of -mcode-object-version for OpenMP

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7116
   if (Triple.isAMDGPU()) {
-handleAMDGPUCodeObjectVersionOptions(D, Args, CmdArgs);
+handleAMDGPUCodeObjectVersionOptions(D, C.getArgs(), CmdArgs);
 

why do you need to change Args to C.getArgs() ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142022

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

[PATCH] D142026: Optimize OptTable::findNearest implementation and usage

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:2475
 if (getOpts().findNearest(Value, Nearest, IncludedFlagsBitmask,
-  ExcludedFlagsBitmask) <= 1) {
+  ExcludedFlagsBitmask, 0, 1) <= 1) {
   Diag(clang::diag::err_drv_no_such_file_with_suggestion)

I think this changes MinimumLength from the default of 4 to 0?



Comment at: llvm/include/llvm/Option/OptTable.h:179
+  /// \param [in] MinimumDistance - Don't find options whose distance is 
greater
+  /// than this value.
   ///

Shouldn't this be MaximumDistance?



Comment at: llvm/lib/Option/OptTable.cpp:285
+  if (std::abs((signed)(CandidatePrefix.size() + CandidateName.size()) -
+   (signed)NormalizedName.size()) > (signed)BestDistance) {
+continue;

Should probably be `ssize_t` rather than `signed`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142026

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

[PATCH] D142028: [clang] remove SUBMODULE_TOPHEADER

[Richard Howell via Phabricator via cfe-commits]

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

We are seeing issues with path serialization of the
`SUBMODULE_TOPHEADER` field when the inputs are links. While working
on a fix I noticed this field does not appear to be used anywhere
outside of libclang. This diff removes the field entirely and 
updates the libclang API to always return empty top headers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142028

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/Basic/Module.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Index/annotate-module.m
  clang/test/Modules/relative-submodule-topheader.m
  clang/tools/libclang/CIndex.cpp

Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -8777,31 +8777,15 @@
 CXModule CXMod) {
   if (isNotUsableTU(TU)) {
 LOG_BAD_TU(TU);
-return 0;
   }
-  if (!CXMod)
-return 0;
-  Module *Mod = static_cast(CXMod);
-  FileManager &FileMgr = cxtu::getASTUnit(TU)->getFileManager();
-  ArrayRef TopHeaders = Mod->getTopHeaders(FileMgr);
-  return TopHeaders.size();
+  return 0;
 }
 
 CXFile clang_Module_getTopLevelHeader(CXTranslationUnit TU, CXModule CXMod,
   unsigned Index) {
   if (isNotUsableTU(TU)) {
 LOG_BAD_TU(TU);
-return nullptr;
   }
-  if (!CXMod)
-return nullptr;
-  Module *Mod = static_cast(CXMod);
-  FileManager &FileMgr = cxtu::getASTUnit(TU)->getFileManager();
-
-  ArrayRef TopHeaders = Mod->getTopHeaders(FileMgr);
-  if (Index < TopHeaders.size())
-return const_cast(TopHeaders[Index]);
-
   return nullptr;
 }
 
Index: clang/test/Modules/relative-submodule-topheader.m
===
--- clang/test/Modules/relative-submodule-topheader.m
+++ clang/test/Modules/relative-submodule-topheader.m
@@ -3,8 +3,5 @@
 // RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
 
 // CHECK:  blob data = 'vector.h'
-// CHECK:  blob data = 'vector.h'
 // CHECK:  blob data = 'type_traits.h'
-// CHECK:  blob data = 'type_traits.h'
 // CHECK:  blob data = 'hash_map.h'
-// CHECK:  blob data = 'hash_map.h'
Index: clang/test/Index/annotate-module.m
===
--- clang/test/Index/annotate-module.m
+++ clang/test/Index/annotate-module.m
@@ -46,6 +46,4 @@
 // RUN: c-index-test -cursor-at=%s:3:11 %s -fmodules-cache-path=%t.cache -fmodules -F %S/../Modules/Inputs \
 // RUN: | FileCheck %s -check-prefix=CHECK-CURSOR
 
-// CHECK-CURSOR:  3:1 ModuleImport=DependsOnModule:3:1 (Definition) Extent=[3:1 - 3:24] Spelling=DependsOnModule ([3:9 - 3:24]) ModuleName=DependsOnModule ({{.*}}DependsOnModule-{{[^.]*}}.pcm) system=0 Headers(2):
-// CHECK-CURSOR-NEXT: {{.*}}other.h
-// CHECK-CURSOR-NEXT: {{.*}}DependsOnModule.h
+// CHECK-CURSOR:  3:1 ModuleImport=DependsOnModule:3:1 (Definition) Extent=[3:1 - 3:24] Spelling=DependsOnModule ([3:9 - 3:24]) ModuleName=DependsOnModule ({{.*}}DependsOnModule-{{[^.]*}}.pcm) system=0 Headers(0):
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -890,7 +890,6 @@
   RECORD(SUBMODULE_DEFINITION);
   RECORD(SUBMODULE_UMBRELLA_HEADER);
   RECORD(SUBMODULE_HEADER);
-  RECORD(SUBMODULE_TOPHEADER);
   RECORD(SUBMODULE_UMBRELLA_DIR);
   RECORD(SUBMODULE_IMPORTS);
   RECORD(SUBMODULE_AFFECTING_MODULES);
@@ -2741,11 +2740,6 @@
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
   unsigned HeaderAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
 
-  Abbrev = std::make_shared();
-  Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_TOPHEADER));
-  Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
-  unsigned TopHeaderAbbrev = Stream.EmitAbbrev(std::move(Abbrev));
-
   Abbrev = std::make_shared();
   Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_UMBRELLA_DIR));
   Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Name
@@ -2873,17 +2867,6 @@
 Stream.EmitRecordWithBlob(HL.Abbrev, Record, H.NameAsWritten);
 }
 
-// Emit the top headers.
-{
-  auto TopHeaders = Mod->getTopHeaders(PP->getFileManager());
-  RecordData::value_type Record[] = {SUBMODULE_TOPHEADER};
-  for (auto *H : TopHeaders) {
-SmallString<128> HeaderName(H->getName());
-PreparePathForOutput(HeaderName);
-Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, HeaderName);
-  }
-}
-
 // Emi

[PATCH] D141929: Add support for clang-cl's option /fexcess-precision.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 490183.
zahiraam marked an inline comment as done.

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

https://reviews.llvm.org/D141929

Files:
  clang/include/clang/Driver/Options.td
  clang/test/Driver/fexcess-precision.c


Index: clang/test/Driver/fexcess-precision.c
===
--- clang/test/Driver/fexcess-precision.c
+++ clang/test/Driver/fexcess-precision.c
@@ -1,29 +1,62 @@
 // RUN: %clang -### -target i386 -fexcess-precision=fast -c %s 2>&1  \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+// RUN: %clang_cl -### -target i386 -fexcess-precision=fast -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+
 // RUN: %clang -### -target i386 -fexcess-precision=standard -c %s 2>&1  \
 // RUN:   | FileCheck --check-prefix=CHECK-STD %s
+// RUN: %clang_cl -### -target i386 -fexcess-precision=standard -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-STD %s
+
 // RUN: %clang -### -target i386 -fexcess-precision=16 -c %s 2>&1  \
 // RUN:   | FileCheck --check-prefix=CHECK-NONE %s
+// RUN: %clang_cl -### -target i386 -fexcess-precision=16 -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-NONE %s
+
 // RUN: %clang -### -target i386 -fexcess-precision=none -c %s 2>&1  \
 // RUN:   | FileCheck --check-prefix=CHECK-ERR-NONE %s
+// RUN: %clang_cl -### -target i386 -fexcess-precision=none -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-ERR-NONE %s
 
 // RUN: %clang -### -target x86_64 -fexcess-precision=fast -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+// RUN: %clang_cl -### -target x86_64 -fexcess-precision=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+
 // RUN: %clang -### -target x86_64 -fexcess-precision=standard -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-STD %s
+// RUN: %clang_cl -### -target x86_64 -fexcess-precision=standard -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-STD %s
+
 // RUN: %clang -### -target x86_64 -fexcess-precision=16 -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-NONE %s
+// RUN: %clang_cl -### -target x86_64 -fexcess-precision=16 -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-NONE %s
+
 // RUN: %clang -### -target x86_64 -fexcess-precision=none -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefixes=CHECK-ERR-NONE %s
+// RUN: %clang_cl -### -target x86_64 -fexcess-precision=none -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefixes=CHECK-ERR-NONE %s
 
 // RUN: %clang -### -target aarch64 -fexcess-precision=fast -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK %s
+// RUN: %clang_cl -### -target aarch64 -fexcess-precision=fast -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK %s
+
 // RUN: %clang -### -target aarch64 -fexcess-precision=standard -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK %s
+// RUN: %clang_cl -### -target aarch64 -fexcess-precision=standard -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK %s
+
 // RUN: %clang -### -target aarch64 -fexcess-precision=16 -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-ERR-16 %s
+// RUN: %clang_cl -### -target aarch64 -fexcess-precision=16 -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ERR-16 %s
+
 // RUN: %clang -### -target aarch64 -fexcess-precision=none -c %s 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-ERR-NONE %s
+// RUN: %clang_cl -### -target aarch64 -fexcess-precision=none -c %s 2>&1 \
+// RUN:   | FileCheck --check-prefix=CHECK-ERR-NONE %s
 
 // CHECK-FAST: "-ffloat16-excess-precision=fast"
 // CHECK-STD: "-ffloat16-excess-precision=standard"
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1579,6 +1579,7 @@
   HelpText<"Enable support for ignoring exception handling constructs">,
   MarshallingInfoFlag>;
 def fexcess_precision_EQ : Joined<["-"], "fexcess-precision=">, Group,
+  Flags<[CoreOption]>,
   HelpText<"Allows control over excess precision on targets where native "
   "support for the precision types is not available. By default, excess "
   "precision is used to calculate intermediate results following the "


Index: clang/test/Driver/fexcess-precision.c
===
--- clang/test/Driver/fexcess-precision.c
+++ clang/test/Driver/fexcess-precision.c
@@ -1,29 +1,62 @@
 // RUN: %clang -### -target i386 -fexcess-precision=fast -c %s 2>&1  \
 // RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+// RUN: %clang_cl -### -target i386 -fexcess-precision=fast -c %s 2>&1  \
+// RUN:   | FileCheck --check-prefix=CHECK-FAST %s
+
 // RUN: %clang -### -target i386 -fexcess-precision=standard -c %s 2>&1  \
 // RUN:   | FileCheck --check-prefix=CHECK-STD %s
+// RUN: %clang_cl -### -target i386 -fexcess-precision=standard -c %s 2>&1  \
+// RUN

[PATCH] D139010: [clang][WebAssembly] Implement support for table types and builtins

[Paulo Matos via Phabricator via cfe-commits]

pmatos marked 5 inline comments as done.
pmatos added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:2171-2176
 def err_reference_bind_to_vector_element : Error<
   "%select{non-const|volatile}0 reference cannot bind to vector element">;
 def err_reference_bind_to_matrix_element : Error<
   "%select{non-const|volatile}0 reference cannot bind to matrix element">;
+def err_reference_bind_to_table_element : Error<
+  "%select{non-const|volatile}0 reference cannot bind to table element">;

aaron.ballman wrote:
> We should combine these diagnostics with a `%select` rather than keep 
> duplicating the same message.
> 
> This is unused and untested.
Right - this is no longer needed.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3035-3040
 def err_attribute_invalid_vector_type : Error<"invalid vector element type 
%0">;
 def err_attribute_invalid_bitint_vector_type : Error<
   "'_BitInt' vector element width must be %select{a power of 2|"
   "at least as wide as 'CHAR_BIT'}0">;
 def err_attribute_invalid_matrix_type : Error<"invalid matrix element type 
%0">;
+def err_attribute_invalid_wasm_table_type : Error<"invalid table element type 
%0">;

aaron.ballman wrote:
> We should probably combine these as well.
> 
> This is unused and untested.
Sigh - again. Apologies. This was an initial patch that used matrix subscript 
to create a table subscript patch but was dropped and therefore is unused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010

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

[PATCH] D142030: [pp-trace] Print HashLoc parameter

[Kyle Edwards via Phabricator via cfe-commits]

KyleFromKitware created this revision.
KyleFromKitware created this object with edit policy "Only User: 
KyleFromKitware (Kyle Edwards)".
Herald added subscribers: kbarton, nemanjai.
Herald added a project: All.
KyleFromKitware requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142030

Files:
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/test/pp-trace/pp-trace-conditional.cpp
  clang-tools-extra/test/pp-trace/pp-trace-include.cpp
  clang-tools-extra/test/pp-trace/pp-trace-macro.cpp

Index: clang-tools-extra/test/pp-trace/pp-trace-macro.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-macro.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-macro.cpp
@@ -32,6 +32,7 @@
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK:  - Callback: MacroDefined
 // CHECK:  - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:3:1"
 // CHECK-NEXT:   MacroNameTok: MACRO
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroExpands
@@ -44,13 +45,16 @@
 // CHECK-NEXT:   MacroDefinition: [(local)]
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:19"]
 // CHECK-NEXT: - Callback: If
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:2"
 // CHECK-NEXT:   ConditionRange: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:19"]
 // CHECK-NEXT:   ConditionValue: CVK_True
 // CHECK-NEXT: - Callback: Endif
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:6:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:6:2"
 // CHECK-NEXT:   IfLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:2"
 // CHECK-NEXT: - Callback: MacroUndefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:7:1"
 // CHECK-NEXT:   MacroNameTok: MACRO
 // CHECK-NEXT:   MacroDefinition: [(local)]
 // CHECK-NEXT: - Callback: Defined
@@ -58,15 +62,18 @@
 // CHECK-NEXT:   MacroDefinition: []
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:19"]
 // CHECK-NEXT: - Callback: If
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:2"
 // CHECK-NEXT:   ConditionRange: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:19"]
 // CHECK-NEXT:   ConditionValue: CVK_False
 // CHECK-NEXT: - Callback: Endif
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:9:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:9:2"
 // CHECK-NEXT:   IfLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:2"
 // CHECK-NEXT: - Callback: SourceRangeSkipped
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:1", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:9:2"]
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:10:1"
 // CHECK-NEXT:   MacroNameTok: FUNCMACRO
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroExpands
@@ -75,12 +82,15 @@
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:11:9", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:11:20"]
 // CHECK-NEXT:   Args: [1]
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:12:1"
 // CHECK-NEXT:   MacroNameTok: X
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:13:1"
 // CHECK-NEXT:   MacroNameTok: X_IMPL
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:14:1"
 // CHECK-NEXT:   MacroNameTok: X_IMPL2
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroExpands
Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -81,6 +81,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level2A.h:1:1"
 // CHECK-NEXT:   MacroNameTok: MACRO_2A
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: FileChanged
@@ -89,6 +90,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level2A.h"
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:2:1"
 // CHECK-NEXT:   MacroNameTok: MACRO_1A
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: FileChanged
@@ -127,6 +129

[PATCH] D142028: [clang] remove SUBMODULE_TOPHEADER

[Richard Howell via Phabricator via cfe-commits]

rmaz added a comment.

@akyrtzi it looks like you added this field a while back. Do you know if there 
are still consumers for the the libclang API, do we need to keep this field 
around?

The alternative is to refactor the `TopHeaders` set to use `FileEntryRef` so 
that we can keep track of the correct import names for the headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142028

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 490187.
erichkeane added a comment.

The changes to the module map file seemed to make things worse! I reverted it 
for now, hoping this just goes green.


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

https://reviews.llvm.org/D141954

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaConcept.cpp
  
clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp


Index: 
clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
===
--- /dev/null
+++ 
clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++20 -x c++ -verify %s
+
+template concept C =
+sizeof(T) == 4 && !true;  // requires atomic constraints sizeof(T) == 4 
and !true
+
+template struct S {
+  constexpr operator bool() const { return true; }
+};
+
+// expected-error@+3{{atomic constraint must be of type 'bool' (found 
'S')}}
+// expected-note@#FINST{{while checking constraint satisfaction}}
+// expected-note@#FINST{{in instantiation of function template specialization}}
+template requires (S{})
+void f(T);
+void f(int);
+
+// Ensure this applies to operator && as well.
+// expected-error@+3{{atomic constraint must be of type 'bool' (found 
'S')}}
+// expected-note@#F2INST{{while checking constraint satisfaction}}
+// expected-note@#F2INST{{in instantiation of function template 
specialization}}
+template requires (S{} && true)
+void f2(T);
+void f2(int);
+
+void g() {
+  f(0); // #FINST
+  f2(0); // #F2INST
+}  
Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -329,14 +329,7 @@
   Sema::SFINAETrap Trap(S);
   SubstitutedExpression =
   S.SubstConstraintExpr(const_cast(AtomicExpr), MLTAL);
-  // Substitution might have stripped off a contextual conversion to
-  // bool if this is the operand of an '&&' or '||'. For example, we
-  // might lose an lvalue-to-rvalue conversion here. If so, put it back
-  // before we try to evaluate.
-  if (SubstitutedExpression.isUsable() &&
-  !SubstitutedExpression.isInvalid())
-SubstitutedExpression =
-
S.PerformContextuallyConvertToBool(SubstitutedExpression.get());
+
   if (SubstitutedExpression.isInvalid() || Trap.hasErrorOccurred()) {
 // C++2a [temp.constr.atomic]p1
 //   ...If substitution results in an invalid type or expression, 
the
@@ -370,8 +363,24 @@
   }
 }
 
+// [temp.constr.atomic]p3: To determine if an atomic constraint is
+// satisfied, the parameter mapping and template arguments are first
+// substituted into its expression.  If substitution results in an
+// invalid type or expression, the constraint is not satisfied.
+// Otherwise, the lvalue-to-rvalue conversion is performed if 
necessary,
+// and E shall be a constant expression of type bool.
+//
+// Perform the L to R Value conversion if necessary. We do so for all
+// non-PRValue categories, else we fail to extend the lifetime of
+// temporaries, and that fails the constant expression check.
+if (!SubstitutedExpression.get()->isPRValue())
+  SubstitutedExpression = ImplicitCastExpr::Create(
+  S.Context, SubstitutedExpression.get()->getType(),
+  CK_LValueToRValue, SubstitutedExpression.get(),
+  /*BasePath=*/nullptr, VK_PRValue, FPOptionsOverride());
+
 if (!S.CheckConstraintExpression(SubstitutedExpression.get()))
-  return ExprError();
+  return ExprError(); // convert to bool here?
 
 return SubstitutedExpression;
   });
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -767,6 +767,9 @@
   and `P1975R0: 
`_,
   which allows parenthesized aggregate-initialization.
 
+- Fixed an issue with concept requirement evaluation, where we incorrectly 
allowed implicit
+  conversions to bool for a requirement.  This fixes `GH54524 
`_.
+
 C++2b Feature Support
 ^
 


Index: clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
===
--- /dev/null
+++ clang/test/CXX/temp/temp.constr/temp.constr.atomic/constrant-satisfaction-conversions.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang_cc1 -std=c++20 -x c++ -verify %s
+
+template concept C =
+sizeof(T) == 4 && !true;  // requires atom

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 updated this revision to Diff 490188.

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

https://reviews.llvm.org/D141871

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/target_ast_print.cpp
  clang/test/OpenMP/target_map_messages.cpp

Index: clang/test/OpenMP/target_map_messages.cpp
===
--- clang/test/OpenMP/target_map_messages.cpp
+++ clang/test/OpenMP/target_map_messages.cpp
@@ -4,6 +4,7 @@
 // RUN: %clang_cc1 -verify=expected,lt50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=45 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,lt51,omp,lt51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=50 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -verify=expected,ge50,ge51,omp,ge51-omp -fopenmp -fno-openmp-extensions -fopenmp-version=51 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,ge50,ge51,ge52,omp,ge52-omp -fopenmp -fno-openmp-extensions -fopenmp-version=52 -ferror-limit 300 %s -Wno-openmp-target -Wuninitialized
 // RUN: %clang_cc1 -DCCODE -verify -fopenmp -fno-openmp-extensions -ferror-limit 300 -x c %s -Wno-openmp -Wuninitialized
 
 // -fopenmp-simd, -fno-openmp-extensions
@@ -158,23 +159,28 @@
 // expected-error@+1 {{use of undeclared identifier 'present'}}
 #pragma omp target map(present)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[1:2],f)
 {}
+// ge52-omp-error@+3 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c,f[1:2])
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(ompx_hold, tofrom: c[:],f)
 {}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // expected-error@+3 {{section length is unspecified and cannot be inferred because subscripted value is not an array}}
 // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
@@ -191,11 +197,15 @@
 // lt51-error@+1 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp target map(present, present, tofrom: a)
 {}
+// ge52-omp-error@+5 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
+// ge52-omp-error@+4 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present', 'iterator'}}
 // ompx-error@+3 {{same map type modifier has been specified more than once}}
 // ge51-omp-error@+2 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper', 'present'}}
 // lt51-omp-error@+1 2 {{incorrect map type modifier, expected one of: 'always', 'close', 'mapper'}}
 #pragma omp

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/include/clang/AST/OpenMPClause.h:5779-5780
 
+  /// Has iterator modifier
+  bool HasIteratorModifier = false;
+

It can be removed



Comment at: clang/test/OpenMP/target_map_messages.cpp:970-979
+  // ompx-error@+8 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // ompx-note@+7 {{'it' declared here}}
+  // omp-error@+6 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // omp-note@+5 {{'it' declared here}}
+  // ge51-ompx-error@+4 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present', 'ompx_hold'}}
+  // lt51-ompx-error@+3 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'ompx_hold'}}
+  // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present'}}

Test cases for wrong variables in mappers?


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

https://reviews.llvm.org/D141871

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

I see the current main pass for the module build in libc++. The error message 
seems different, from the message for missing parts of the module map. I'll dig 
a bit further to see whether I can find what's wrong.


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

https://reviews.llvm.org/D141954

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

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D131052#4052563 , @mstorsjo wrote:

> Switched the macro to a function and changed the variables to cache 
> variables, to allow setting them in the grandparent scope without being a 
> macro.

@beanz - Does this seems ok to you now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131052

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

[PATCH] D142033: [OpenCL] Always add nounwind attribute for OpenCL

[Sven van Haastregt via Phabricator via cfe-commits]

svenvh created this revision.
svenvh added reviewers: Anastasia, yaxunl, bader.
Herald added subscribers: kosarev, Naghasan, ldrumm, kerbowa, jvesely.
Herald added a project: All.
svenvh requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Neither OpenCL nor C++ for OpenCL support exceptions, so add the
`nounwind` attribute unconditionally for those languages.

Unblocks D138958 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142033

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
  clang/test/CodeGenOpenCL/convergent.cl


Index: clang/test/CodeGenOpenCL/convergent.cl
===
--- clang/test/CodeGenOpenCL/convergent.cl
+++ clang/test/CodeGenOpenCL/convergent.cl
@@ -139,4 +139,5 @@
 // CHECK: attributes #3 = { {{[^}]*}}convergent noduplicate{{[^}]*}} }
 // CHECK: attributes #4 = { {{[^}]*}}convergent{{[^}]*}} }
 // CHECK: attributes #5 = { {{[^}]*}}convergent{{[^}]*}} }
-// CHECK: attributes #6 = { {{[^}]*}}convergent noduplicate{{[^}]*}} }
+// CHECK: attributes #6 = { {{[^}]*}}nounwind{{[^}]*}} }
+// CHECK: attributes #7 = { {{[^}]*}}convergent noduplicate nounwind{{[^}]*}} }
Index: clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
+++ clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
@@ -298,7 +298,7 @@
 // CHECK: attributes #2 = { nocallback nofree nounwind willreturn 
memory(argmem: readwrite) }
 // CHECK: attributes #3 = { convergent noinline nounwind optnone 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" 
"target-cpu"="gfx900" 
"target-features"="+16-bit-insts,+ci-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64"
 }
 // CHECK: attributes #4 = { nounwind "enqueued-block" }
-// CHECK: attributes #5 = { convergent }
+// CHECK: attributes #5 = { convergent nounwind }
 //.
 // CHECK: !0 = !{i32 1, !"amdgpu_code_object_version", i32 400}
 // CHECK: !1 = !{i32 1, !"wchar_size", i32 4}
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1969,11 +1969,11 @@
 FuncAttrs.addAttribute(llvm::Attribute::Convergent);
   }
 
-  // TODO: NoUnwind attribute should be added for other GPU modes OpenCL, HIP,
+  // TODO: NoUnwind attribute should be added for other GPU modes HIP,
   // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
   // code.
-  if (getLangOpts().CUDA && getLangOpts().CUDAIsDevice) {
-// Exceptions aren't supported in CUDA device code.
+  if ((getLangOpts().CUDA && getLangOpts().CUDAIsDevice) ||
+  getLangOpts().OpenCL) {
 FuncAttrs.addAttribute(llvm::Attribute::NoUnwind);
   }
 


Index: clang/test/CodeGenOpenCL/convergent.cl
===
--- clang/test/CodeGenOpenCL/convergent.cl
+++ clang/test/CodeGenOpenCL/convergent.cl
@@ -139,4 +139,5 @@
 // CHECK: attributes #3 = { {{[^}]*}}convergent noduplicate{{[^}]*}} }
 // CHECK: attributes #4 = { {{[^}]*}}convergent{{[^}]*}} }
 // CHECK: attributes #5 = { {{[^}]*}}convergent{{[^}]*}} }
-// CHECK: attributes #6 = { {{[^}]*}}convergent noduplicate{{[^}]*}} }
+// CHECK: attributes #6 = { {{[^}]*}}nounwind{{[^}]*}} }
+// CHECK: attributes #7 = { {{[^}]*}}convergent noduplicate nounwind{{[^}]*}} }
Index: clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
===
--- clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
+++ clang/test/CodeGenOpenCL/amdgpu-enqueue-kernel.cl
@@ -298,7 +298,7 @@
 // CHECK: attributes #2 = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
 // CHECK: attributes #3 = { convergent noinline nounwind optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx900" "target-features"="+16-bit-insts,+ci-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64" }
 // CHECK: attributes #4 = { nounwind "enqueued-block" }
-// CHECK: attributes #5 = { convergent }
+// CHECK: attributes #5 = { convergent nounwind }
 //.
 // CHECK: !0 = !{i32 1, !"amdgpu_code_object_version", i32 400}
 // CHECK: !1 = !{i32 1, !"wchar_size", i32 4}
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -1969,11 +1969,11 @@
 FuncAttrs.addAttribute(llvm::Attribute::Convergent);
   }
 
-  // TODO: NoUnwind attribute should be added for other GPU modes OpenCL, HIP,
+  // TODO: NoUnwind attribute should be added for other GPU modes HIP,
   // SYCL, OpenMP offload. AFAIK, none of them support exceptions in device
   // code.
-  if (getLangOpts().CUD

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/test/OpenMP/target_map_messages.cpp:970-979
+  // ompx-error@+8 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // ompx-note@+7 {{'it' declared here}}
+  // omp-error@+6 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // omp-note@+5 {{'it' declared here}}
+  // ge51-ompx-error@+4 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present', 'ompx_hold'}}
+  // lt51-ompx-error@+3 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'ompx_hold'}}
+  // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present'}}

ABataev wrote:
> Test cases for wrong variables in mappers?
You mean as part of the iterator ? like iterator(it = 0:UndefVar) ?


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

https://reviews.llvm.org/D141871

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

[PATCH] D142033: [OpenCL] Always add nounwind attribute for OpenCL

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142033

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

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_map_messages.cpp:970-979
+  // ompx-error@+8 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // ompx-note@+7 {{'it' declared here}}
+  // omp-error@+6 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // omp-note@+5 {{'it' declared here}}
+  // ge51-ompx-error@+4 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present', 'ompx_hold'}}
+  // lt51-ompx-error@+3 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'ompx_hold'}}
+  // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present'}}

doru1004 wrote:
> ABataev wrote:
> > Test cases for wrong variables in mappers?
> You mean as part of the iterator ? like iterator(it = 0:UndefVar) ?
I mean, you have a check that in mappers only iterator vars are allowed. Can 
you add a check for this?


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

https://reviews.llvm.org/D141871

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

[PATCH] D141800: [clangd] Fix qualifier not being dropped for using declaration referring to scoped enum

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1993
 Builder.emplace(Recorder ? &Recorder->CCSema->getASTContext() : 
nullptr,
-Item, SemaCCS, QueryScopes, *Inserter, FileName,
+Recorder ? Recorder->CCSema->CurContext : nullptr, 
Item,
+SemaCCS, QueryScopes, *Inserter, FileName,

instead of passing it here, can we just do the traversal as part of 
`getQueryScopes` and let these be handled uniformly by the shortest qualifier 
logic ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141800

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

[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.



Comment at: clang/test/OpenMP/target_map_messages.cpp:970-979
+  // ompx-error@+8 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // ompx-note@+7 {{'it' declared here}}
+  // omp-error@+6 {{use of undeclared identifier 'itt'; did you mean 'it'?}}
+  // omp-note@+5 {{'it' declared here}}
+  // ge51-ompx-error@+4 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present', 'ompx_hold'}}
+  // lt51-ompx-error@+3 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'ompx_hold'}}
+  // ge51-omp-error@+2 {{incorrect map type modifier, expected one of: 
'always', 'close', 'mapper', 'present'}}

ABataev wrote:
> doru1004 wrote:
> > ABataev wrote:
> > > Test cases for wrong variables in mappers?
> > You mean as part of the iterator ? like iterator(it = 0:UndefVar) ?
> I mean, you have a check that in mappers only iterator vars are allowed. Can 
> you add a check for this?
As far as I understand it the additional check I added to the existing mapper 
checks is needed because of the way the mapper check was written. The mapper 
checks looks at declarations and if a mapper clause exists then it assumes that 
the declaration must be coming from that mapper clause. This used to hold in 
the past since that was the only way you could have a declaration. This is not 
true anymore since we can now have declarations coming from both the mapper and 
the iterator modifier. I'll add a test to showcase this.


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

https://reviews.llvm.org/D141871

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

[PATCH] D119708: [clang][lex] Remove `PPCallbacks::FileNotFound()`

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

If the plan is to eventually upstream that part of Cling, I'm fine with 
re-adding a safe version of this API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119708

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

[clang] 29d25f9 - [clang][Interp][NFC] Simplify InterpFrame constructor

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2023-01-18T18:14:23+01:00
New Revision: 29d25f9e9a4c075023ac7d8f19e7073e9b2a0359

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

LOG: [clang][Interp][NFC] Simplify InterpFrame constructor

Try to decrease the block depth here a bit.

Added: 


Modified: 
clang/lib/AST/Interp/InterpFrame.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/InterpFrame.cpp 
b/clang/lib/AST/Interp/InterpFrame.cpp
index 9acfbe3700e98..da228d011f874 100644
--- a/clang/lib/AST/Interp/InterpFrame.cpp
+++ b/clang/lib/AST/Interp/InterpFrame.cpp
@@ -24,19 +24,22 @@ InterpFrame::InterpFrame(InterpState &S, const Function 
*Func,
 : Caller(Caller), S(S), Func(Func), RetPC(RetPC),
   ArgSize(Func ? Func->getArgSize() : 0),
   Args(static_cast(S.Stk.top())), FrameOffset(S.Stk.size()) {
-  if (Func) {
-if (unsigned FrameSize = Func->getFrameSize()) {
-  Locals = std::make_unique(FrameSize);
-  for (auto &Scope : Func->scopes()) {
-for (auto &Local : Scope.locals()) {
-  Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
-  B->invokeCtor();
-  InlineDescriptor *ID = localInlineDesc(Local.Offset);
-  ID->Desc = Local.Desc;
-  ID->IsActive = true;
-  ID->Offset = sizeof(InlineDescriptor);
-}
-  }
+  if (!Func)
+return;
+
+  unsigned FrameSize = Func->getFrameSize();
+  if (FrameSize == 0)
+return;
+
+  Locals = std::make_unique(FrameSize);
+  for (auto &Scope : Func->scopes()) {
+for (auto &Local : Scope.locals()) {
+  Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
+  B->invokeCtor();
+  InlineDescriptor *ID = localInlineDesc(Local.Offset);
+  ID->Desc = Local.Desc;
+  ID->IsActive = true;
+  ID->Offset = sizeof(InlineDescriptor);
 }
   }
 }



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

[PATCH] D131052: [CMake] Allow setting the location of host tools with LLVM_NATIVE_TOOL_DIR

[Chris Bieneman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131052

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

I just tested locally with the latest nightly build

  Debian clang version 16.0.0 
(++20230116071901+2563ad8ef1a8-1~exp1~20230116072002.532)
  Target: x86_64-pc-linux-gnu
  Thread model: posix
  InstalledDir: /usr/bin

and that passes the test that fails in the CI.

Can you test whether the issue occurs for you in HEAD without your patch.

@ldionne would it make sense to test the Clang CI in the scheduled build too, 
that would make pin-pointing a possible regression easier.


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

https://reviews.llvm.org/D141954

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

[PATCH] D142033: [OpenCL] Always add nounwind attribute for OpenCL

[Alexey Bader via Phabricator via cfe-commits]

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

Should we generalize and rename `clang/test/CodeGenOpenCL/convergent.cl` to 
validate function attributes other than `convergent`? It's not obvious that 
presence of `nounwind` attribute is validated by 
`clang/test/CodeGenOpenCL/convergent.cl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142033

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

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D141954#4062669 , @Mordante wrote:

> I just tested locally with the latest nightly build
>
>   Debian clang version 16.0.0 
> (++20230116071901+2563ad8ef1a8-1~exp1~20230116072002.532)
>   Target: x86_64-pc-linux-gnu
>   Thread model: posix
>   InstalledDir: /usr/bin
>
> and that passes the test that fails in the CI.
>
> Can you test whether the issue occurs for you in HEAD without your patch.
>
> @ldionne would it make sense to test the Clang CI in the scheduled build too, 
> that would make pin-pointing a possible regression easier.

I don't have the libcxx build/tests set up in my environment anymore (and it 
was a bit of a hassle the last time, which was my motivation for asking 
@ldionne for pre-commit on clang patches :D ), so I don't really have the 
ability to run that test.


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

https://reviews.llvm.org/D141954

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

[PATCH] D142030: [pp-trace] Print HashLoc parameter

[Kyle Edwards via Phabricator via cfe-commits]

KyleFromKitware updated this revision to Diff 490201.

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

https://reviews.llvm.org/D142030

Files:
  clang-tools-extra/docs/pp-trace.rst
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/test/pp-trace/pp-trace-conditional.cpp
  clang-tools-extra/test/pp-trace/pp-trace-include.cpp
  clang-tools-extra/test/pp-trace/pp-trace-macro.cpp

Index: clang-tools-extra/test/pp-trace/pp-trace-macro.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-macro.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-macro.cpp
@@ -32,6 +32,7 @@
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK:  - Callback: MacroDefined
 // CHECK:  - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:3:1"
 // CHECK-NEXT:   MacroNameTok: MACRO
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroExpands
@@ -44,13 +45,16 @@
 // CHECK-NEXT:   MacroDefinition: [(local)]
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:19"]
 // CHECK-NEXT: - Callback: If
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:2"
 // CHECK-NEXT:   ConditionRange: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:19"]
 // CHECK-NEXT:   ConditionValue: CVK_True
 // CHECK-NEXT: - Callback: Endif
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:6:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:6:2"
 // CHECK-NEXT:   IfLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:2"
 // CHECK-NEXT: - Callback: MacroUndefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:7:1"
 // CHECK-NEXT:   MacroNameTok: MACRO
 // CHECK-NEXT:   MacroDefinition: [(local)]
 // CHECK-NEXT: - Callback: Defined
@@ -58,15 +62,18 @@
 // CHECK-NEXT:   MacroDefinition: []
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:19"]
 // CHECK-NEXT: - Callback: If
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:2"
 // CHECK-NEXT:   ConditionRange: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:19"]
 // CHECK-NEXT:   ConditionValue: CVK_False
 // CHECK-NEXT: - Callback: Endif
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:9:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:9:2"
 // CHECK-NEXT:   IfLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:2"
 // CHECK-NEXT: - Callback: SourceRangeSkipped
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:1", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:9:2"]
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:10:1"
 // CHECK-NEXT:   MacroNameTok: FUNCMACRO
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroExpands
@@ -75,12 +82,15 @@
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:11:9", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:11:20"]
 // CHECK-NEXT:   Args: [1]
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:12:1"
 // CHECK-NEXT:   MacroNameTok: X
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:13:1"
 // CHECK-NEXT:   MacroNameTok: X_IMPL
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:14:1"
 // CHECK-NEXT:   MacroNameTok: X_IMPL2
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroExpands
Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -81,6 +81,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level2A.h:1:1"
 // CHECK-NEXT:   MacroNameTok: MACRO_2A
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: FileChanged
@@ -89,6 +90,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level2A.h"
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:2:1"
 // CHECK-NEXT:   MacroNameTok: MACRO_1A
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: FileChanged
@@ -127,6 +129,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1B.h"
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level2B.h:1:1"
 // CHECK-NEXT:   

[PATCH] D141324: [clang] extend external_source_symbol attribute with the USR clause

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

I posted the Swift patch here:
https://github.com/apple/swift/pull/63002


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

https://reviews.llvm.org/D141324

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

[PATCH] D142022: [Clang][OpenMP] Fix handling of -mcode-object-version for OpenMP

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam updated this revision to Diff 490204.
saiislam added a comment.

Removed the unnecessary call to getArgs() and added test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142022

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/amdgpu-openmp-toolchain.c


Index: clang/test/Driver/amdgpu-openmp-toolchain.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain.c
+++ clang/test/Driver/amdgpu-openmp-toolchain.c
@@ -69,3 +69,10 @@
 // RUN:   --rocm-device-lib-path=%S/Inputs/rocm/amdgcn/bitcode 
-fopenmp-new-driver %s  2>&1 | \
 // RUN: FileCheck %s --check-prefix=CHECK-LIB-DEVICE-NOGPULIB
 // CHECK-LIB-DEVICE-NOGPULIB-NOT: "-cc1" 
{{.*}}ocml.bc"{{.*}}ockl.bc"{{.*}}oclc_daz_opt_on.bc"{{.*}}oclc_unsafe_math_off.bc"{{.*}}oclc_finite_only_off.bc"{{.*}}oclc_correctly_rounded_sqrt_on.bc"{{.*}}oclc_wavefrontsize64_on.bc"{{.*}}oclc_isa_version_803.bc"
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu 
-mcode-object-version=4 -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa 
-Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-COV4
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu 
-mcode-object-version=5 -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa 
-Xopenmp-target=amdgcn-amd-amdhsa -march=gfx803 %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-COV5
+// CHECK-COV4: "-cc1" {{.*}}oclc_abi_version_400.bc
+// CHECK-COV4-NOT: "-cc1" {{.*}}oclc_abi_version_500.bc
+// CHECK-COV5: "-cc1" {{.*}}oclc_abi_version_500.bc
+// CHECK-COV5-NOT: "-cc1" {{.*}}oclc_abi_version_400.bc
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8087,7 +8087,8 @@
   }
 
   if (Triple.isAMDGPU())
-handleAMDGPUCodeObjectVersionOptions(D, Args, CmdArgs, /*IsCC1As=*/true);
+handleAMDGPUCodeObjectVersionOptions(D, C.getArgs(), CmdArgs,
+ /*IsCC1As=*/true);
 
   assert(Input.isFilename() && "Invalid input.");
   CmdArgs.push_back(Input.getFilename());
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.h
@@ -26,8 +26,8 @@
 : public ROCMToolChain {
 public:
   AMDGPUOpenMPToolChain(const Driver &D, const llvm::Triple &Triple,
-const ToolChain &HostTC,
-const llvm::opt::ArgList &Args);
+const ToolChain &HostTC, const llvm::opt::ArgList 
&Args,
+const llvm::opt::DerivedArgList &DerivedArgs);
 
   const llvm::Triple *getAuxTriple() const override {
 return &HostTC.getTriple();
@@ -58,6 +58,7 @@
   getDeviceLibs(const llvm::opt::ArgList &Args) const override;
 
   const ToolChain &HostTC;
+  const llvm::opt::DerivedArgList &DerivedArgs;
 };
 
 } // end namespace toolchains
Index: clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
===
--- clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
+++ clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
@@ -58,8 +58,9 @@
 AMDGPUOpenMPToolChain::AMDGPUOpenMPToolChain(const Driver &D,
  const llvm::Triple &Triple,
  const ToolChain &HostTC,
- const ArgList &Args)
-: ROCMToolChain(D, Triple, Args), HostTC(HostTC) {
+ const ArgList &Args,
+ const DerivedArgList &DerivedArgs)
+: ROCMToolChain(D, Triple, Args), HostTC(HostTC), DerivedArgs(DerivedArgs) 
{
   // Lookup binaries into the driver directory, this is used to
   // discover the clang-offload-bundler executable.
   getProgramPaths().push_back(getDriver().Dir);
@@ -190,7 +191,7 @@
   getTriple(), Args.getLastArgValue(options::OPT_march_EQ));
 
   SmallVector BCLibs;
-  for (auto BCLib : getCommonDeviceLibNames(Args, GpuArch.str(),
+  for (auto BCLib : getCommonDeviceLibNames(DerivedArgs, GpuArch.str(),
 /*IsOpenMP=*/true))
 BCLibs.emplace_back(BCLib);
 
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -872,7 +872,7 @@
   }
   if (AMDTriple) {
 auto TempTC = std::make_unique(
-*this, *AMDTriple, *HostTC, C.getInputArgs());
+*this, *AMDTriple, *HostTC, C.getInputArgs(), C.getArgs());
 for (String

[PATCH] D142022: [Clang][OpenMP] Fix handling of -mcode-object-version for OpenMP

[Saiyedul Islam via Phabricator via cfe-commits]

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



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7116
   if (Triple.isAMDGPU()) {
-handleAMDGPUCodeObjectVersionOptions(D, Args, CmdArgs);
+handleAMDGPUCodeObjectVersionOptions(D, C.getArgs(), CmdArgs);
 

yaxunl wrote:
> why do you need to change Args to C.getArgs() ?
You are right. Don't need it. Removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142022

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

[PATCH] D142030: [pp-trace] Print HashLoc parameter

[Kyle Edwards via Phabricator via cfe-commits]

KyleFromKitware updated this revision to Diff 490206.

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

https://reviews.llvm.org/D142030

Files:
  clang-tools-extra/docs/pp-trace.rst
  clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
  clang-tools-extra/test/pp-trace/pp-trace-conditional.cpp
  clang-tools-extra/test/pp-trace/pp-trace-include.cpp
  clang-tools-extra/test/pp-trace/pp-trace-macro.cpp

Index: clang-tools-extra/test/pp-trace/pp-trace-macro.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-macro.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-macro.cpp
@@ -32,6 +32,7 @@
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK:  - Callback: MacroDefined
 // CHECK:  - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:3:1"
 // CHECK-NEXT:   MacroNameTok: MACRO
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroExpands
@@ -44,13 +45,16 @@
 // CHECK-NEXT:   MacroDefinition: [(local)]
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:19"]
 // CHECK-NEXT: - Callback: If
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:2"
 // CHECK-NEXT:   ConditionRange: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:19"]
 // CHECK-NEXT:   ConditionValue: CVK_True
 // CHECK-NEXT: - Callback: Endif
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:6:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:6:2"
 // CHECK-NEXT:   IfLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:5:2"
 // CHECK-NEXT: - Callback: MacroUndefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:7:1"
 // CHECK-NEXT:   MacroNameTok: MACRO
 // CHECK-NEXT:   MacroDefinition: [(local)]
 // CHECK-NEXT: - Callback: Defined
@@ -58,15 +62,18 @@
 // CHECK-NEXT:   MacroDefinition: []
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:19"]
 // CHECK-NEXT: - Callback: If
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:2"
 // CHECK-NEXT:   ConditionRange: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:5", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:19"]
 // CHECK-NEXT:   ConditionValue: CVK_False
 // CHECK-NEXT: - Callback: Endif
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:9:1"
 // CHECK-NEXT:   Loc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:9:2"
 // CHECK-NEXT:   IfLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:2"
 // CHECK-NEXT: - Callback: SourceRangeSkipped
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:8:1", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:9:2"]
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:10:1"
 // CHECK-NEXT:   MacroNameTok: FUNCMACRO
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroExpands
@@ -75,12 +82,15 @@
 // CHECK-NEXT:   Range: ["{{.*}}{{[/\\]}}pp-trace-macro.cpp:11:9", "{{.*}}{{[/\\]}}pp-trace-macro.cpp:11:20"]
 // CHECK-NEXT:   Args: [1]
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:12:1"
 // CHECK-NEXT:   MacroNameTok: X
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:13:1"
 // CHECK-NEXT:   MacroNameTok: X_IMPL
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-macro.cpp:14:1"
 // CHECK-NEXT:   MacroNameTok: X_IMPL2
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: MacroExpands
Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -81,6 +81,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level2A.h:1:1"
 // CHECK-NEXT:   MacroNameTok: MACRO_2A
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: FileChanged
@@ -89,6 +90,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level2A.h"
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:2:1"
 // CHECK-NEXT:   MacroNameTok: MACRO_1A
 // CHECK-NEXT:   MacroDirective: MD_Define
 // CHECK-NEXT: - Callback: FileChanged
@@ -127,6 +129,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1B.h"
 // CHECK-NEXT: - Callback: MacroDefined
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level2B.h:1:1"
 // CHECK-NEXT:   

[clang] 9ee625b - [Clang] Update the help message for `--offload-arch`

[Joseph Huber via cfe-commits]

Author: Joseph Huber
Date: 2023-01-18T11:41:08-06:00
New Revision: 9ee625bd992fba4ba5ef9102e5e02bc87c7252c4

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

LOG: [Clang] Update the help message for `--offload-arch`

Summary:
This works for `OpenMP` and supports the `native` option. It should be
better documented.

Added: 


Modified: 
clang/include/clang/Driver/Options.td

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index a163c77fdb13c..ba49b335cf287 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -940,10 +940,10 @@ def cuda_include_ptx_EQ : Joined<["--"], 
"cuda-include-ptx=">, Flags<[NoXarchOpt
 def no_cuda_include_ptx_EQ : Joined<["--"], "no-cuda-include-ptx=">, 
Flags<[NoXarchOption]>,
   HelpText<"Do not include PTX for the following GPU architecture (e.g. sm_35) 
or 'all'. May be specified more than once.">;
 def offload_arch_EQ : Joined<["--"], "offload-arch=">, Flags<[NoXarchOption]>,
-  HelpText<"CUDA offloading device architecture (e.g. sm_35), or HIP 
offloading target ID in the form of a "
-   "device architecture followed by target ID features delimited by a 
colon. Each target ID feature "
-   "is a pre-defined string followed by a plus or minus sign (e.g. 
gfx908:xnack+:sramecc-).  May be "
-   "specified more than once.">;
+  HelpText<"Specify an offloading device architecture for CUDA, HIP, or 
OpenMP. (e.g. sm_35). "
+   "If 'native' is used the compiler will detect locally installed 
architectures. "
+   "For HIP offloading, the device architecture can be followed by 
target ID features "
+   "delimited by a colon (e.g. gfx908:xnack+:sramecc-). May be 
specified more than once.">;
 def cuda_gpu_arch_EQ : Joined<["--"], "cuda-gpu-arch=">, 
Flags<[NoXarchOption]>,
   Alias;
 def cuda_feature_EQ : Joined<["--"], "cuda-feature=">, HelpText<"Manually 
specify the CUDA feature to use">;



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

[PATCH] D140874: [clang][Interp] Support pointer types in compound assignment operations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:727-728
+this->emitSubOffset(*RT, E);
+  else
+return false;
+

Should this be an early return before we visit anything? Or an assert on the 
assumption that we should only get here for `+=` and `-=`?


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

https://reviews.llvm.org/D140874

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

[PATCH] D136031: [DirectX backend] support ConstantBuffer to DXILResource.h

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

@python3kgae, this change introduced a bunch of warning spew because it is 
using an API that was deprecated shortly before the change merged. Can you 
please address this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136031

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

[PATCH] D139010: [clang][WebAssembly] Implement support for table types and builtins

[Paulo Matos via Phabricator via cfe-commits]

pmatos updated this revision to Diff 490208.
pmatos added a comment.

Remove some unnecessary diagnostics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGValue.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/WebAssembly/builtins-table.c
  clang/test/CodeGen/WebAssembly/table.c
  clang/test/Sema/builtins-wasm.c
  clang/test/Sema/wasm-refs.c
  clang/test/SemaCXX/wasm-refs-and-tables.cpp
  clang/test/SemaCXX/wasm-refs.cpp
  llvm/include/llvm/CodeGen/WasmAddressSpaces.h
  llvm/include/llvm/IR/Type.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp
@@ -62,8 +62,9 @@
   for (inst_iterator I = inst_begin(F), E = inst_end(F); I != E; ++I) {
 PtrToIntInst *PTI = dyn_cast(&*I);
 IntToPtrInst *ITP = dyn_cast(&*I);
-if (!(PTI && WebAssembly::isRefType(PTI->getPointerOperand()->getType())) &&
-!(ITP && WebAssembly::isRefType(ITP->getDestTy(
+if (!(PTI &&
+  PTI->getPointerOperand()->getType()->isWebAssemblyReferenceType()) &&
+!(ITP && ITP->getDestTy()->isWebAssemblyReferenceType()))
   continue;
 
 UndefValue *U = UndefValue::get(I->getType());
Index: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1202,7 +1202,7 @@
   // Lastly, if this is a call to a funcref we need to add an instruction
   // table.set to the chain and transform the call.
   if (CLI.CB &&
-  WebAssembly::isFuncrefType(CLI.CB->getCalledOperand()->getType())) {
+  CLI.CB->getCalledOperand()->getType()->isWebAssemblyFuncrefType()) {
 // In the absence of function references proposal where a funcref call is
 // lowered to call_ref, using reference types we generate a table.set to set
 // the funcref to a special table used solely for this purpose, followed by
Index: llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
===
--- llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
+++ llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
@@ -45,43 +45,6 @@
   Multivalue = 0x,
 };
 
-enum WasmAddressSpace : unsigned {
-  // Default address space, for pointers to linear memory (stack, heap, data).
-  WASM_ADDRESS_SPACE_DEFAULT = 0,
-  // A non-integral address space for pointers to named objects outside of
-  // linear memory: WebAssembly globals or WebAssembly locals.  Loads and stores
-  // to these pointers are lowered to global.get / global.set or local.get /
-  // local.set, as appropriate.
-  WASM_ADDRESS_SPACE_VAR = 1,
-  // A non-integral address space for externref values
-  WASM_ADDRESS_SPACE_EXTERNREF = 10,
-  // A non-integral address space for funcref values
-  WASM_ADDRESS_SPACE_FUNCREF = 20,
-};
-
-inline bool isDefaultAddressSpace(unsigned AS) {
-  return AS == WASM_ADDRESS_SPACE_DEFAULT;
-}
-inline bool isWasmVarAddressSpace(unsigned AS) {
-  return AS == WASM_ADDRESS_SPACE_VAR;
-}
-inline bool isValidAddressSpace(unsigned AS) {
-  return isDefaultAddressSpace(AS) || isWasmVarAddressSpace(AS);
-}
-inline bool isFuncrefType(const Type *Ty) {
-  return isa(Ty) &&
- Ty->getPointerAddressSpace() ==
- WasmAddressSpace::WASM_ADDRESS_SPACE_FUNCREF;
-}
-inline bool isExternrefType(const Type *Ty) {
-  return isa(Ty) &&
- Ty->getPointerAddressSpace() ==
- WasmAddressSpace::WASM_ADDRESS_SPACE_EXTERNREF;
-}
-inline bool isRe

[PATCH] D141310: [clang] add -Wcompare-function-pointers

[Adrian Dole via Phabricator via cfe-commits]

adriandole added a comment.

@dblaikie, we would use this warning in Chrome OS. We use `icf=all` and have 
encountered bugs caused by function pointer comparisons.

It's not that noisy compiling clang (eight hits). Working on testing it for 
Chrome OS.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141310

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

[PATCH] D141472: [clang][Interp] Add function pointers

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:138-139
   case CK_IntegralToPointer: {
 if (isa(SubExpr))
   return this->visit(SubExpr);
 

Can we still reach this bit now?


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

https://reviews.llvm.org/D141472

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

[PATCH] D141324: [clang] extend external_source_symbol attribute with the USR clause

[Alex Lorenz via Phabricator via cfe-commits]

arphaman updated this revision to Diff 490214.
arphaman added a comment.

add more test coverage


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

https://reviews.llvm.org/D141324

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticCommonKinds.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Index/USRGeneration.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/Index/Core/external-source-symbol-attr-cxx.cpp
  clang/test/Index/Core/external-source-symbol-attr.m
  clang/test/Parser/attr-external-source-symbol.m
  clang/test/Sema/attr-external-source-symbol-cxx.cpp
  clang/test/Sema/attr-external-source-symbol.c

Index: clang/test/Sema/attr-external-source-symbol.c
===
--- clang/test/Sema/attr-external-source-symbol.c
+++ clang/test/Sema/attr-external-source-symbol.c
@@ -4,7 +4,9 @@
 
 void twoClauses(void) __attribute__((external_source_symbol(language="Swift", defined_in="module")));
 
-void fourClauses(void) __attribute__((external_source_symbol(language="Swift", defined_in="module", generated_declaration, generated_declaration))); // expected-error {{duplicate 'generated_declaration' clause in an 'external_source_symbol' attribute}}
+void fourClauses(void) __attribute__((external_source_symbol(language="Swift", defined_in="module", generated_declaration, USR="test")));
+
+void fiveClauses(void) __attribute__((external_source_symbol(language="Swift", defined_in="module", generated_declaration, generated_declaration, USR="test"))); // expected-error {{duplicate 'generated_declaration' clause in an 'external_source_symbol' attribute}}
 
 void oneClause(void) __attribute__((external_source_symbol(generated_declaration)));
 
@@ -22,8 +24,8 @@
 
 [[clang::external_source_symbol(language="Swift", defined_in="module")]] void twoClauses2(void);
 
-[[clang::external_source_symbol(language="Swift", defined_in="module", generated_declaration, generated_declaration)]] // expected-error {{duplicate 'generated_declaration' clause in an 'external_source_symbol' attribute}}
-void fourClauses2(void);
+[[clang::external_source_symbol(language="Swift", defined_in="module", USR="test", generated_declaration, generated_declaration)]] // expected-error {{duplicate 'generated_declaration' clause in an 'external_source_symbol' attribute}}
+void fiveClauses2(void);
 
 [[clang::external_source_symbol(generated_declaration)]] void oneClause2(void);
 
Index: clang/test/Sema/attr-external-source-symbol-cxx.cpp
===
--- /dev/null
+++ clang/test/Sema/attr-external-source-symbol-cxx.cpp
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fsyntax-only -fblocks -verify -fdouble-square-bracket-attributes %s
+
+template
+class Class {
+public:
+[[clang::external_source_symbol(language="Swift", defined_in="module", USR="test", generated_declaration)]]
+void testExternalSourceSymbol();
+
+// expected-error@+1 {{expected string literal for USR in 'external_source_symbol' attribute}}
+[[clang::external_source_symbol(language="Swift", defined_in="module", USR=T, generated_declaration)]]
+void testExternalSourceSymbol2();
+};
+
+template
+void Class::testExternalSourceSymbol() {
+}
Index: clang/test/Parser/attr-external-source-symbol.m
===
--- clang/test/Parser/attr-external-source-symbol.m
+++ clang/test/Parser/attr-external-source-symbol.m
@@ -14,10 +14,14 @@
   CaseB __attribute__((external_source_symbol(generated_declaration, language="Swift")))
 } __attribute__((external_source_symbol(language = "Swift")));
 
+void functionCustomUSR(void) __attribute__((external_source_symbol(language="Swift", defined_in="module", generated_declaration, USR="s:6module17functionCustomUSRyyF")));
+
+void functionCustomUSR2(void) __attribute__((external_source_symbol(language="Swift", defined_in="module", USR="s:6module18functionCustomUSR2yyF", generated_declaration)));
+
 void f2(void)
-__attribute__((external_source_symbol())); // expected-error {{expected 'language', 'defined_in', or 'generated_declaration'}}
+__attribute__((external_source_symbol())); // expected-error {{expected 'language', 'defined_in', 'generated_declaration', or 'USR'}}
 void f3(void)
-__attribute__((external_source_symbol(invalid))); // expected-error {{expected 'language', 'defined_in', or 'generated_declaration'}}
+__attribute__((external_source_symbol(invalid))); // expected-error {{expected 'language', 'defined_in', 'generated_declaration', or 'USR'}}
 void f4(void)
 __attribute__((external_source_symbol(language))); // expected-error {{expected '=' after language}}
 void f5(void)
@@ -31,6 +35,8 @@
 __attribute__((externa