[PATCH] D133633: [CMake] Add ClangBootstrap configuration

2022-11-15 Thread Petr Hosek via Phabricator via cfe-commits
phosek added inline comments.



Comment at: clang/cmake/modules/ClangBootstrap.cmake:10
+# Optional arguments to pass to the build tool
+macro(clang_Bootstrap_Add name)
+  cmake_parse_arguments(ARG "" "LINKER;AR;RANLIB;OBJCOPY;STRIP"

We usually use lowercase names.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133633

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


[PATCH] D137181: [clang-format] Don't use 'PPIndentWidth' inside multi-line macros

2022-11-15 Thread Owen Pan via Phabricator via cfe-commits
owenpan added a comment.

In D137181#3924002 , @sstwcw wrote:

> Can you make `TokenAnnotator::printDebugInfo` print `PPLevel`?

@goldstein.w.n can you add it as follows?

  $ git diff TokenAnnotator.cpp
  diff --git a/clang/lib/Format/TokenAnnotator.cpp 
b/clang/lib/Format/TokenAnnotator.cpp
  index 75570552146c..536472e9d136 100644
  --- a/clang/lib/Format/TokenAnnotator.cpp
  +++ b/clang/lib/Format/TokenAnnotator.cpp
  @@ -5093,8 +5093,9 @@ bool TokenAnnotator::canBreakBefore(const AnnotatedLine 
&Line,
   }
   
   void TokenAnnotator::printDebugInfo(const AnnotatedLine &Line) const {
  -  llvm::errs() << "AnnotatedTokens(L=" << Line.Level << ", T=" << Line.Type
  -   << ", C=" << Line.IsContinuation << "):\n";
  +  llvm::errs() << "AnnotatedTokens(L=" << Line.Level << ", P=" << 
Line.PPLevel
  +   << ", T=" << Line.Type << ", C=" << Line.IsContinuation
  +   << "):\n";
 const FormatToken *Tok = Line.First;
 while (Tok) {
   llvm::errs() << " M=" << Tok->MustBreakBefore



> Since you changed the rules for indentation in `UnwrappedLineFormatter`, do 
> you also need to change `UnwrappedLineParser::mightFitOnOneLine`?

`mightFitOnOneLine` is not called on PPDirective lines, but it's a good idea to 
add an assertion.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:836
   }
 
   return Line.Level * Style.IndentWidth + Length <= ColumnLimit;

We don't call `mightFitOnOneLine` on PPDirective lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137181

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


[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

2022-11-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet updated this revision to Diff 475362.
kadircet added a comment.

- Drop documentation
- Call remove_dots on the common path


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137962

Files:
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -92,16 +92,32 @@
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/file1", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
+  llvm::sys::path::native("//net/dir/file3", PathStorage);
+  expected_files.push_back(std::string(PathStorage.str()));
   EXPECT_EQ(expected_files,
-getAllFiles("[{\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"../file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file2\"}]",
+getAllFiles(R"json(
+[
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "../file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file2"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "//net/dir/foo/../file3"
+  }
+])json",
 ErrorMessage, JSONCommandLineSyntax::Gnu))
   << ErrorMessage;
 }
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -419,14 +419,13 @@
 SmallString<128> NativeFilePath;
 if (llvm::sys::path::is_relative(FileName)) {
   SmallString<8> DirectoryStorage;
-  SmallString<128> AbsolutePath(
-  Directory->getValue(DirectoryStorage));
+  SmallString<128> AbsolutePath(Directory->getValue(DirectoryStorage));
   llvm::sys::path::append(AbsolutePath, FileName);
-  llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/ true);
   llvm::sys::path::native(AbsolutePath, NativeFilePath);
 } else {
   llvm::sys::path::native(FileName, NativeFilePath);
 }
+llvm::sys::path::remove_dots(NativeFilePath, /*remove_dot_dot=*/true);
 auto Cmd = CompileCommandRef(Directory, File, *Command, Output);
 IndexByFile[NativeFilePath].push_back(Cmd);
 AllCommands.push_back(Cmd);


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -92,16 +92,32 @@
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/file1", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
+  llvm::sys::path::native("//net/dir/file3", PathStorage);
+  expected_files.push_back(std::string(PathStorage.str()));
   EXPECT_EQ(expected_files,
-getAllFiles("[{\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"../file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file2\"}]",
+getAllFiles(R"json(
+[
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "../file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file2"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "//net/dir/foo/../file3"
+  

[clang] d649452 - [code-owners] Add Vassil as a code owner for clang incremental compilation.

2022-11-15 Thread Vassil Vassilev via cfe-commits

Author: Vassil Vassilev
Date: 2022-11-15T08:56:16Z
New Revision: d6494524490e0d4ffb99dcf21d633c11108b6bf6

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

LOG: [code-owners] Add Vassil as a code owner for clang incremental compilation.

Update the code owners file based on the discussion in the forum:
https://discourse.llvm.org/t/rfc-add-a-code-owner-for-incremental-compilation-incremental-c/66345

Added: 


Modified: 
clang/CodeOwners.rst

Removed: 




diff  --git a/clang/CodeOwners.rst b/clang/CodeOwners.rst
index 25c8521b2e06..30a164f99034 100644
--- a/clang/CodeOwners.rst
+++ b/clang/CodeOwners.rst
@@ -196,6 +196,12 @@ General Windows support
 | rnk\@google.com (email), rnk (Phabricator), rnk (GitHub)
 
 
+Incremental compilation, REPLs, clang-repl
+~~
+| Vassil Vassilev
+| Vassil.Vassilev\@cern.ch (email), v.g.vassilev (Phabricator), vgvassilev 
(GitHub)
+
+
 Standards Conformance
 -
 The following people are responsible for validating that changes are conforming



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


[clang] cb2289f - [C++20] [Modules] Attach implicitly declared allocation funcitons to

2022-11-15 Thread Chuanqi Xu via cfe-commits

Author: Chuanqi Xu
Date: 2022-11-15T17:21:48+08:00
New Revision: cb2289f39240a0fdccc9a853a02ae9751578a0fd

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

LOG: [C++20] [Modules] Attach implicitly declared allocation funcitons to
global module fragment

[basic.stc.dynamic.general]p2 says:
> The library provides default definitions for the global allocation
> and deallocation functions. Some global allocation and
> deallocation
> functions are replaceable ([new.delete]); these are attached to
> the global module ([module.unit]).

But we didn't take this before and the implicitly generated functions
will live in the module purview if we're compiling a module unit. This
is bad since the owning module will affect the linkage of the
declarations. This patch addresses this.

Closes https://github.com/llvm/llvm-project/issues/58560

Added: 
clang/test/Modules/implicit-declared-allocation-functions.cppm

Modified: 
clang/lib/Basic/Module.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/unittests/AST/DeclTest.cpp

Removed: 




diff  --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp
index 9a58faee1fdd8..2f30fa0f02adf 100644
--- a/clang/lib/Basic/Module.cpp
+++ b/clang/lib/Basic/Module.cpp
@@ -633,7 +633,9 @@ LLVM_DUMP_METHOD void Module::dump() const {
 
 void VisibleModuleSet::setVisible(Module *M, SourceLocation Loc,
   VisibleCallback Vis, ConflictCallback Cb) {
-  assert(Loc.isValid() && "setVisible expects a valid import location");
+  // We can't import a global module fragment so the location can be invalid.
+  assert((M->isGlobalModule() || Loc.isValid()) &&
+ "setVisible expects a valid import location");
   if (isVisible(M))
 return;
 

diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 0cf79265b7448..8b3cd4211f88c 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2979,6 +2979,14 @@ void Sema::DeclareGlobalNewDelete() {
   if (getLangOpts().OpenCLCPlusPlus)
 return;
 
+  // C++ [basic.stc.dynamic.general]p2:
+  //   The library provides default definitions for the global allocation
+  //   and deallocation functions. Some global allocation and deallocation
+  //   functions are replaceable ([new.delete]); these are attached to the
+  //   global module ([module.unit]).
+  if (getLangOpts().CPlusPlusModules && getCurrentModule())
+PushGlobalModuleFragment(SourceLocation(), /*IsImplicit=*/true);
+
   // C++ [basic.std.dynamic]p2:
   //   [...] The following allocation and deallocation functions (18.4) are
   //   implicitly declared in global scope in each translation unit of a
@@ -3018,6 +3026,14 @@ void Sema::DeclareGlobalNewDelete() {
   
&PP.getIdentifierTable().get("bad_alloc"),
 nullptr);
 getStdBadAlloc()->setImplicit(true);
+
+// The implicitly declared "std::bad_alloc" should live in global module
+// fragment.
+if (GlobalModuleFragment) {
+  getStdBadAlloc()->setModuleOwnershipKind(
+  Decl::ModuleOwnershipKind::ReachableWhenImported);
+  getStdBadAlloc()->setLocalOwningModule(GlobalModuleFragment);
+}
   }
   if (!StdAlignValT && getLangOpts().AlignedAllocation) {
 // The "std::align_val_t" enum class has not yet been declared, so build it
@@ -3025,9 +3041,19 @@ void Sema::DeclareGlobalNewDelete() {
 auto *AlignValT = EnumDecl::Create(
 Context, getOrCreateStdNamespace(), SourceLocation(), SourceLocation(),
 &PP.getIdentifierTable().get("align_val_t"), nullptr, true, true, 
true);
+
+// The implicitly declared "std::align_val_t" should live in global module
+// fragment.
+if (GlobalModuleFragment) {
+  AlignValT->setModuleOwnershipKind(
+  Decl::ModuleOwnershipKind::ReachableWhenImported);
+  AlignValT->setLocalOwningModule(GlobalModuleFragment);
+}
+
 AlignValT->setIntegerType(Context.getSizeType());
 AlignValT->setPromotionType(Context.getSizeType());
 AlignValT->setImplicit(true);
+
 StdAlignValT = AlignValT;
   }
 
@@ -3069,6 +3095,9 @@ void Sema::DeclareGlobalNewDelete() {
   DeclareGlobalAllocationFunctions(OO_Array_New, VoidPtr, SizeT);
   DeclareGlobalAllocationFunctions(OO_Delete, Context.VoidTy, VoidPtr);
   DeclareGlobalAllocationFunctions(OO_Array_Delete, Context.VoidTy, VoidPtr);
+
+  if (getLangOpts().CPlusPlusModules && getCurrentModule())
+PopGlobalModuleFragment();
 }
 
 /// DeclareGlobalAllocationFunction - Declares a single implicit global
@@ -3137,6 +3166,22 @@ void 
Sema::DeclareGlobalAllocationFunction(DeclarationName Name,
   Alloc->addAttr(
   ReturnsNonNullAttr::CreateImplicit(Context, Alloc->getLocation()));
 
+   

[PATCH] D138010: [AArch64][ARM] add Armv8.9-a/Armv9.4-a identifier support

2022-11-15 Thread Ties Stuij via Phabricator via cfe-commits
stuij created this revision.
Herald added subscribers: hiraditya, kristof.beyls.
Herald added a project: All.
stuij requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, MaskRay.
Herald added projects: clang, LLVM.

For both ARM and AArch64 add support for specifying -march=armv8.9a/armv9.4a to
clang. Add backend plumbing like target parser and predicate support.

For a summary of Amv8.9/Armv9.4 features, see:
https://community.arm.com/arm-community-blogs/b/architectures-and-processors-blog/posts/arm-a-profile-architecture-2022

For detailed information, consult the Arm Architecture Reference Manual for
A-profile architecture:
https://developer.arm.com/documentation/ddi0487/latest/

People who contributed to this patch:

- Keith Walker
- Ties Stuij


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138010

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-v89a.c
  clang/test/Driver/aarch64-v94a.c
  clang/test/Driver/arm-cortex-cpus-1.c
  clang/test/Preprocessor/arm-target-features.c
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -31,10 +31,12 @@
 "armv8a",   "armv8l",  "armv8.1-a",  "armv8.1a","armv8.2-a",
 "armv8.2a", "armv8.3-a",   "armv8.3a",   "armv8.4-a",   "armv8.4a",
 "armv8.5-a","armv8.5a","armv8.6-a",  "armv8.6a","armv8.7-a",
-"armv8.7a", "armv8.8-a",   "armv8.8a",   "armv8-r", "armv8r",
-"armv8-m.base", "armv8m.base", "armv8-m.main",   "armv8m.main", "iwmmxt",
-"iwmmxt2",  "xscale",  "armv8.1-m.main", "armv9-a", "armv9",
-"armv9a",   "armv9.1-a",   "armv9.1a",   "armv9.2-a",   "armv9.2a",
+"armv8.7a", "armv8.8-a",   "armv8.8a",   "armv8.9-a",   "armv8.9a",
+"armv8-r",  "armv8r",  "armv8-m.base",   "armv8m.base", "armv8-m.main",
+"armv8m.main",  "iwmmxt",  "iwmmxt2","xscale",  "armv8.1-m.main",
+"armv9-a",  "armv9",   "armv9a", "armv9.1-a",   "armv9.1a",
+"armv9.2-a","armv9.2a","armv9.3-a",  "armv9.3a","armv9.4-a",
+"armv9.4a",
 };
 
 template 
@@ -509,6 +511,9 @@
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testARMArch("armv8.8-a", "generic", "v8.8a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(
+  testARMArch("armv8.9-a", "generic", "v8.9a",
+  ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(
   testARMArch("armv9-a", "generic", "v9a",
   ARMBuildAttrs::CPUArch::v9_A));
@@ -521,6 +526,9 @@
   EXPECT_TRUE(
   testARMArch("armv9.3-a", "generic", "v9.3a",
   ARMBuildAttrs::CPUArch::v9_A));
+  EXPECT_TRUE(
+  testARMArch("armv9.4-a", "generic", "v9.4a",
+  ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(
   testARMArch("armv8-r", "cortex-r52", "v8r",
   ARMBuildAttrs::CPUArch::v8_R));
@@ -851,10 +859,12 @@
 case ARM::ArchKind::ARMV8_6A:
 case ARM::ArchKind::ARMV8_7A:
 case ARM::ArchKind::ARMV8_8A:
+case ARM::ArchKind::ARMV8_9A:
 case ARM::ArchKind::ARMV9A:
 case ARM::ArchKind::ARMV9_1A:
 case ARM::ArchKind::ARMV9_2A:
 case ARM::ArchKind::ARMV9_3A:
+case ARM::ArchKind::ARMV9_4A:
   EXPECT_EQ(ARM::ProfileKind::A, ARM::parseArchProfile(ARMArch[i]));
   break;
 default:
@@ -1429,12 +1439,18 @@
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv8.8-a", "generic", "v8.8a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv8.9-a", "generic", "v8.9a",
+  ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv9-a", "generic", "v9a",
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv9.1-a", "generic", "v9.1a",
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv9.2-a", "generic", "v9.2a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv9.3-a", "generic

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

2022-11-15 Thread Dilshod Urazov via Phabricator via cfe-commits
urazoff updated this revision to Diff 475370.
urazoff added a comment.

Added test to show the advantage in AST dump.
Missing keywords are added in 'IsUnknownTypedefName', the function is static 
now.
'DisambiguatingWithExpression' check is added to narrow down the effect of the 
changes.


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

https://reviews.llvm.org/D137020

Files:
  clang/lib/Parse/ParseDecl.cpp
  clang/test/AST/ast-dump-recovery.c
  clang/test/Driver/types.c
  clang/test/Parser/CompoundStmtScope.c
  clang/test/Parser/opencl-atomics-cl20.cl
  clang/test/Parser/recovery.c
  clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
  clang/test/SemaOpenCL/invalid-device-enqueue-types-cl3.0.cl
  clang/test/SemaOpenCL/invalid-pipes-cl1.2.cl

Index: clang/test/SemaOpenCL/invalid-pipes-cl1.2.cl
===
--- clang/test/SemaOpenCL/invalid-pipes-cl1.2.cl
+++ clang/test/SemaOpenCL/invalid-pipes-cl1.2.cl
@@ -28,7 +28,7 @@
 void bar(void) {
  reserve_id_t r;
 #if defined(__OPENCL_C_VERSION__)
-// expected-error@-2 {{use of undeclared identifier 'reserve_id_t'}}
+// expected-error@-2 {{unknown type name 'reserve_id_t'}}
 #else
 // expected-error@-4 {{unknown type name 'reserve_id_t'}}
 #endif
Index: clang/test/SemaOpenCL/invalid-device-enqueue-types-cl3.0.cl
===
--- clang/test/SemaOpenCL/invalid-device-enqueue-types-cl3.0.cl
+++ clang/test/SemaOpenCL/invalid-device-enqueue-types-cl3.0.cl
@@ -5,8 +5,8 @@
   clk_event_t e;
   queue_t q;
 #ifndef __opencl_c_device_enqueue
-// expected-error@-3 {{use of undeclared identifier 'clk_event_t'}}
-// expected-error@-3 {{use of undeclared identifier 'queue_t'}}
+// expected-error@-3 {{unknown type name 'clk_event_t'}}
+// expected-error@-3 {{unknown type name 'queue_t'}}
 #else
 // expected-no-diagnostics
 #endif
Index: clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
===
--- clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
+++ clang/test/SemaOpenCL/intel-subgroup-avc-ext-types.cl
@@ -46,19 +46,19 @@
 // expected-error@-14 {{initializing '__private intel_sub_group_avc_ime_single_reference_streamin_t' with an expression of incompatible type '__private char'}}
 // expected-error@-14 {{initializing '__private intel_sub_group_avc_ime_dual_reference_streamin_t' with an expression of incompatible type 'int'}}
 #else
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_mce_payload_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_ime_payload_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_ref_payload_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_sic_payload_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_mce_result_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_ime_result_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_ref_result_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_sic_result_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_ime_result_single_reference_streamout_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_ime_result_dual_reference_streamout_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_ime_dual_reference_streamin_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_ime_single_reference_streamin_t'}}
-// expected-error@-28 {{use of undeclared identifier 'intel_sub_group_avc_ime_dual_reference_streamin_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_mce_payload_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_ime_payload_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_ref_payload_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_sic_payload_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_mce_result_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_ime_result_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_ref_result_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_sic_result_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_ime_result_single_reference_streamout_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_ime_result_dual_reference_streamout_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_ime_dual_reference_streamin_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_ime_single_reference_streamin_t'}}
+// expected-error@-28 {{unknown type name 'intel_sub_group_avc_ime_dual_reference_streamin_t'}}
 #endif
 }
 
@@ -75,13 +75,12 @@
 // expected-error@-5 

[clang] ae59131 - [clang][Tooling] Make the filename behaviour consistent

2022-11-15 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2022-11-15T10:47:52+01:00
New Revision: ae59131d3ef311fb4b1e50627c6457be00e60dc9

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

LOG: [clang][Tooling] Make the filename behaviour consistent

Dotdots were removed only when converting a relative path to absolute
one. This patch makes the behaviour consistent for absolute file paths by
removing them in that case as well. Also updates the documentation to mention
the behaviour.

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

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

Added: 


Modified: 
clang/lib/Tooling/JSONCompilationDatabase.cpp
clang/unittests/Tooling/CompilationDatabaseTest.cpp

Removed: 




diff  --git a/clang/lib/Tooling/JSONCompilationDatabase.cpp 
b/clang/lib/Tooling/JSONCompilationDatabase.cpp
index 5e18d7a576c08..158b9a5473eac 100644
--- a/clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ b/clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -419,14 +419,13 @@ bool JSONCompilationDatabase::parse(std::string 
&ErrorMessage) {
 SmallString<128> NativeFilePath;
 if (llvm::sys::path::is_relative(FileName)) {
   SmallString<8> DirectoryStorage;
-  SmallString<128> AbsolutePath(
-  Directory->getValue(DirectoryStorage));
+  SmallString<128> AbsolutePath(Directory->getValue(DirectoryStorage));
   llvm::sys::path::append(AbsolutePath, FileName);
-  llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/ true);
   llvm::sys::path::native(AbsolutePath, NativeFilePath);
 } else {
   llvm::sys::path::native(FileName, NativeFilePath);
 }
+llvm::sys::path::remove_dots(NativeFilePath, /*remove_dot_dot=*/true);
 auto Cmd = CompileCommandRef(Directory, File, *Command, Output);
 IndexByFile[NativeFilePath].push_back(Cmd);
 AllCommands.push_back(Cmd);

diff  --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp 
b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
index 3314ecd36c193..89763f9f8d52c 100644
--- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -92,16 +92,32 @@ TEST(JSONCompilationDatabase, GetAllFiles) {
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/file1", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
+  llvm::sys::path::native("//net/dir/file3", PathStorage);
+  expected_files.push_back(std::string(PathStorage.str()));
   EXPECT_EQ(expected_files,
-getAllFiles("[{\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"../file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file2\"}]",
+getAllFiles(R"json(
+[
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "../file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file2"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "//net/dir/foo/../file3"
+  }
+])json",
 ErrorMessage, JSONCommandLineSyntax::Gnu))
   << ErrorMessage;
 }



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


[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

2022-11-15 Thread Kadir Cetinkaya 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 rGae59131d3ef3: [clang][Tooling] Make the filename behaviour 
consistent (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137962

Files:
  clang/lib/Tooling/JSONCompilationDatabase.cpp
  clang/unittests/Tooling/CompilationDatabaseTest.cpp


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -92,16 +92,32 @@
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/file1", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
+  llvm::sys::path::native("//net/dir/file3", PathStorage);
+  expected_files.push_back(std::string(PathStorage.str()));
   EXPECT_EQ(expected_files,
-getAllFiles("[{\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"../file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file2\"}]",
+getAllFiles(R"json(
+[
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "../file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file2"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "//net/dir/foo/../file3"
+  }
+])json",
 ErrorMessage, JSONCommandLineSyntax::Gnu))
   << ErrorMessage;
 }
Index: clang/lib/Tooling/JSONCompilationDatabase.cpp
===
--- clang/lib/Tooling/JSONCompilationDatabase.cpp
+++ clang/lib/Tooling/JSONCompilationDatabase.cpp
@@ -419,14 +419,13 @@
 SmallString<128> NativeFilePath;
 if (llvm::sys::path::is_relative(FileName)) {
   SmallString<8> DirectoryStorage;
-  SmallString<128> AbsolutePath(
-  Directory->getValue(DirectoryStorage));
+  SmallString<128> AbsolutePath(Directory->getValue(DirectoryStorage));
   llvm::sys::path::append(AbsolutePath, FileName);
-  llvm::sys::path::remove_dots(AbsolutePath, /*remove_dot_dot=*/ true);
   llvm::sys::path::native(AbsolutePath, NativeFilePath);
 } else {
   llvm::sys::path::native(FileName, NativeFilePath);
 }
+llvm::sys::path::remove_dots(NativeFilePath, /*remove_dot_dot=*/true);
 auto Cmd = CompileCommandRef(Directory, File, *Command, Output);
 IndexByFile[NativeFilePath].push_back(Cmd);
 AllCommands.push_back(Cmd);


Index: clang/unittests/Tooling/CompilationDatabaseTest.cpp
===
--- clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -92,16 +92,32 @@
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/file1", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
+  llvm::sys::path::native("//net/dir/file3", PathStorage);
+  expected_files.push_back(std::string(PathStorage.str()));
   EXPECT_EQ(expected_files,
-getAllFiles("[{\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"../file1\"},"
-" {\"directory\":\"//net/dir\","
-"\"command\":\"command\","
-"\"file\":\"file2\"}]",
+getAllFiles(R"json(
+[
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "../file1"
+  },
+  {
+"directory": "//net/dir",
+"command": "command",
+"file": "file2"
+  },
+  {
+"directory":

[PATCH] D138010: [AArch64][ARM] add Armv8.9-a/Armv9.4-a identifier support

2022-11-15 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas added inline comments.



Comment at: llvm/include/llvm/Support/AArch64TargetParser.def:70-76
+AARCH64_ARCH("armv8.9-a", ARMV8_9A, "8.9-A", "v8.9a",
+ ARMBuildAttrs::CPUArch::v8_A, FK_CRYPTO_NEON_FP_ARMV8,
+ (AArch64::AEK_CRC | AArch64::AEK_FP |
+  AArch64::AEK_SIMD | AArch64::AEK_RAS | AArch64::AEK_LSE |
+  AArch64::AEK_RDM | AArch64::AEK_RCPC | AArch64::AEK_DOTPROD |
+  AArch64::AEK_SM4 | AArch64::AEK_SHA3 | AArch64::AEK_BF16 |
+  AArch64::AEK_SHA2 | AArch64::AEK_AES | AArch64::AEK_I8MM))

The new entries need to be updated, as D137924 has recently landed removing a 
couple of unused fields from `AARCH64_ARCH`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138010

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


[PATCH] D138010: [AArch64][ARM] add Armv8.9-a/Armv9.4-a identifier support

2022-11-15 Thread Tomas Matheson via Phabricator via cfe-commits
tmatheson added inline comments.



Comment at: llvm/include/llvm/Support/ARMTargetParser.def:127
+  ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_CRC | ARM::AEK_RAS |
+  ARM::AEK_DOTPROD | ARM::AEK_BF16 | ARM::AEK_I8MM))
 ARM_ARCH("armv9-a", ARMV9A, "9-A", "v9a",

No ARM::AEK_SHA2 | ARM::AEK_AES? Or does 8.8 need updated?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138010

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


[PATCH] D137865: [clang-format][NFC] Improve documentation on ReflowComments

2022-11-15 Thread Emilia Dreamer via Phabricator via cfe-commits
rymiel added inline comments.



Comment at: clang/include/clang/Format/Format.h:3073-3076
+  /// If ``true``, clang-format will attempt to re-flow comments. That is it
+  /// will touch a comment and *reflow* long comments into new lines, trying to
+  /// obey the ``ColumnLimit``. ``/*`` comments will get a leading ``*`` on the
+  /// new lines.

HazardyKnusperkeks wrote:
> owenpan wrote:
> > Let's leave it as is because the new lines don't always get a leading `*`:
> > ```
> > $ cat foo.cpp
> > /* The LLVM Project is a collection of modular and reusable compiler and 
> > toolchain
> >technologies. */
> > $ clang-format -style='{ReflowComments: true}' foo.cpp
> > /* The LLVM Project is a collection of modular and reusable compiler and
> >toolchain technologies. */
> > ```
> That was from a short glimpse on the code.
> Adapted the text.
> Let's leave it as is because the new lines don't always get a leading `*`:

I suppose this also ties into https://github.com/llvm/llvm-project/issues/58710




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

https://reviews.llvm.org/D137865

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


[PATCH] D136354: [Driver] Enable nested configuration files

2022-11-15 Thread Serge Pavlov via Phabricator via cfe-commits
sepavloff updated this revision to Diff 475400.
sepavloff added a comment.

Missed changes in tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136354

Files:
  clang/docs/UsersManual.rst
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/Inputs/config-6.cfg
  clang/test/Driver/config-file-errs.c
  clang/test/Driver/config-file.c
  clang/unittests/Driver/ToolChainTest.cpp
  llvm/lib/Support/CommandLine.cpp

Index: llvm/lib/Support/CommandLine.cpp
===
--- llvm/lib/Support/CommandLine.cpp
+++ llvm/lib/Support/CommandLine.cpp
@@ -1191,27 +1191,44 @@
 return Error::success();
 
   StringRef BasePath = llvm::sys::path::parent_path(FName);
-  for (auto I = NewArgv.begin(), E = NewArgv.end(); I != E; ++I) {
-const char *&Arg = *I;
-if (Arg == nullptr)
+  for (const char *&Arg : NewArgv) {
+if (!Arg)
   continue;
 
 // Substitute  with the file's base path.
 if (InConfigFile)
   ExpandBasePaths(BasePath, Saver, Arg);
 
-// Get expanded file name.
-StringRef FileName(Arg);
-if (!FileName.consume_front("@"))
-  continue;
-if (!llvm::sys::path::is_relative(FileName))
+// Discover the case, when argument should be transformed into '@file' and
+// evaluate 'file' for it.
+StringRef ArgStr(Arg);
+StringRef FileName;
+bool ConfigInclusion = false;
+if (ArgStr.consume_front("@")) {
+  FileName = ArgStr;
+  if (!llvm::sys::path::is_relative(FileName))
+continue;
+} else if (ArgStr.consume_front("--config=")) {
+  FileName = ArgStr;
+  ConfigInclusion = true;
+} else {
   continue;
+}
 
 // Update expansion construct.
 SmallString<128> ResponseFile;
 ResponseFile.push_back('@');
-ResponseFile.append(BasePath);
-llvm::sys::path::append(ResponseFile, FileName);
+if (ConfigInclusion && !llvm::sys::path::has_parent_path(FileName)) {
+  SmallString<128> FilePath;
+  if (!findConfigFile(FileName, FilePath))
+return createStringError(
+std::make_error_code(std::errc::no_such_file_or_directory),
+"cannot not find configuration file: " + FileName);
+  ResponseFile.append(FilePath);
+} else {
+  ResponseFile.append(BasePath);
+  llvm::sys::path::append(ResponseFile, FileName);
+}
 Arg = Saver.save(ResponseFile.str()).data();
   }
   return Error::success();
Index: clang/unittests/Driver/ToolChainTest.cpp
===
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -652,4 +652,56 @@
 #undef INCLUDED1
 }
 
+TEST(ToolChainTest, NestedConfigFile) {
+  IntrusiveRefCntPtr DiagOpts = new DiagnosticOptions();
+  IntrusiveRefCntPtr DiagID(new DiagnosticIDs());
+  struct TestDiagnosticConsumer : public DiagnosticConsumer {};
+  DiagnosticsEngine Diags(DiagID, &*DiagOpts, new TestDiagnosticConsumer);
+  IntrusiveRefCntPtr FS(
+  new llvm::vfs::InMemoryFileSystem);
+
+#ifdef _WIN32
+  const char *TestRoot = "C:\\";
+#else
+  const char *TestRoot = "/";
+#endif
+  FS->setCurrentWorkingDirectory(TestRoot);
+
+  FS->addFile("/opt/sdk/root.cfg", 0,
+  llvm::MemoryBuffer::getMemBuffer("--config=platform.cfg\n"));
+  FS->addFile("/opt/sdk/platform.cfg", 0,
+  llvm::MemoryBuffer::getMemBuffer("--sysroot=/platform-sys\n"));
+  FS->addFile("/home/test/bin/platform.cfg", 0,
+  llvm::MemoryBuffer::getMemBuffer("--sysroot=/platform-bin\n"));
+
+  SmallString<128> ClangExecutable("/home/test/bin/clang");
+  FS->makeAbsolute(ClangExecutable);
+
+  // User file is absent - use system definitions.
+  {
+Driver TheDriver(ClangExecutable, "arm-linux-gnueabi", Diags,
+ "clang LLVM compiler", FS);
+std::unique_ptr C(TheDriver.BuildCompilation(
+{"/home/test/bin/clang", "--config", "root.cfg",
+ "--config-system-dir=/opt/sdk", "--config-user-dir=/home/test/sdk"}));
+ASSERT_TRUE(C);
+ASSERT_FALSE(C->containsError());
+EXPECT_EQ("/platform-sys", TheDriver.SysRoot);
+  }
+
+  // User file overrides system definitions.
+  FS->addFile("/home/test/sdk/platform.cfg", 0,
+  llvm::MemoryBuffer::getMemBuffer("--sysroot=/platform-user\n"));
+  {
+Driver TheDriver(ClangExecutable, "arm-linux-gnueabi", Diags,
+ "clang LLVM compiler", FS);
+std::unique_ptr C(TheDriver.BuildCompilation(
+{"/home/test/bin/clang", "--config", "root.cfg",
+ "--config-system-dir=/opt/sdk", "--config-user-dir=/home/test/sdk"}));
+ASSERT_TRUE(C);
+ASSERT_FALSE(C->containsError());
+EXPECT_EQ("/platform-user", TheDriver.SysRoot);
+  }
+}
+
 } // end anonymous namespace.
Index: clang/test/Driver/config-file.c
===

[PATCH] D137851: [OPENMP]Initial support for at clause

2022-11-15 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2168
+Actions.ActOnOpenMPErrorDirective(Clauses, StartLoc, SourceLocation(),
+  /*InExcontext = */ false);
+break;

InExContext



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11036
+  OMPExecutableDirective::getClausesOfKind(Clauses);
+  const OMPAtClause *AtC = AtClauses.empty() ? nullptr : (*AtClauses.begin());
+

`AtClauses.front()`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137851

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


[PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

2022-11-15 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

This breaks tests on windows: http://45.33.8.238/win/69993/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137962

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


[PATCH] D137572: [AVR][Clang] Implement __AVR_HAVE_*__ macros

2022-11-15 Thread Ayke via Phabricator via cfe-commits
aykevl updated this revision to Diff 475419.
aykevl added a comment.

- Fix ArchHas3BytePC to remove arch 107 (of which no chips have more than 128kB 
flash)
- Fix ArchHasELPM/ArchHasELPMX to add arch 102 (it does support elpm even 
though avr-gcc claims it doesn't).
- Add notes where the provided macros may not be correct in all cases. It's 
still better than avr-gcc though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137572

Files:
  clang/lib/Basic/Targets/AVR.cpp
  clang/test/Preprocessor/avr-atmega328p.c
  clang/test/Preprocessor/avr-attiny104.c

Index: clang/test/Preprocessor/avr-attiny104.c
===
--- clang/test/Preprocessor/avr-attiny104.c
+++ clang/test/Preprocessor/avr-attiny104.c
@@ -4,5 +4,6 @@
 // CHECK: #define __AVR 1
 // CHECK: #define __AVR_ARCH__ 100
 // CHECK: #define __AVR_ATtiny104__ 1
+// CHECK-NOT: #define __AVR_HAVE_MUL__ 1
 // CHECK: #define __AVR__ 1
 // CHECK: #define __ELF__ 1
Index: clang/test/Preprocessor/avr-atmega328p.c
===
--- clang/test/Preprocessor/avr-atmega328p.c
+++ clang/test/Preprocessor/avr-atmega328p.c
@@ -4,5 +4,9 @@
 // CHECK: #define __AVR 1
 // CHECK: #define __AVR_ARCH__ 5
 // CHECK: #define __AVR_ATmega328P__ 1
+// CHECK-NOT: #define __AVR_HAVE_EIJMP_EICALL__
+// CHECK: #define __AVR_HAVE_LPMX__ 1
+// CHECK: #define __AVR_HAVE_MOVW__ 1
+// CHECK: #define __AVR_HAVE_MUL__ 1
 // CHECK: #define __AVR__ 1
 // CHECK: #define __ELF__ 1
Index: clang/lib/Basic/Targets/AVR.cpp
===
--- clang/lib/Basic/Targets/AVR.cpp
+++ clang/lib/Basic/Targets/AVR.cpp
@@ -12,6 +12,7 @@
 
 #include "AVR.h"
 #include "clang/Basic/MacroBuilder.h"
+#include "llvm/ADT/StringSwitch.h"
 
 using namespace clang;
 using namespace clang::targets;
@@ -348,6 +349,58 @@
 } // namespace targets
 } // namespace clang
 
+static bool ArchHasELPM(StringRef Arch) {
+  return llvm::StringSwitch(Arch)
+.Cases("31", "51", "6", true)
+.Cases("102", "104", "105", "106", "107", true)
+.Default(false);
+}
+
+static bool ArchHasELPMX(StringRef Arch) {
+  return llvm::StringSwitch(Arch)
+.Cases("51", "6", true)
+.Cases("102", "104", "105", "106", "107", true)
+.Default(false);
+}
+
+static bool ArchHasMOVW(StringRef Arch) {
+  return llvm::StringSwitch(Arch)
+.Cases("25", "35", "4", "5", "51", "6", true)
+.Cases("102", "103", "104", "105", "106", "107", true)
+.Default(false);
+}
+
+static bool ArchHasLPMX(StringRef Arch) {
+  return ArchHasMOVW(Arch); // same architectures
+}
+
+static bool ArchHasMUL(StringRef Arch) {
+  return llvm::StringSwitch(Arch)
+.Cases("4", "5", "51", "6", true)
+.Cases("102", "103", "104", "105", "106", "107", true)
+.Default(false);
+}
+
+static bool ArchHasJMPCALL(StringRef Arch) {
+  return llvm::StringSwitch(Arch)
+.Cases("3", "31", "35", "5", "51", "6", true)
+.Cases("102", "103", "104", "105", "106", "107", true)
+.Default(false);
+}
+
+static bool ArchHas3BytePC(StringRef Arch) {
+  // These devices have more than 128kB of program memory.
+  // Note:
+  //   - Not fully correct for arch 106: only about half the chips have more
+  // than 128kB program memory and therefore a 3 byte PC.
+  //   - Doesn't match GCC entirely: avr-gcc thinks arch 107 goes beyond 128kB
+  // but in fact it doesn't.
+  return llvm::StringSwitch(Arch)
+.Case("6", true)
+.Case("106", true)
+.Default(false);
+}
+
 bool AVRTargetInfo::isValidCPUName(StringRef Name) const {
   return llvm::any_of(
   AVRMcus, [&](const MCUInfo &Info) { return Info.Name == Name; });
@@ -390,6 +443,30 @@
 
   Builder.defineMacro("__AVR_ARCH__", Arch);
 
+  // TODO: perhaps we should use the information from AVRDevices.td instead?
+  if (ArchHasELPM(Arch))
+Builder.defineMacro("__AVR_HAVE_ELPM__");
+  if (ArchHasELPMX(Arch))
+Builder.defineMacro("__AVR_HAVE_ELPMX__");
+  if (ArchHasMOVW(Arch))
+Builder.defineMacro("__AVR_HAVE_MOVW__");
+  if (ArchHasLPMX(Arch))
+Builder.defineMacro("__AVR_HAVE_LPMX__");
+  if (ArchHasMUL(Arch))
+Builder.defineMacro("__AVR_HAVE_MUL__");
+  if (ArchHasJMPCALL(Arch))
+Builder.defineMacro("__AVR_HAVE_JMP_CALL__");
+  if (ArchHas3BytePC(Arch)) {
+// Note: some devices do support eijmp/eicall even though this macro isn't
+// set. This is the case if they have less than 128kB flash and so
+// eijmp/eicall isn't very useful anyway. (This matches gcc, although it's
+// debatable whether we should be bug-compatible in this case).
+Builder.defineMacro("__AVR_HAVE_EIJMP_EICALL__");
+Builder.defineMacro("__AVR_3_BYTE_PC__");
+  } else {
+Builder.defineMacro("__AVR_2_BYTE_PC__");
+  }
+
   if (NumFlashBanks >= 1)
 Builder.defineMacro("__flash", "__attribute__((address_space(1)))");
   if (NumFla

[PATCH] D137572: [AVR][Clang] Implement __AVR_HAVE_*__ macros

2022-11-15 Thread Ayke via Phabricator via cfe-commits
aykevl added inline comments.



Comment at: clang/lib/Basic/Targets/AVR.cpp:355
+.Cases("31", "51", "6", true)
+.Cases("104", "105", "106", "107", true)
+.Default(false);

benshi001 wrote:
> benshi001 wrote:
> > ATxmega16a4 with family code 102 also supports ELPM. Could you please make 
> > a careful check on ELPM and all other features? 
> > 
> > Generally speaking I am very glad to have this patch committed, since it 
> > fixes
> > 
> > https://github.com/llvm/llvm-project/issues/56157
> > 
> I suggest that you can make your code in accordance with 
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/AVR/AVRDevices.td
I checked all 102 family chips and indeed they all support ELPM. This looks 
like a bug in avr-gcc, which claims these devices don't support ELPM.

I did check AVRDevices.td but apparently I made some mistakes. I've checked 
again and fixed a few small issues. There are still some left but they are 
difficult to fix without reading AVRDevices.td directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137572

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


[clang] eed1f33 - [clang][Tooling] Sort filenames in test

2022-11-15 Thread Kadir Cetinkaya via cfe-commits

Author: Kadir Cetinkaya
Date: 2022-11-15T13:31:40+01:00
New Revision: eed1f337a1fbf8c1e7e53a6e1d0dfd4eb800aa08

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

LOG: [clang][Tooling] Sort filenames in test

Added: 


Modified: 
clang/unittests/Tooling/CompilationDatabaseTest.cpp

Removed: 




diff  --git a/clang/unittests/Tooling/CompilationDatabaseTest.cpp 
b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
index 89763f9f8d52..fb8c10df38e8 100644
--- a/clang/unittests/Tooling/CompilationDatabaseTest.cpp
+++ b/clang/unittests/Tooling/CompilationDatabaseTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Support/TargetSelect.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace tooling {
@@ -62,7 +63,9 @@ static std::vector getAllFiles(StringRef 
JSONDatabase,
 ADD_FAILURE() << ErrorMessage;
 return std::vector();
   }
-  return Database->getAllFiles();
+  auto Result = Database->getAllFiles();
+  std::sort(Result.begin(), Result.end());
+  return Result;
 }
 
 static std::vector
@@ -90,10 +93,10 @@ TEST(JSONCompilationDatabase, GetAllFiles) {
   expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/dir/file2", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
-  llvm::sys::path::native("//net/file1", PathStorage);
-  expected_files.push_back(std::string(PathStorage.str()));
   llvm::sys::path::native("//net/dir/file3", PathStorage);
   expected_files.push_back(std::string(PathStorage.str()));
+  llvm::sys::path::native("//net/file1", PathStorage);
+  expected_files.push_back(std::string(PathStorage.str()));
   EXPECT_EQ(expected_files,
 getAllFiles(R"json(
 [



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


Re: [PATCH] D137962: [clang][Tooling] Make the filename behaviour consistent

2022-11-15 Thread Kadir Çetinkaya via cfe-commits
oops, should be fixed with eed1f337a1fbf8c1e7e53a6e1d0dfd4eb800aa08

On Tue, Nov 15, 2022 at 1:03 PM Nico Weber via Phabricator <
revi...@reviews.llvm.org> wrote:

> thakis added a comment.
>
> This breaks tests on windows: http://45.33.8.238/win/69993/step_7.txt
>
> Please take a look and revert for now if it takes a while to fix.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D137962/new/
>
> https://reviews.llvm.org/D137962
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] 00b1f7a - Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI

2022-11-15 Thread Alex Richardson via cfe-commits

Author: Alex Richardson
Date: 2022-11-15T12:42:19Z
New Revision: 00b1f7a6ab2aecd2309e2faebde18a11309c6181

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

LOG: Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI

I noticed that the values for __{CLANG,GCC}_ATOMIC_POINTER_LOCK_FREE were
incorrectly set to 1 instead of two in downstream CHERI targets because
pointers are handled specially there. While fixing this downstream, I
noticed that the existing code could be refactored to use
TargetInfo::hasBuiltinAtomic instead of repeating the almost identical
logic. In theory there could be a difference here since hasBuiltinAtomic() also
returns true for types less than 1 char in size, but since
InitializePredefinedMacros() never passes such a value this change should
not introduce any functional changes.

Reviewed By: rprichard, efriedma

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

Added: 


Modified: 
clang/lib/Frontend/InitPreprocessor.cpp

Removed: 




diff  --git a/clang/lib/Frontend/InitPreprocessor.cpp 
b/clang/lib/Frontend/InitPreprocessor.cpp
index 8e0a42c644799..bdff0bd04f290 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -299,12 +299,12 @@ static void DefineFastIntType(unsigned TypeWidth, bool 
IsSigned,
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, const TargetInfo &TI) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
   // Note: we do not need to check alignment since _Atomic(T) is always
   // appropriately-aligned in clang.
-  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
+  if (TI.hasBuiltinAtomic(TypeWidth, TypeWidth))
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1150,11 +1150,9 @@ static void InitializePredefinedMacros(const TargetInfo 
&TI,
 
   auto addLockFreeMacros = [&](const llvm::Twine &Prefix) {
 // Used by libc++ and libstdc++ to implement ATOMIC__LOCK_FREE.
-unsigned InlineWidthBits = TI.getMaxAtomicInlineWidth();
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) 
\
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", 
\
-  getLockFreeValue(TI.get##Type##Width(),  
\
-   InlineWidthBits));
+  getLockFreeValue(TI.get##Type##Width(), TI));
 DEFINE_LOCK_FREE_MACRO(BOOL, Bool);
 DEFINE_LOCK_FREE_MACRO(CHAR, Char);
 if (LangOpts.Char8)
@@ -1167,8 +1165,7 @@ static void InitializePredefinedMacros(const TargetInfo 
&TI,
 DEFINE_LOCK_FREE_MACRO(LONG, Long);
 DEFINE_LOCK_FREE_MACRO(LLONG, LongLong);
 Builder.defineMacro(Prefix + "POINTER_LOCK_FREE",
-getLockFreeValue(TI.getPointerWidth(0),
- InlineWidthBits));
+getLockFreeValue(TI.getPointerWidth(0), TI));
 #undef DEFINE_LOCK_FREE_MACRO
   };
   addLockFreeMacros("__CLANG_ATOMIC_");



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


[PATCH] D135142: Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI

2022-11-15 Thread Alexander Richardson 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 rG00b1f7a6ab2a: Use TI.hasBuiltinAtomic() when setting 
ATOMIC_*_LOCK_FREE values. NFCI (authored by arichardson).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135142

Files:
  clang/lib/Frontend/InitPreprocessor.cpp


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -299,12 +299,12 @@
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, const TargetInfo &TI) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
   // Note: we do not need to check alignment since _Atomic(T) is always
   // appropriately-aligned in clang.
-  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
+  if (TI.hasBuiltinAtomic(TypeWidth, TypeWidth))
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1150,11 +1150,9 @@
 
   auto addLockFreeMacros = [&](const llvm::Twine &Prefix) {
 // Used by libc++ and libstdc++ to implement ATOMIC__LOCK_FREE.
-unsigned InlineWidthBits = TI.getMaxAtomicInlineWidth();
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) 
\
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", 
\
-  getLockFreeValue(TI.get##Type##Width(),  
\
-   InlineWidthBits));
+  getLockFreeValue(TI.get##Type##Width(), TI));
 DEFINE_LOCK_FREE_MACRO(BOOL, Bool);
 DEFINE_LOCK_FREE_MACRO(CHAR, Char);
 if (LangOpts.Char8)
@@ -1167,8 +1165,7 @@
 DEFINE_LOCK_FREE_MACRO(LONG, Long);
 DEFINE_LOCK_FREE_MACRO(LLONG, LongLong);
 Builder.defineMacro(Prefix + "POINTER_LOCK_FREE",
-getLockFreeValue(TI.getPointerWidth(0),
- InlineWidthBits));
+getLockFreeValue(TI.getPointerWidth(0), TI));
 #undef DEFINE_LOCK_FREE_MACRO
   };
   addLockFreeMacros("__CLANG_ATOMIC_");


Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -299,12 +299,12 @@
 
 /// Get the value the ATOMIC_*_LOCK_FREE macro should have for a type with
 /// the specified properties.
-static const char *getLockFreeValue(unsigned TypeWidth, unsigned InlineWidth) {
+static const char *getLockFreeValue(unsigned TypeWidth, const TargetInfo &TI) {
   // Fully-aligned, power-of-2 sizes no larger than the inline
   // width will be inlined as lock-free operations.
   // Note: we do not need to check alignment since _Atomic(T) is always
   // appropriately-aligned in clang.
-  if ((TypeWidth & (TypeWidth - 1)) == 0 && TypeWidth <= InlineWidth)
+  if (TI.hasBuiltinAtomic(TypeWidth, TypeWidth))
 return "2"; // "always lock free"
   // We cannot be certain what operations the lib calls might be
   // able to implement as lock-free on future processors.
@@ -1150,11 +1150,9 @@
 
   auto addLockFreeMacros = [&](const llvm::Twine &Prefix) {
 // Used by libc++ and libstdc++ to implement ATOMIC__LOCK_FREE.
-unsigned InlineWidthBits = TI.getMaxAtomicInlineWidth();
 #define DEFINE_LOCK_FREE_MACRO(TYPE, Type) \
   Builder.defineMacro(Prefix + #TYPE "_LOCK_FREE", \
-  getLockFreeValue(TI.get##Type##Width(),  \
-   InlineWidthBits));
+  getLockFreeValue(TI.get##Type##Width(), TI));
 DEFINE_LOCK_FREE_MACRO(BOOL, Bool);
 DEFINE_LOCK_FREE_MACRO(CHAR, Char);
 if (LangOpts.Char8)
@@ -1167,8 +1165,7 @@
 DEFINE_LOCK_FREE_MACRO(LONG, Long);
 DEFINE_LOCK_FREE_MACRO(LLONG, LongLong);
 Builder.defineMacro(Prefix + "POINTER_LOCK_FREE",
-getLockFreeValue(TI.getPointerWidth(0),
- InlineWidthBits));
+getLockFreeValue(TI.getPointerWidth(0), TI));
 #undef DEFINE_LOCK_FREE_MACRO
   };
   addLockFreeMacros("__CLANG_ATOMIC_");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: How can Autoconf help with the transition to stricter compilation defaults?

2022-11-15 Thread Sam James via cfe-commits


> On 13 Nov 2022, at 00:43, Paul Eggert  wrote:
> 
> On 2022-11-11 07:11, Aaron Ballman wrote:
>> We believe the runtime behavior is sufficiently dangerous to
>> warrant a conservative view that any call to a function will be a call
>> that gets executed at runtime, hence a definitive signature mismatch
>> is something we feel comfortable diagnosing (in some form) by default.
> 
> As long as these diagnostics by default do not cause the compiler to exit 
> with nonzero status, we should be OK with Autoconf-generated 'configure' 
> scripts. Although there will be problems with people who run "./configure 
> CFLAGS='-Werror'", that sort of usage has always been problematic and 
> unsupported by Autoconf, so we can simply continue to tell people "don't do 
> that".
> 

Is there somewhere in the autoconf docs we actually say this?

I've seen a few instances of folks adding it themselves very
early in their configure scripts (which is a pain for distros
anyway) which then ends up affecting the rest.


signature.asc
Description: Message signed with OpenPGP
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


Re: How can Autoconf help with the transition to stricter compilation defaults?

2022-11-15 Thread Zack Weinberg via cfe-commits
On Tue, Nov 15, 2022, at 12:03 AM, Sam James wrote:
>> On 13 Nov 2022, at 00:43, Paul Eggert  wrote:
>> 
>> Although there will be problems with people who run "./configure 
>> CFLAGS='-Werror'", that sort of usage has always been problematic and 
>> unsupported by Autoconf, so we can simply continue to tell people "don't do 
>> that".
>> 
>
> Is there somewhere in the autoconf docs we actually say this?
>
> I've seen a few instances of folks adding it themselves very
> early in their configure scripts (which is a pain for distros
> anyway) which then ends up affecting the rest.

It's mentioned in the NEWS entry for 2.70: 
https://git.savannah.gnu.org/cgit/autoconf.git/tree/NEWS#n170

It should be discussed in the actual manual as well, but I've been reluctant to 
add anything about warnings to the manual as long as Autoconf proper doesn't 
have any support for controlling warnings.

Note that there have been bug reports for cases where running builds with more 
warnings than configure uses (and I think also -Werror), means that configure 
checks spuriously succeed (i.e. configure reports that something is available, 
but then you get compiler errors on the code that tries to use it).  I can't 
remember any concrete examples right now, though.

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


[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

2022-11-15 Thread v1nh1shungry via Phabricator via cfe-commits
v1nh1shungry created this revision.
v1nh1shungry added a reviewer: tom-anders.
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.

Existing version ignores symbols declared in an inline namespace `ns`
when removing `using namespace ns`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138028

Files:
  clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
  clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -264,6 +264,17 @@
   }
   
   int main() { 1.5_w; }
+)cpp"},
+  {
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  using namespace a::[[b]];
+  int main() { foobar(); }
+)cpp",
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  
+  int main() { a::b::foobar(); }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -89,16 +89,15 @@
   return Node->Parent && Node->Parent->ASTNode.get();
 }
 
-// Returns the first visible context that contains this DeclContext.
-// For example: Returns ns1 for S1 and a.
-// namespace ns1 {
-// inline namespace ns2 { struct S1 {}; }
-// enum E { a, b, c, d };
-// }
-const DeclContext *visibleContext(const DeclContext *D) {
-  while (D->isInlineNamespace() || D->isTransparentContext())
+// Return true if `LHS` is declared in `RHS`
+bool declareIn(const NamedDecl *LHS, const DeclContext *RHS) {
+  const auto *D = LHS->getDeclContext();
+  while (D->isInlineNamespace() || D->isTransparentContext()) {
+if (D->Equals(RHS))
+  return true;
 D = D->getParent();
-  return D;
+  }
+  return D->Equals(RHS);
 }
 
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
@@ -152,8 +151,7 @@
 return; // This reference is already qualified.
 
   for (auto *T : Ref.Targets) {
-if (!visibleContext(T->getDeclContext())
- ->Equals(TargetDirective->getNominatedNamespace()))
+if (!declareIn(T, TargetDirective->getNominatedNamespace()))
   return;
 auto Kind = T->getDeclName().getNameKind();
 // Avoid adding qualifiers before operators, e.g.


Index: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
===
--- clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
+++ clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp
@@ -264,6 +264,17 @@
   }
   
   int main() { 1.5_w; }
+)cpp"},
+  {
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  using namespace a::[[b]];
+  int main() { foobar(); }
+)cpp",
+  R"cpp(
+  namespace a { inline namespace b { void foobar(); } }
+  
+  int main() { a::b::foobar(); }
 )cpp"}};
   for (auto C : Cases)
 EXPECT_EQ(C.second, apply(C.first)) << C.first;
Index: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp
@@ -89,16 +89,15 @@
   return Node->Parent && Node->Parent->ASTNode.get();
 }
 
-// Returns the first visible context that contains this DeclContext.
-// For example: Returns ns1 for S1 and a.
-// namespace ns1 {
-// inline namespace ns2 { struct S1 {}; }
-// enum E { a, b, c, d };
-// }
-const DeclContext *visibleContext(const DeclContext *D) {
-  while (D->isInlineNamespace() || D->isTransparentContext())
+// Return true if `LHS` is declared in `RHS`
+bool declareIn(const NamedDecl *LHS, const DeclContext *RHS) {
+  const auto *D = LHS->getDeclContext();
+  while (D->isInlineNamespace() || D->isTransparentContext()) {
+if (D->Equals(RHS))
+  return true;
 D = D->getParent();
-  return D;
+  }
+  return D->Equals(RHS);
 }
 
 bool RemoveUsingNamespace::prepare(const Selection &Inputs) {
@@ -152,8 +151,7 @@
 return; // This reference is already qualified.
 
   for (auto *T : Ref.Targets) {
-if (!visibleContext(T->getDeclContext())
-   

[PATCH] D138028: [clangd] Fix action `RemoveUsingNamespace` for inline namespace

2022-11-15 Thread v1nh1shungry via Phabricator via cfe-commits
v1nh1shungry added inline comments.



Comment at: 
clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:212
   // Produce replacements to add the qualifiers.
   std::string Qualifier = printUsingNamespaceName(Ctx, *TargetDirective) + 
"::";
   for (auto Loc : IdentsToQualify) {

We can replace `printUsingNamespaceName` with `printNamespaceScope` here so 
that we can get `a::foobar()` in the test. 

However, it can sometimes cause redundancy such as in the 10th test. 

And I don't know whether it is worth it. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138028

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


[PATCH] D137968: [clang-tidy] Ignore overriden methods in `readability-const-return-type`.

2022-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 475440.
ymandel added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137968

Files:
  clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
@@ -279,13 +279,22 @@
   // CHECK-NOT-FIXES: virtual int getC() = 0;
 };
 
-class PVDerive : public PVBase {
+class NVDerive : public PVBase {
 public:
-  const int getC() { return 1; }
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 
'const'-qualified at the top level, which may reduce code readability without 
improving const correctness
-  // CHECK-NOT-FIXES: int getC() { return 1; }
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as the user may not be able to change the base class.
+  const int getC() override { return 1; }
 };
 
+class NVDeriveOutOfLine : public PVBase {
+public:
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as one may not be able to change the base class
+  const int getC();
+};
+
+const int NVDeriveOutOfLine::getC() { return 1; }
+
 // Don't warn about const auto types, because it may be impossible to make 
them non-const
 // without a significant semantics change. Since `auto` drops cv-qualifiers,
 // tests check `decltype(auto)`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,11 @@
   :doc:`readability-simplify-boolean-expr 
`
   when using a C++23 ``if consteval`` statement.
 
+- Change the behavior of :doc:`readability-const-return-type
+  ` to not
+  warn about `const` return types in overridden functions since the derived
+  class cannot always choose to change the function signature.
+
 - Improved :doc:`misc-redundant-expression 
`
   check.
 
Index: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
@@ -114,7 +114,9 @@
   Finder->addMatcher(
   functionDecl(
   returns(allOf(isConstQualified(), unless(NonLocalConstType))),
-  anyOf(isDefinition(), cxxMethodDecl(isPure(
+  anyOf(isDefinition(), cxxMethodDecl(isPure())),
+  // Overridden functions are not actionable.
+  unless(cxxMethodDecl(isOverride(
   .bind("func"),
   this);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
@@ -279,13 +279,22 @@
   // CHECK-NOT-FIXES: virtual int getC() = 0;
 };
 
-class PVDerive : public PVBase {
+class NVDerive : public PVBase {
 public:
-  const int getC() { return 1; }
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness
-  // CHECK-NOT-FIXES: int getC() { return 1; }
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as the user may not be able to change the base class.
+  const int getC() override { return 1; }
 };
 
+class NVDeriveOutOfLine : public PVBase {
+public:
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as one may not be able to change the base class
+  const int getC();
+};
+
+const int NVDeriveOutOfLine::getC() { return 1; }
+
 // Don't warn about const auto types, because it may be impossible to make them non-const
 // without a significant semantics change. Since `auto` drops cv-qualifiers,
 // tests check `decltype(auto)`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,11 @@
   :doc:`readability-simplify-boolean-expr `
   when using a C++23 ``if consteval`` statement.
 
+- Change the behavior of :doc:`readability-const-return-type
+  ` to not
+  warn about `const` return types in overridden functions

[PATCH] D137972: [clang-tidy] Optionally ignore findings in macros in `readability-const-return-type`.

2022-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 475441.
ymandel added a comment.

reorder release notes blurb


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137972

Files:
  clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
  clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type-macros.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
@@ -196,13 +196,22 @@
 // Test cases where the `const` token lexically is hidden behind some form of
 // indirection.
 
+// Regression tests involving macros, which are ignored by default because
+// IgnoreMacros defaults to true.
+#define CONCAT(a, b) a##b
+CONCAT(cons, t) int n22(){}
+
 #define CONSTINT const int
-CONSTINT p18() {}
-// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+CONSTINT n23() {}
 
 #define CONST const
-CONST int p19() {}
-// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+CONST int n24() {}
+
+#define CREATE_FUNCTION()\
+const int n_inside_macro() { \
+  return 1; \
+}
+CREATE_FUNCTION();
 
 using ty = const int;
 ty p21() {}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type-macros.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type-macros.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy -std=c++14 %s readability-const-return-type %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-const-return-type.IgnoreMacros, value: false}]}"
+
+//  p# = positive test
+//  n# = negative test
+
+// Regression tests involving macros
+#define CONCAT(a, b) a##b
+CONCAT(cons, t) int p22(){}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+// We warn, but we can't give a fix
+
+#define CONSTINT const int
+CONSTINT p23() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+
+#define CONST const
+CONST int p24() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+
+#define CREATE_FUNCTION()\
+const int p_inside_macro() { \
+  return 1; \
+}
+CREATE_FUNCTION();
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+// We warn, but we can't give a fix
Index: clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
@@ -24,3 +24,12 @@
const int* foo();
const int& foo();
const Clazz* foo();
+
+
+Options
+---
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will not give warnings inside macros. Default
+   is `true`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,10 @@
   warn about `const` return types in overridden functions since the derived
   class cannot always choose to change the function signature.
 
+- Change the default behavior of :doc:`readability-const-return-type
+  ` to not
+  warn about `const` value parameters of declarations inside macros.
+
 - Improved :doc:`misc-redundant-expression `
   check.
 
Index: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
+++ clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
@@ -22,9 +22,15 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/readability/const-return-type.html
 class ConstReturnTypeCheck : public ClangTidyCheck {
  public:
-  using ClangTidyCheck::ClangTidyCheck;
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+   ConstReturnTypeCheck(StringRef Name, ClangTidyContext *Context)
+   : ClangTidyCheck(Name, Context),
+ IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
+   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+   void registerMatchers(ast_matchers::MatchFinder *F

[PATCH] D137960: [Lexer] Speedup LexTokenInternal

2022-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added inline comments.



Comment at: clang/lib/Lex/Lexer.cpp:3522
-  Result.clearFlag(Token::NeedsCleaning);
-  Result.setIdentifierInfo(nullptr);
 

Was this setting the identifier info to nullptr as a shorthand to clear *any* 
token data that was previously built up (basically, a cheaper call to 
`startToken()`)? Should we be asserting that `Token::PtrData` is `nullptr`?



Comment at: clang/lib/Lex/Lexer.cpp:4142-4144
+  if (CurPtr[1] == '|' && HandleEndOfConflictMarker(CurPtr - 1)) {
 goto LexNextToken;
+  }

Unrelated formatting changes?



Comment at: clang/lib/Lex/Lexer.cpp:4172-4174
+  if (CurPtr[1] == '=' && HandleEndOfConflictMarker(CurPtr - 1)) {
 goto LexNextToken;
+  }

Unrelated formatting changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137960

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


[PATCH] D137531: [clang] Add the check of membership in decltype for the issue #58674

2022-11-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

I dont really get the logic here, I'm in need of a much better commit 
message/description here before I can review this.


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

https://reviews.llvm.org/D137531

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


[clang-tools-extra] f3afd16 - [clang-tidy] Ignore overriden methods in `readability-const-return-type`.

2022-11-15 Thread Yitzhak Mandelbaum via cfe-commits

Author: Thomas Etter
Date: 2022-11-15T14:24:05Z
New Revision: f3afd16b65ebda19e417120acf3c3793c171cf5e

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

LOG: [clang-tidy] Ignore overriden methods in `readability-const-return-type`.

Overrides are constrained by the signature of the overridden method, so a
warning on an override is frequently unactionable.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
index ec9222677378..5a451cca800d 100644
--- a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
@@ -114,7 +114,9 @@ void ConstReturnTypeCheck::registerMatchers(MatchFinder 
*Finder) {
   Finder->addMatcher(
   functionDecl(
   returns(allOf(isConstQualified(), unless(NonLocalConstType))),
-  anyOf(isDefinition(), cxxMethodDecl(isPure(
+  anyOf(isDefinition(), cxxMethodDecl(isPure())),
+  // Overridden functions are not actionable.
+  unless(cxxMethodDecl(isOverride(
   .bind("func"),
   this);
 }

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index c5cc967259af..7852c6f3d994 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,11 @@ Changes in existing checks
   :doc:`readability-simplify-boolean-expr 
`
   when using a C++23 ``if consteval`` statement.
 
+- Change the behavior of :doc:`readability-const-return-type
+  ` to not
+  warn about `const` return types in overridden functions since the derived
+  class cannot always choose to change the function signature.
+
 - Improved :doc:`misc-redundant-expression 
`
   check.
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
index 4b59cafb9f66..dd3d7aa9fa22 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
@@ -279,13 +279,22 @@ class PVBase {
   // CHECK-NOT-FIXES: virtual int getC() = 0;
 };
 
-class PVDerive : public PVBase {
+class NVDerive : public PVBase {
 public:
-  const int getC() { return 1; }
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 
'const'-qualified at the top level, which may reduce code readability without 
improving const correctness
-  // CHECK-NOT-FIXES: int getC() { return 1; }
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as the user may not be able to change the base class.
+  const int getC() override { return 1; }
 };
 
+class NVDeriveOutOfLine : public PVBase {
+public:
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as one may not be able to change the base class
+  const int getC();
+};
+
+const int NVDeriveOutOfLine::getC() { return 1; }
+
 // Don't warn about const auto types, because it may be impossible to make 
them non-const
 // without a significant semantics change. Since `auto` drops cv-qualifiers,
 // tests check `decltype(auto)`.



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


[PATCH] D137968: [clang-tidy] Ignore overriden methods in `readability-const-return-type`.

2022-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3afd16b65eb: [clang-tidy] Ignore overriden methods in 
`readability-const-return-type`. (authored by thomasetter, committed by 
ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137968

Files:
  clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
@@ -279,13 +279,22 @@
   // CHECK-NOT-FIXES: virtual int getC() = 0;
 };
 
-class PVDerive : public PVBase {
+class NVDerive : public PVBase {
 public:
-  const int getC() { return 1; }
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 
'const'-qualified at the top level, which may reduce code readability without 
improving const correctness
-  // CHECK-NOT-FIXES: int getC() { return 1; }
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as the user may not be able to change the base class.
+  const int getC() override { return 1; }
 };
 
+class NVDeriveOutOfLine : public PVBase {
+public:
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as one may not be able to change the base class
+  const int getC();
+};
+
+const int NVDeriveOutOfLine::getC() { return 1; }
+
 // Don't warn about const auto types, because it may be impossible to make 
them non-const
 // without a significant semantics change. Since `auto` drops cv-qualifiers,
 // tests check `decltype(auto)`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,11 @@
   :doc:`readability-simplify-boolean-expr 
`
   when using a C++23 ``if consteval`` statement.
 
+- Change the behavior of :doc:`readability-const-return-type
+  ` to not
+  warn about `const` return types in overridden functions since the derived
+  class cannot always choose to change the function signature.
+
 - Improved :doc:`misc-redundant-expression 
`
   check.
 
Index: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
@@ -114,7 +114,9 @@
   Finder->addMatcher(
   functionDecl(
   returns(allOf(isConstQualified(), unless(NonLocalConstType))),
-  anyOf(isDefinition(), cxxMethodDecl(isPure(
+  anyOf(isDefinition(), cxxMethodDecl(isPure())),
+  // Overridden functions are not actionable.
+  unless(cxxMethodDecl(isOverride(
   .bind("func"),
   this);
 }


Index: clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
@@ -279,13 +279,22 @@
   // CHECK-NOT-FIXES: virtual int getC() = 0;
 };
 
-class PVDerive : public PVBase {
+class NVDerive : public PVBase {
 public:
-  const int getC() { return 1; }
-  // CHECK-MESSAGES: [[@LINE-1]]:3: warning: return type 'const int' is 'const'-qualified at the top level, which may reduce code readability without improving const correctness
-  // CHECK-NOT-FIXES: int getC() { return 1; }
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as the user may not be able to change the base class.
+  const int getC() override { return 1; }
 };
 
+class NVDeriveOutOfLine : public PVBase {
+public:
+  // Don't warn about overridden methods, because it may be impossible to make
+  // them non-const as one may not be able to change the base class
+  const int getC();
+};
+
+const int NVDeriveOutOfLine::getC() { return 1; }
+
 // Don't warn about const auto types, because it may be impossible to make them non-const
 // without a significant semantics change. Since `auto` drops cv-qualifiers,
 // tests check `decltype(auto)`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -171,6 +171,11 @@
   :doc:`readability-simplify-boolean-expr `
   when using a C++23 ``if consteval`` sta

[clang-tools-extra] a49fcca - [clang-tidy] Optionally ignore findings in macros in `readability-const-return-type`.

2022-11-15 Thread Yitzhak Mandelbaum via cfe-commits

Author: Thomas Etter
Date: 2022-11-15T14:28:03Z
New Revision: a49fcca9e3ec9e310312416599405d26c189a81b

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

LOG: [clang-tidy] Optionally ignore findings in macros in 
`readability-const-return-type`.

Adds support for options-controlled configuration of the check to ignore 
results in macros.

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type-macros.cpp

Modified: 
clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
index 5a451cca800d..7d8bee4db96e 100644
--- a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
@@ -105,6 +105,10 @@ static CheckResult checkDef(const clang::FunctionDecl *Def,
   return Result;
 }
 
+void ConstReturnTypeCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "IgnoreMacros", IgnoreMacros);
+}
+
 void ConstReturnTypeCheck::registerMatchers(MatchFinder *Finder) {
   // Find all function definitions for which the return types are `const`
   // qualified, ignoring decltype types.
@@ -123,6 +127,11 @@ void ConstReturnTypeCheck::registerMatchers(MatchFinder 
*Finder) {
 
 void ConstReturnTypeCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *Def = Result.Nodes.getNodeAs("func");
+  // Suppress the check if macros are involved.
+  if (IgnoreMacros &&
+  (Def->getBeginLoc().isMacroID() || Def->getEndLoc().isMacroID()))
+return;
+
   CheckResult CR = checkDef(Def, Result);
   {
 // Clang only supports one in-flight diagnostic at a time. So, delimit the

diff  --git a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h 
b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
index 6c89f7601c9b..f28e7c3ecd87 100644
--- a/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
@@ -22,9 +22,15 @@ namespace readability {
 /// 
http://clang.llvm.org/extra/clang-tidy/checks/readability/const-return-type.html
 class ConstReturnTypeCheck : public ClangTidyCheck {
  public:
-  using ClangTidyCheck::ClangTidyCheck;
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+   ConstReturnTypeCheck(StringRef Name, ClangTidyContext *Context)
+   : ClangTidyCheck(Name, Context),
+ IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
+   void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+ private:
+   const bool IgnoreMacros;
 };
 
 } // namespace readability

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 7852c6f3d994..44c3743549ef 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,10 @@ Changes in existing checks
   warn about `const` return types in overridden functions since the derived
   class cannot always choose to change the function signature.
 
+- Change the default behavior of :doc:`readability-const-return-type
+  ` to not
+  warn about `const` value parameters of declarations inside macros.
+
 - Improved :doc:`misc-redundant-expression 
`
   check.
 

diff  --git 
a/clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst 
b/clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
index 154131b60351..ec81d46750d4 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
@@ -24,3 +24,12 @@ pointers or references to const values. For example, these 
are fine:
const int* foo();
const int& foo();
const Clazz* foo();
+
+
+Options
+---
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will not give warnings inside macros. Default
+   is `true`.

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type-macros.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/reada

[PATCH] D137972: [clang-tidy] Optionally ignore findings in macros in `readability-const-return-type`.

2022-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa49fcca9e3ec: [clang-tidy] Optionally ignore findings in 
macros in `readability-const-return… (authored by thomasetter, committed by 
ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137972

Files:
  clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp
  clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type-macros.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
@@ -196,13 +196,22 @@
 // Test cases where the `const` token lexically is hidden behind some form of
 // indirection.
 
+// Regression tests involving macros, which are ignored by default because
+// IgnoreMacros defaults to true.
+#define CONCAT(a, b) a##b
+CONCAT(cons, t) int n22(){}
+
 #define CONSTINT const int
-CONSTINT p18() {}
-// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+CONSTINT n23() {}
 
 #define CONST const
-CONST int p19() {}
-// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+CONST int n24() {}
+
+#define CREATE_FUNCTION()\
+const int n_inside_macro() { \
+  return 1; \
+}
+CREATE_FUNCTION();
 
 using ty = const int;
 ty p21() {}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type-macros.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type-macros.cpp
@@ -0,0 +1,27 @@
+// RUN: %check_clang_tidy -std=c++14 %s readability-const-return-type %t -- \
+// RUN:   -config="{CheckOptions: [{key: readability-const-return-type.IgnoreMacros, value: false}]}"
+
+//  p# = positive test
+//  n# = negative test
+
+// Regression tests involving macros
+#define CONCAT(a, b) a##b
+CONCAT(cons, t) int p22(){}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+// We warn, but we can't give a fix
+
+#define CONSTINT const int
+CONSTINT p23() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+
+#define CONST const
+CONST int p24() {}
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+
+#define CREATE_FUNCTION()\
+const int p_inside_macro() { \
+  return 1; \
+}
+CREATE_FUNCTION();
+// CHECK-MESSAGES: [[@LINE-1]]:1: warning: return type 'const int' is 'const'-qu
+// We warn, but we can't give a fix
Index: clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/const-return-type.rst
@@ -24,3 +24,12 @@
const int* foo();
const int& foo();
const Clazz* foo();
+
+
+Options
+---
+
+.. option:: IgnoreMacros
+
+   If set to `true`, the check will not give warnings inside macros. Default
+   is `true`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -176,6 +176,10 @@
   warn about `const` return types in overridden functions since the derived
   class cannot always choose to change the function signature.
 
+- Change the default behavior of :doc:`readability-const-return-type
+  ` to not
+  warn about `const` value parameters of declarations inside macros.
+
 - Improved :doc:`misc-redundant-expression `
   check.
 
Index: clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
===
--- clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
+++ clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.h
@@ -22,9 +22,15 @@
 /// http://clang.llvm.org/extra/clang-tidy/checks/readability/const-return-type.html
 class ConstReturnTypeCheck : public ClangTidyCheck {
  public:
-  using ClangTidyCheck::ClangTidyCheck;
-  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
-  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+   ConstReturnTypeCheck(StringRef Name, ClangTidyContext *Context)
+   : ClangTidyCheck(Name, Context),
+ IgnoreMacros(Options.getLocalOrGlobal("IgnoreMac

[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

2022-11-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/include/clang/AST/Type.h:3926
+SME_AttributeMask = 255 // We only support maximum 8 bits because of the
+// bitmask in FunctionTypeExtraBitfields
+  };





Comment at: clang/include/clang/AST/Type.h:3940
+/// on declarations and function pointers.
+unsigned AArch64SMEAttributes : 8;
+

We seem to be missing all of the modules-storage code for these.  Since this is 
modifying the AST, we need to increment the 'breaking change' AST code, plus 
add this to the ASTWriter/ASTReader interface.



Comment at: clang/include/clang/AST/Type.h:4008
 bool HasTrailingReturn : 1;
+unsigned AArch64SMEAttributes : 8;
 Qualifiers TypeQuals;

sdesmalen wrote:
> aaron.ballman wrote:
> > So functions without prototypes cannot have any of these attributes?
> Yes, the ACLE explicitly states that 
> [[https://github.com/ARM-software/acle/pull/188/commits/59751df91d9630400531a64108f179e3951c3b89#diff-516526d4a18101dc85300bc2033d0f86dc46c505b7510a7694baabea851aedfaR503|here]]:
> > The function type attributes cannot be used with K&R-style “unprototyped” C 
> > function types
Are they aware that includes; `void Baz();` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127762

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


[clang] d256377 - [AVR][Clang] Move family names into MCU list

2022-11-15 Thread Ayke van Laethem via cfe-commits

Author: Ayke van Laethem
Date: 2022-11-15T15:29:37+01:00
New Revision: d2563775cd6e7b6b26d306ff233349443ef1945c

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

LOG: [AVR][Clang] Move family names into MCU list

This simplifies the code by avoiding some special cases for family names
(as opposed to device names).

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

Added: 


Modified: 
clang/lib/Basic/Targets/AVR.cpp
clang/lib/Basic/Targets/AVR.h
clang/test/Misc/target-invalid-cpu-note.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/AVR.cpp b/clang/lib/Basic/Targets/AVR.cpp
index 9855b6eca5c9..81656b81a18b 100644
--- a/clang/lib/Basic/Targets/AVR.cpp
+++ b/clang/lib/Basic/Targets/AVR.cpp
@@ -29,11 +29,13 @@ struct LLVM_LIBRARY_VISIBILITY MCUInfo {
 
 // NOTE: This list has been synchronized with gcc-avr 5.4.0 and avr-libc 2.0.0.
 static MCUInfo AVRMcus[] = {
+{"avr1", NULL, 0, false},
 {"at90s1200", "__AVR_AT90S1200__", 0, false},
 {"attiny11", "__AVR_ATtiny11__", 0, false},
 {"attiny12", "__AVR_ATtiny12__", 0, false},
 {"attiny15", "__AVR_ATtiny15__", 0, false},
 {"attiny28", "__AVR_ATtiny28__", 0, false},
+{"avr2", NULL, 1, false},
 {"at90s2313", "__AVR_AT90S2313__", 1, false},
 {"at90s2323", "__AVR_AT90S2323__", 1, false},
 {"at90s2333", "__AVR_AT90S2333__", 1, false},
@@ -47,6 +49,7 @@ static MCUInfo AVRMcus[] = {
 {"at90s8515", "__AVR_AT90S8515__", 1, false},
 {"at90c8534", "__AVR_AT90c8534__", 1, false},
 {"at90s8535", "__AVR_AT90S8535__", 1, false},
+{"avr25", NULL, 1, false},
 {"ata5272", "__AVR_ATA5272__", 1, false},
 {"ata6616c", "__AVR_ATA6616c__", 1, false},
 {"attiny13", "__AVR_ATtiny13__", 1, false},
@@ -76,10 +79,13 @@ static MCUInfo AVRMcus[] = {
 {"attiny48", "__AVR_ATtiny48__", 1, false},
 {"attiny88", "__AVR_ATtiny88__", 1, false},
 {"attiny828", "__AVR_ATtiny828__", 1, false},
+{"avr3", NULL, 1, false},
 {"at43usb355", "__AVR_AT43USB355__", 1, false},
 {"at76c711", "__AVR_AT76C711__", 1, false},
+{"avr31", NULL, 1, false},
 {"atmega103", "__AVR_ATmega103__", 1, false},
 {"at43usb320", "__AVR_AT43USB320__", 1, false},
+{"avr35", NULL, 1, false},
 {"attiny167", "__AVR_ATtiny167__", 1, false},
 {"at90usb82", "__AVR_AT90USB82__", 1, false},
 {"at90usb162", "__AVR_AT90USB162__", 1, false},
@@ -90,6 +96,7 @@ static MCUInfo AVRMcus[] = {
 {"atmega16u2", "__AVR_ATmega16U2__", 1, false},
 {"atmega32u2", "__AVR_ATmega32U2__", 1, false},
 {"attiny1634", "__AVR_ATtiny1634__", 1, false},
+{"avr4", NULL, 1, false},
 {"atmega8", "__AVR_ATmega8__", 1, false},
 {"ata6289", "__AVR_ATA6289__", 1, false},
 {"atmega8a", "__AVR_ATmega8A__", 1, false},
@@ -115,6 +122,7 @@ static MCUInfo AVRMcus[] = {
 {"at90pwm3", "__AVR_AT90PWM3__", 1, false},
 {"at90pwm3b", "__AVR_AT90PWM3B__", 1, false},
 {"at90pwm81", "__AVR_AT90PWM81__", 1, false},
+{"avr5", NULL, 1, false},
 {"ata5702m322", "__AVR_ATA5702M322__", 1, false},
 {"ata5782", "__AVR_ATA5782__", 1, false},
 {"ata5790", "__AVR_ATA5790__", 1, false},
@@ -221,6 +229,7 @@ static MCUInfo AVRMcus[] = {
 {"at90scr100", "__AVR_AT90SCR100__", 1, false},
 {"at94k", "__AVR_AT94K__", 1, false},
 {"m3000", "__AVR_AT000__", 1, false},
+{"avr51", NULL, 2, false},
 {"atmega128", "__AVR_ATmega128__", 2, false},
 {"atmega128a", "__AVR_ATmega128A__", 2, false},
 {"atmega1280", "__AVR_ATmega1280__", 2, false},
@@ -233,10 +242,12 @@ static MCUInfo AVRMcus[] = {
 {"at90can128", "__AVR_AT90CAN128__", 2, false},
 {"at90usb1286", "__AVR_AT90USB1286__", 2, false},
 {"at90usb1287", "__AVR_AT90USB1287__", 2, false},
+{"avr6", NULL, 4, false},
 {"atmega2560", "__AVR_ATmega2560__", 4, false},
 {"atmega2561", "__AVR_ATmega2561__", 4, false},
 {"atmega256rfr2", "__AVR_ATmega256RFR2__", 4, false},
 {"atmega2564rfr2", "__AVR_ATmega2564RFR2__", 4, false},
+{"avrxmega2", NULL, 1, false},
 {"atxmega16a4", "__AVR_ATxmega16A4__", 1, false},
 {"atxmega16a4u", "__AVR_ATxmega16A4U__", 1, false},
 {"atxmega16c4", "__AVR_ATxmega16C4__", 1, false},
@@ -250,6 +261,7 @@ static MCUInfo AVRMcus[] = {
 {"atxmega32e5", "__AVR_ATxmega32E5__", 1, false},
 {"atxmega16e5", "__AVR_ATxmega16E5__", 1, false},
 {"atxmega8e5", "__AVR_ATxmega8E5__", 1, false},
+{"avrxmega4", NULL, 1, false},
 {"atxmega64a3", "__AVR_ATxmega64A3__", 1, false},
 {"atxmega64a3u", "__AVR_ATxmega64A3U__", 1, false},
 {"atxmega64a4u", "__AVR_ATxmega64A4U__", 1, false},
@@ -258,8 +270,10 @@ static MCUInfo AVRMcus[] = {
 {"atxmega64c3", "__AVR_ATxmega64C3__", 1, false},
 {"atxmega64d3", "__AVR_ATxmega64D3__", 1, f

[PATCH] D137520: [AVR][Clang] Move family names into MCU list

2022-11-15 Thread Ayke 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 rGd2563775cd6e: [AVR][Clang] Move family names into MCU list 
(authored by aykevl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137520

Files:
  clang/lib/Basic/Targets/AVR.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/test/Misc/target-invalid-cpu-note.c

Index: clang/test/Misc/target-invalid-cpu-note.c
===
--- clang/test/Misc/target-invalid-cpu-note.c
+++ clang/test/Misc/target-invalid-cpu-note.c
@@ -77,7 +77,7 @@
 
 // RUN: not %clang_cc1 -triple avr--- -target-cpu not-a-cpu -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix AVR
 // AVR: error: unknown target CPU 'not-a-cpu'
-// AVR-NEXT: note: valid target CPU values are: avr1, avr2, avr25, avr3, avr31, avr35, avr4, avr5, avr51, avr6, avrxmega1, avrxmega2, avrxmega3, avrxmega4, avrxmega5, avrxmega6, avrxmega7, avrtiny, at90s1200, attiny11, attiny12, attiny15, attiny28, at90s2313, at90s2323, at90s2333, at90s2343, attiny22, attiny26, at86rf401, at90s4414, at90s4433, at90s4434, at90s8515, at90c8534, at90s8535, ata5272, ata6616c, attiny13, attiny13a, attiny2313, attiny2313a, attiny24, attiny24a, attiny4313, attiny44, attiny44a, attiny84, attiny84a, attiny25, attiny45, attiny85, attiny261, attiny261a, attiny441, attiny461, attiny461a, attiny841, attiny861, attiny861a, attiny87, attiny43u, attiny48, attiny88, attiny828, at43usb355, at76c711, atmega103, at43usb320, attiny167, at90usb82, at90usb162, ata5505, ata6617c, ata664251, atmega8u2, atmega16u2, atmega32u2, attiny1634, atmega8, ata6289, atmega8a, ata6285, ata6286, ata6612c, atmega48, atmega48a, atmega48pa, atmega48pb, atmega48p, atmega88, atmega88a, atmega88p, atmega88pa, atmega88pb, atmega8515, atmega8535, atmega8hva, at90pwm1, at90pwm2, at90pwm2b, at90pwm3, at90pwm3b, at90pwm81, ata5702m322, ata5782, ata5790, ata5790n, ata5791, ata5795, ata5831, ata6613c, ata6614q, ata8210, ata8510, atmega16, atmega16a, atmega161, atmega162, atmega163, atmega164a, atmega164p, atmega164pa, atmega165, atmega165a, atmega165p, atmega165pa, atmega168, atmega168a, atmega168p, atmega168pa, atmega168pb, atmega169, atmega169a, atmega169p, atmega169pa, atmega32, atmega32a, atmega323, atmega324a, atmega324p, atmega324pa, atmega324pb, atmega325, atmega325a, atmega325p, atmega325pa, atmega3250, atmega3250a, atmega3250p, atmega3250pa, atmega328, atmega328p, atmega328pb, atmega329, atmega329a, atmega329p, atmega329pa, atmega3290, atmega3290a, atmega3290p, atmega3290pa, atmega406, atmega64, atmega64a, atmega640, atmega644, atmega644a, atmega644p, atmega644pa, atmega645, atmega645a, atmega645p, atmega649, atmega649a, atmega649p, atmega6450, atmega6450a, atmega6450p, atmega6490, atmega6490a, atmega6490p, atmega64rfr2, atmega644rfr2, atmega16hva, atmega16hva2, atmega16hvb, atmega16hvbrevb, atmega32hvb, atmega32hvbrevb, atmega64hve, atmega64hve2, at90can32, at90can64, at90pwm161, at90pwm216, at90pwm316, atmega32c1, atmega64c1, atmega16m1, atmega32m1, atmega64m1, atmega16u4, atmega32u4, atmega32u6, at90usb646, at90usb647, at90scr100, at94k, m3000, atmega128, atmega128a, atmega1280, atmega1281, atmega1284, atmega1284p, atmega128rfa1, atmega128rfr2, atmega1284rfr2, at90can128, at90usb1286, at90usb1287, atmega2560, atmega2561, atmega256rfr2, atmega2564rfr2, atxmega16a4, atxmega16a4u, atxmega16c4, atxmega16d4, atxmega32a4, atxmega32a4u, atxmega32c3, atxmega32c4, atxmega32d3, atxmega32d4, atxmega32e5, atxmega16e5, atxmega8e5, atxmega64a3, atxmega64a3u, atxmega64a4u, atxmega64b1, atxmega64b3, atxmega64c3, atxmega64d3, atxmega64d4, atxmega64a1, atxmega64a1u, atxmega128a3, atxmega128a3u, atxmega128b1, atxmega128b3, atxmega128c3, atxmega128d3, atxmega128d4, atxmega192a3, atxmega192a3u, atxmega192c3, atxmega192d3, atxmega256a3, atxmega256a3u, atxmega256a3b, atxmega256a3bu, atxmega256c3, atxmega256d3, atxmega384c3, atxmega384d3, atxmega128a1, atxmega128a1u, atxmega128a4u, attiny4, attiny5, attiny9, attiny10, attiny20, attiny40, attiny102, attiny104, attiny202, attiny402, attiny204, attiny404, attiny804, attiny1604, attiny406, attiny806, attiny1606, attiny807, attiny1607, attiny212, attiny412, attiny214, attiny414, attiny814, attiny1614, attiny416, attiny816, attiny1616, attiny3216, attiny417, attiny817, attiny1617, attiny3217, attiny1624, attiny1626, attiny1627, atmega808, atmega809, atmega1608, atmega1609, atmega3208, atmega3209, atmega4808, atmega4809
+// AVR-NEXT: note: valid target CPU values are: avr1, at90s1200, attiny11, attiny12, attiny15, attiny28, avr2, at90s2313, at90s2323, at90s2333, at90s2343, attiny22, attiny26, at86rf401, at90s4414, at90s4433, at90s4434, at90s8515, at90c8534, at90s8535, avr25, ata5272, ata6616c, attiny13, attiny13a, attiny2313, attiny2313a, attiny24, attiny24a, attiny4313, attiny44, attiny44a, attiny84, attiny84a, attiny25, at

[PATCH] D137521: [AVR][Clang] Implement __AVR_ARCH__ macro

2022-11-15 Thread Ayke 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 rG09ab9d4d111f: [AVR][Clang] Implement __AVR_ARCH__ macro 
(authored by aykevl).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137521

Files:
  clang/lib/Basic/Targets/AVR.cpp
  clang/lib/Basic/Targets/AVR.h
  clang/test/Preprocessor/avr-atmega328p.c
  clang/test/Preprocessor/avr-attiny104.c

Index: clang/test/Preprocessor/avr-attiny104.c
===
--- clang/test/Preprocessor/avr-attiny104.c
+++ clang/test/Preprocessor/avr-attiny104.c
@@ -2,6 +2,7 @@
 
 // CHECK: #define AVR 1
 // CHECK: #define __AVR 1
+// CHECK: #define __AVR_ARCH__ 100
 // CHECK: #define __AVR_ATtiny104__ 1
 // CHECK: #define __AVR__ 1
 // CHECK: #define __ELF__ 1
Index: clang/test/Preprocessor/avr-atmega328p.c
===
--- clang/test/Preprocessor/avr-atmega328p.c
+++ clang/test/Preprocessor/avr-atmega328p.c
@@ -2,6 +2,7 @@
 
 // CHECK: #define AVR 1
 // CHECK: #define __AVR 1
+// CHECK: #define __AVR_ARCH__ 5
 // CHECK: #define __AVR_ATmega328P__ 1
 // CHECK: #define __AVR__ 1
 // CHECK: #define __ELF__ 1
Index: clang/lib/Basic/Targets/AVR.h
===
--- clang/lib/Basic/Targets/AVR.h
+++ clang/lib/Basic/Targets/AVR.h
@@ -175,6 +175,7 @@
   std::string CPU;
   StringRef ABI;
   StringRef DefineName;
+  StringRef Arch;
   int NumFlashBanks;
 };
 
Index: clang/lib/Basic/Targets/AVR.cpp
===
--- clang/lib/Basic/Targets/AVR.cpp
+++ clang/lib/Basic/Targets/AVR.cpp
@@ -23,326 +23,326 @@
 struct LLVM_LIBRARY_VISIBILITY MCUInfo {
   const char *Name;
   const char *DefineName;
+  StringRef Arch; // The __AVR_ARCH__ value.
   const int NumFlashBanks; // Set to 0 for the devices do not support LPM/ELPM.
-  bool IsTiny; // Set to true for the devices belong to the avrtiny family.
 };
 
 // NOTE: This list has been synchronized with gcc-avr 5.4.0 and avr-libc 2.0.0.
 static MCUInfo AVRMcus[] = {
-{"avr1", NULL, 0, false},
-{"at90s1200", "__AVR_AT90S1200__", 0, false},
-{"attiny11", "__AVR_ATtiny11__", 0, false},
-{"attiny12", "__AVR_ATtiny12__", 0, false},
-{"attiny15", "__AVR_ATtiny15__", 0, false},
-{"attiny28", "__AVR_ATtiny28__", 0, false},
-{"avr2", NULL, 1, false},
-{"at90s2313", "__AVR_AT90S2313__", 1, false},
-{"at90s2323", "__AVR_AT90S2323__", 1, false},
-{"at90s2333", "__AVR_AT90S2333__", 1, false},
-{"at90s2343", "__AVR_AT90S2343__", 1, false},
-{"attiny22", "__AVR_ATtiny22__", 1, false},
-{"attiny26", "__AVR_ATtiny26__", 1, false},
-{"at86rf401", "__AVR_AT86RF401__", 1, false},
-{"at90s4414", "__AVR_AT90S4414__", 1, false},
-{"at90s4433", "__AVR_AT90S4433__", 1, false},
-{"at90s4434", "__AVR_AT90S4434__", 1, false},
-{"at90s8515", "__AVR_AT90S8515__", 1, false},
-{"at90c8534", "__AVR_AT90c8534__", 1, false},
-{"at90s8535", "__AVR_AT90S8535__", 1, false},
-{"avr25", NULL, 1, false},
-{"ata5272", "__AVR_ATA5272__", 1, false},
-{"ata6616c", "__AVR_ATA6616c__", 1, false},
-{"attiny13", "__AVR_ATtiny13__", 1, false},
-{"attiny13a", "__AVR_ATtiny13A__", 1, false},
-{"attiny2313", "__AVR_ATtiny2313__", 1, false},
-{"attiny2313a", "__AVR_ATtiny2313A__", 1, false},
-{"attiny24", "__AVR_ATtiny24__", 1, false},
-{"attiny24a", "__AVR_ATtiny24A__", 1, false},
-{"attiny4313", "__AVR_ATtiny4313__", 1, false},
-{"attiny44", "__AVR_ATtiny44__", 1, false},
-{"attiny44a", "__AVR_ATtiny44A__", 1, false},
-{"attiny84", "__AVR_ATtiny84__", 1, false},
-{"attiny84a", "__AVR_ATtiny84A__", 1, false},
-{"attiny25", "__AVR_ATtiny25__", 1, false},
-{"attiny45", "__AVR_ATtiny45__", 1, false},
-{"attiny85", "__AVR_ATtiny85__", 1, false},
-{"attiny261", "__AVR_ATtiny261__", 1, false},
-{"attiny261a", "__AVR_ATtiny261A__", 1, false},
-{"attiny441", "__AVR_ATtiny441__", 1, false},
-{"attiny461", "__AVR_ATtiny461__", 1, false},
-{"attiny461a", "__AVR_ATtiny461A__", 1, false},
-{"attiny841", "__AVR_ATtiny841__", 1, false},
-{"attiny861", "__AVR_ATtiny861__", 1, false},
-{"attiny861a", "__AVR_ATtiny861A__", 1, false},
-{"attiny87", "__AVR_ATtiny87__", 1, false},
-{"attiny43u", "__AVR_ATtiny43U__", 1, false},
-{"attiny48", "__AVR_ATtiny48__", 1, false},
-{"attiny88", "__AVR_ATtiny88__", 1, false},
-{"attiny828", "__AVR_ATtiny828__", 1, false},
-{"avr3", NULL, 1, false},
-{"at43usb355", "__AVR_AT43USB355__", 1, false},
-{"at76c711", "__AVR_AT76C711__", 1, false},
-{"avr31", NULL, 1, false},
-{"atmega103", "__AVR_ATmega103__", 1, false},
-{"at43usb320", "__AVR_AT43USB320__", 1, false},
-{

[clang] 09ab9d4 - [AVR][Clang] Implement __AVR_ARCH__ macro

2022-11-15 Thread Ayke van Laethem via cfe-commits

Author: Ayke van Laethem
Date: 2022-11-15T15:29:37+01:00
New Revision: 09ab9d4d111f7ee2c31b7783099ca41f3aab625d

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

LOG: [AVR][Clang] Implement __AVR_ARCH__ macro

This macro is defined in avr-gcc, and is very useful especially in
assembly code to check whether particular instructions are supported. It
is also the basis for other macros like __AVR_HAVE_ELPM__.

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

Added: 


Modified: 
clang/lib/Basic/Targets/AVR.cpp
clang/lib/Basic/Targets/AVR.h
clang/test/Preprocessor/avr-atmega328p.c
clang/test/Preprocessor/avr-attiny104.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/AVR.cpp b/clang/lib/Basic/Targets/AVR.cpp
index 81656b81a18b..e3d1c661df5a 100644
--- a/clang/lib/Basic/Targets/AVR.cpp
+++ b/clang/lib/Basic/Targets/AVR.cpp
@@ -23,326 +23,326 @@ namespace targets {
 struct LLVM_LIBRARY_VISIBILITY MCUInfo {
   const char *Name;
   const char *DefineName;
+  StringRef Arch; // The __AVR_ARCH__ value.
   const int NumFlashBanks; // Set to 0 for the devices do not support LPM/ELPM.
-  bool IsTiny; // Set to true for the devices belong to the avrtiny family.
 };
 
 // NOTE: This list has been synchronized with gcc-avr 5.4.0 and avr-libc 2.0.0.
 static MCUInfo AVRMcus[] = {
-{"avr1", NULL, 0, false},
-{"at90s1200", "__AVR_AT90S1200__", 0, false},
-{"attiny11", "__AVR_ATtiny11__", 0, false},
-{"attiny12", "__AVR_ATtiny12__", 0, false},
-{"attiny15", "__AVR_ATtiny15__", 0, false},
-{"attiny28", "__AVR_ATtiny28__", 0, false},
-{"avr2", NULL, 1, false},
-{"at90s2313", "__AVR_AT90S2313__", 1, false},
-{"at90s2323", "__AVR_AT90S2323__", 1, false},
-{"at90s2333", "__AVR_AT90S2333__", 1, false},
-{"at90s2343", "__AVR_AT90S2343__", 1, false},
-{"attiny22", "__AVR_ATtiny22__", 1, false},
-{"attiny26", "__AVR_ATtiny26__", 1, false},
-{"at86rf401", "__AVR_AT86RF401__", 1, false},
-{"at90s4414", "__AVR_AT90S4414__", 1, false},
-{"at90s4433", "__AVR_AT90S4433__", 1, false},
-{"at90s4434", "__AVR_AT90S4434__", 1, false},
-{"at90s8515", "__AVR_AT90S8515__", 1, false},
-{"at90c8534", "__AVR_AT90c8534__", 1, false},
-{"at90s8535", "__AVR_AT90S8535__", 1, false},
-{"avr25", NULL, 1, false},
-{"ata5272", "__AVR_ATA5272__", 1, false},
-{"ata6616c", "__AVR_ATA6616c__", 1, false},
-{"attiny13", "__AVR_ATtiny13__", 1, false},
-{"attiny13a", "__AVR_ATtiny13A__", 1, false},
-{"attiny2313", "__AVR_ATtiny2313__", 1, false},
-{"attiny2313a", "__AVR_ATtiny2313A__", 1, false},
-{"attiny24", "__AVR_ATtiny24__", 1, false},
-{"attiny24a", "__AVR_ATtiny24A__", 1, false},
-{"attiny4313", "__AVR_ATtiny4313__", 1, false},
-{"attiny44", "__AVR_ATtiny44__", 1, false},
-{"attiny44a", "__AVR_ATtiny44A__", 1, false},
-{"attiny84", "__AVR_ATtiny84__", 1, false},
-{"attiny84a", "__AVR_ATtiny84A__", 1, false},
-{"attiny25", "__AVR_ATtiny25__", 1, false},
-{"attiny45", "__AVR_ATtiny45__", 1, false},
-{"attiny85", "__AVR_ATtiny85__", 1, false},
-{"attiny261", "__AVR_ATtiny261__", 1, false},
-{"attiny261a", "__AVR_ATtiny261A__", 1, false},
-{"attiny441", "__AVR_ATtiny441__", 1, false},
-{"attiny461", "__AVR_ATtiny461__", 1, false},
-{"attiny461a", "__AVR_ATtiny461A__", 1, false},
-{"attiny841", "__AVR_ATtiny841__", 1, false},
-{"attiny861", "__AVR_ATtiny861__", 1, false},
-{"attiny861a", "__AVR_ATtiny861A__", 1, false},
-{"attiny87", "__AVR_ATtiny87__", 1, false},
-{"attiny43u", "__AVR_ATtiny43U__", 1, false},
-{"attiny48", "__AVR_ATtiny48__", 1, false},
-{"attiny88", "__AVR_ATtiny88__", 1, false},
-{"attiny828", "__AVR_ATtiny828__", 1, false},
-{"avr3", NULL, 1, false},
-{"at43usb355", "__AVR_AT43USB355__", 1, false},
-{"at76c711", "__AVR_AT76C711__", 1, false},
-{"avr31", NULL, 1, false},
-{"atmega103", "__AVR_ATmega103__", 1, false},
-{"at43usb320", "__AVR_AT43USB320__", 1, false},
-{"avr35", NULL, 1, false},
-{"attiny167", "__AVR_ATtiny167__", 1, false},
-{"at90usb82", "__AVR_AT90USB82__", 1, false},
-{"at90usb162", "__AVR_AT90USB162__", 1, false},
-{"ata5505", "__AVR_ATA5505__", 1, false},
-{"ata6617c", "__AVR_ATA6617C__", 1, false},
-{"ata664251", "__AVR_ATA664251__", 1, false},
-{"atmega8u2", "__AVR_ATmega8U2__", 1, false},
-{"atmega16u2", "__AVR_ATmega16U2__", 1, false},
-{"atmega32u2", "__AVR_ATmega32U2__", 1, false},
-{"attiny1634", "__AVR_ATtiny1634__", 1, false},
-{"avr4", NULL, 1, false},
-{"atmega8", "__AVR_ATmega8__", 1, false},
-{"ata6289", "__AVR_ATA6289__", 1, false},
-{"atmega8a", "__AVR_ATmega8A__", 1, false},
-

[PATCH] D138010: [AArch64][ARM] add Armv8.9-a/Armv9.4-a identifier support

2022-11-15 Thread Ties Stuij via Phabricator via cfe-commits
stuij updated this revision to Diff 475454.
stuij added a comment.

make work with recent TargetParser changes (D137924 
)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138010

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-v89a.c
  clang/test/Driver/aarch64-v94a.c
  clang/test/Driver/arm-cortex-cpus-1.c
  clang/test/Preprocessor/arm-target-features.c
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Support/ARMTargetParserCommon.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -32,10 +32,12 @@
 "armv8a",   "armv8l",  "armv8.1-a",  "armv8.1a","armv8.2-a",
 "armv8.2a", "armv8.3-a",   "armv8.3a",   "armv8.4-a",   "armv8.4a",
 "armv8.5-a","armv8.5a","armv8.6-a",  "armv8.6a","armv8.7-a",
-"armv8.7a", "armv8.8-a",   "armv8.8a",   "armv8-r", "armv8r",
-"armv8-m.base", "armv8m.base", "armv8-m.main",   "armv8m.main", "iwmmxt",
-"iwmmxt2",  "xscale",  "armv8.1-m.main", "armv9-a", "armv9",
-"armv9a",   "armv9.1-a",   "armv9.1a",   "armv9.2-a",   "armv9.2a",
+"armv8.7a", "armv8.8-a",   "armv8.8a",   "armv8.9-a",   "armv8.9a",
+"armv8-r",  "armv8r",  "armv8-m.base",   "armv8m.base", "armv8-m.main",
+"armv8m.main",  "iwmmxt",  "iwmmxt2","xscale",  "armv8.1-m.main",
+"armv9-a",  "armv9",   "armv9a", "armv9.1-a",   "armv9.1a",
+"armv9.2-a","armv9.2a","armv9.3-a",  "armv9.3a","armv9.4-a",
+"armv9.4a",
 };
 
 template 
@@ -510,6 +512,9 @@
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testARMArch("armv8.8-a", "generic", "v8.8a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(
+  testARMArch("armv8.9-a", "generic", "v8.9a",
+  ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(
   testARMArch("armv9-a", "generic", "v9a",
   ARMBuildAttrs::CPUArch::v9_A));
@@ -522,6 +527,9 @@
   EXPECT_TRUE(
   testARMArch("armv9.3-a", "generic", "v9.3a",
   ARMBuildAttrs::CPUArch::v9_A));
+  EXPECT_TRUE(
+  testARMArch("armv9.4-a", "generic", "v9.4a",
+  ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(
   testARMArch("armv8-r", "cortex-r52", "v8r",
   ARMBuildAttrs::CPUArch::v8_R));
@@ -852,10 +860,12 @@
 case ARM::ArchKind::ARMV8_6A:
 case ARM::ArchKind::ARMV8_7A:
 case ARM::ArchKind::ARMV8_8A:
+case ARM::ArchKind::ARMV8_9A:
 case ARM::ArchKind::ARMV9A:
 case ARM::ArchKind::ARMV9_1A:
 case ARM::ArchKind::ARMV9_2A:
 case ARM::ArchKind::ARMV9_3A:
+case ARM::ArchKind::ARMV9_4A:
   EXPECT_EQ(ARM::ProfileKind::A, ARM::parseArchProfile(ARMArch[i]));
   break;
 default:
@@ -1422,12 +1432,18 @@
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv8.8-a", "generic", "v8.8a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv8.9-a", "generic", "v8.9a",
+  ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv9-a", "generic", "v9a",
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv9.1-a", "generic", "v9.1a",
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv9.2-a", "generic", "v9.2a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv9.3-a", "generic", "v9.3a",
+  ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv9.4-a", "generic", "v9.4a",
+  ARMBuildAttrs::CPUArch::v8_A));
 }
 
 bool testAArch64Extension(StringRef CPUName, AArch64::ArchKind AK,
Index: llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
===
--- llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
+++ llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
@@ -890,10

[PATCH] D138010: [AArch64][ARM] add Armv8.9-a/Armv9.4-a identifier support

2022-11-15 Thread Ties Stuij via Phabricator via cfe-commits
stuij marked an inline comment as done.
stuij added inline comments.



Comment at: llvm/include/llvm/Support/ARMTargetParser.def:127
+  ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_CRC | ARM::AEK_RAS |
+  ARM::AEK_DOTPROD | ARM::AEK_BF16 | ARM::AEK_I8MM))
 ARM_ARCH("armv9-a", ARMV9A, "9-A", "v9a",

tmatheson wrote:
> No ARM::AEK_SHA2 | ARM::AEK_AES? Or does 8.8 need updated?
Yes, I think 8.8 needs update, and some other arches as well.

In the A profile armarm, section A2.3, it is stated that from 8.2 SME(2) and 
EAS aren't by default included in the cryptographic extension as the 
Cryptographic Extension in an implementation is subject to export license 
controls. Inclusion of the extension can be either/or or none, so we should 
default to none.

I think this should be handled by  separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138010

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


[PATCH] D138031: Add clang-tidy check for missing move constructors

2022-11-15 Thread Martin Bidlingmaier via Phabricator via cfe-commits
mbid created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a reviewer: njames93.
Herald added a project: All.
mbid requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138031

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/MissingMoveConstructorCheck.cpp
  clang-tools-extra/clang-tidy/performance/MissingMoveConstructorCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance/missing-move-constructor.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance/missing-move-constructor.cpp
  clang/include/clang/Lex/Token.h

Index: clang/include/clang/Lex/Token.h
===
--- clang/include/clang/Lex/Token.h
+++ clang/include/clang/Lex/Token.h
@@ -96,9 +96,7 @@
   /// "if (Tok.is(tok::l_brace)) {...}".
   bool is(tok::TokenKind K) const { return Kind == K; }
   bool isNot(tok::TokenKind K) const { return Kind != K; }
-  bool isOneOf(tok::TokenKind K1, tok::TokenKind K2) const {
-return is(K1) || is(K2);
-  }
+  bool isOneOf() const { return false; }
   template  bool isOneOf(tok::TokenKind K1, Ts... Ks) const {
 return is(K1) || isOneOf(Ks...);
   }
Index: clang-tools-extra/test/clang-tidy/checkers/performance/missing-move-constructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance/missing-move-constructor.cpp
@@ -0,0 +1,99 @@
+// RUN: %check_clang_tidy %s performance-missing-move-constructor %t
+
+class OffendingDtor  {
+public:
+  ~OffendingDtor() = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: explicit destructor prevents generation of implicit move constructor and move assignment operator [performance-missing-move-constructor]
+};
+
+class OffendingCopyCtor  {
+public:
+  OffendingCopyCtor(const OffendingCopyCtor &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: explicit copy constructor prevents generation of implicit move constructor and move assignment operator [performance-missing-move-constructor]
+};
+
+class OffendingMoveCtor  {
+public:
+  OffendingMoveCtor(OffendingMoveCtor &&) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: explicit move constructor prevents generation of implicit move assignment operator [performance-missing-move-constructor]
+
+  // Copy assignment operator is not implicitly defined because of the explicit
+  // move constructor.
+  OffendingMoveCtor &operator=(const OffendingMoveCtor &) = default;
+};
+
+class OffendingCopyAssign  {
+public:
+  OffendingCopyAssign &operator=(const OffendingCopyAssign &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: explicit copy assignment operator prevents generation of implicit move constructor and move assignment operator [performance-missing-move-constructor]
+};
+
+class OffendingMoveAssign  {
+public:
+  // Copy constructor is not implicitly defined because of the explicit move
+  // assignment operator. This means that the warning appears here, because the
+  // copy constructor has higher priority in where we print than the move
+  // assignment operator.
+  OffendingMoveAssign(const OffendingMoveAssign &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: explicit copy constructor prevents generation of implicit move constructor [performance-missing-move-constructor]
+
+  OffendingMoveAssign &operator=(OffendingMoveAssign &&) = default;
+};
+
+
+class InlineFix {
+public:
+  InlineFix(const InlineFix &) = default;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: explicit copy constructor prevents generation of implicit move constructor and move assignment operator [performance-missing-move-constructor]
+  // CHECK-FIXES: InlineFix(InlineFix &&) = default;
+
+  InlineFix& operator=(const InlineFix &) = default;
+  // CHECK-FIXES: InlineFix &operator=(InlineFix &&) = default;
+};
+
+namespace N {
+
+class OutOfLineFix {
+public:
+  OutOfLineFix(const OutOfLineFix &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: explicit copy constructor prevents generation of implicit move constructor and move assignment operator [performance-missing-move-constructor]
+  // CHECK-FIXES: OutOfLineFix(OutOfLineFix &&);
+
+  OutOfLineFix& operator=(const OutOfLineFix &);
+  // CHECK-FIXES: OutOfLineFix &operator=(OutOfLineFix &&);
+};
+
+}
+
+N::OutOfLineFix::OutOfLineFix(const OutOfLineFix &) = default;
+// CHECK-FIXES: N::OutOfLineFix::OutOfLineFix(OutOfLineFix &&) = default;
+
+N::OutOfLineFix &N::OutOfLineFix::operator=(const OutOfLineFix &) = default;
+// CHECK-FIXES: N::OutOfLineFix &N::OutOfLineFix::operator=(OutOfLineFix &&) = default;
+
+class InlineFix

[PATCH] D136354: [Driver] Enable nested configuration files

2022-11-15 Thread Michał Górny via Phabricator via cfe-commits
mgorny accepted this revision.
mgorny added a comment.
This revision is now accepted and ready to land.

LGTM (without testing, just eyeball review).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136354

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


[PATCH] D137901: [Clang] `nothrow`-implying attributes should actually manifest `nothrow` attribute (PR58798)

2022-11-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:8794
+  case ParsedAttr::AT_Pure:
+handleSimpleAttribute(S, D, AL);
+if (!D->hasAttr()) {

First, I'm not a fan of doing this in this list ast all, we shouldn't really 
have logic here, this is mostly just a jump table.

Second, I guess I'm not getting why IRGen cannot figure out from the AST that 
Pure/Const/NoAlias all imply nothrow?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137901

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


Re: How can Autoconf help with the transition to stricter compilation defaults?

2022-11-15 Thread Jonathan Wakely via cfe-commits
On Mon, 14 Nov 2022 at 18:15, Paul Eggert  wrote:
>
> On 2022-11-14 04:41, Aaron Ballman wrote:
> > it's generally a problem when autoconf relies on invalid
> > language constructs
>
> Autoconf *must* rely on invalid language constructs, if only to test
> whether the language constructs work. And Clang therefore must be
> careful about how it diagnoses invalid constructs. Clang shouldn't
> willy-nilly change the way it reports invalid constructs, as that can
> break Autoconf.

There's a difference between checking whether an invalid construct
works, where that construct is the subject of the test, and
incidentally relying on invalid language constructs as part of testing
for other things.



> > issues of security
> > like statically known instances of UB
>
> It's fine to report those; I'm not saying don't report them. All I'm
> saying is that Clang should be careful about *how* it reports them.

Could you clarify what you mean, with a concrete example? Surely as
long as errors are reported on stderr and the compiler exits with
non-zero status, that's an acceptable way to report errors? What kind
of changes to error reporting are you saying to be careful with?

If Clang starts to diagnose a given provable-UB case (or any other
construct) as an error instead of a warning, then it seems entirely
correct for autoconf to report that the case does not work. That's the
desired behaviour, isn't it?

What we don't want is for autoconf to start reporting that *other*
things don't work, as a result of autoconf relying on UB or ill-formed
code when trying to check other things like the presence of function
'foo'. And that's why autoconf should avoid using invalid/undefined
code when possible.

>
> At the very least if there are going to be changes in this area, the
> Clang developers should notify Autoconf (and presumably other)
> downstream users of the changes, and provide a supported way to get the
> old behavior for reporting, and give downstream time to adapt.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[clang] de034cf - [NFC] Fix the typo and the format in the StandardCPlusPlusModules

2022-11-15 Thread Chuanqi Xu via cfe-commits

Author: Chuanqi Xu
Date: 2022-11-15T22:53:43+08:00
New Revision: de034cf3136dfe421189d8627654aad0313cf988

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

LOG: [NFC] Fix the typo and the format in the StandardCPlusPlusModules
document

Added: 


Modified: 
clang/docs/StandardCPlusPlusModules.rst

Removed: 




diff  --git a/clang/docs/StandardCPlusPlusModules.rst 
b/clang/docs/StandardCPlusPlusModules.rst
index c9790703e2eed..c7c04767c7b88 100644
--- a/clang/docs/StandardCPlusPlusModules.rst
+++ b/clang/docs/StandardCPlusPlusModules.rst
@@ -275,7 +275,9 @@ Module name requirement
 
 [module.unit]p1 says:
 
-> All module-names either beginning with an identifier consisting of std 
followed by zero
+::
+
+  All module-names either beginning with an identifier consisting of std 
followed by zero
   or more digits or containing a reserved identifier ([lex.name]) are reserved 
and shall not
   be specified in a module-declaration; no diagnostic is required. If any 
identifier in a reserved
   module-name is a reserved identifier, the module name is reserved for use by 
C++ implementations;
@@ -297,7 +299,7 @@ in the front of the module declaration like:
 .. code-block:: c++
 
   # __LINE_NUMBER__ __FILE__ 1 3
-  export moudle std;
+  export module std;
 
 Here the `__LINE_NUMBER__` is the actual line number of the corresponding 
line. The `__FILE__` means the filename
 of the translation unit. The `1` means the following is a new file. And `3` 
means this is a system header/file so



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


[PATCH] D138031: Add clang-tidy check for missing move constructors

2022-11-15 Thread Martin Bidlingmaier via Phabricator via cfe-commits
mbid added inline comments.



Comment at: clang/include/clang/Lex/Token.h:101
-return is(K1) || is(K2);
-  }
   template  bool isOneOf(tok::TokenKind K1, Ts... Ks) const {

This change is necessary to make `isOneOf` work with a single argument (and 
also no arguments). isOneOf is called from findNextAnyTokenKind, which is used 
in this patch with a single token type.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138031

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


[PATCH] D138010: [AArch64][ARM] add Armv8.9-a/Armv9.4-a identifier support

2022-11-15 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas added inline comments.



Comment at: llvm/include/llvm/Support/ARMTargetParser.def:127
+  ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_CRC | ARM::AEK_RAS |
+  ARM::AEK_DOTPROD | ARM::AEK_BF16 | ARM::AEK_I8MM))
 ARM_ARCH("armv9-a", ARMV9A, "9-A", "v9a",

stuij wrote:
> tmatheson wrote:
> > No ARM::AEK_SHA2 | ARM::AEK_AES? Or does 8.8 need updated?
> Yes, I think 8.8 needs update, and some other arches as well.
> 
> In the A profile armarm, section A2.3, it is stated that from 8.2 SME(2) and 
> EAS aren't by default included in the cryptographic extension as the 
> Cryptographic Extension in an implementation is subject to export license 
> controls. Inclusion of the extension can be either/or or none, so we should 
> default to none.
> 
> I think this should be handled by  separate patch.
The v8.9-a entry on AArch64TargetParser.def includes both `AEK_SHA2` and 
`AEK_AES`. Can you also update it to make sure they are consistent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138010

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


[PATCH] D138035: Fix clang-tidy util findNextTokenSkippingComments

2022-11-15 Thread Martin Bidlingmaier via Phabricator via cfe-commits
mbid created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a reviewer: njames93.
Herald added a project: All.
mbid requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

The function did not update the Start source location, resulting in an
infinite loop if the first token was a comment token.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138035

Files:
  clang-tools-extra/clang-tidy/utils/LexerUtils.cpp


Index: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -81,9 +81,13 @@
   const SourceManager &SM,
   const LangOptions &LangOpts) {
   Optional CurrentToken;
-  do {
+  while (true) {
 CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
-  } while (CurrentToken && CurrentToken->is(tok::comment));
+if (!CurrentToken || !CurrentToken->is(tok::comment)) {
+  break;
+}
+Start = CurrentToken->getLastLoc();
+  }
   return CurrentToken;
 }
 


Index: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
===
--- clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
+++ clang-tools-extra/clang-tidy/utils/LexerUtils.cpp
@@ -81,9 +81,13 @@
   const SourceManager &SM,
   const LangOptions &LangOpts) {
   Optional CurrentToken;
-  do {
+  while (true) {
 CurrentToken = Lexer::findNextToken(Start, SM, LangOpts);
-  } while (CurrentToken && CurrentToken->is(tok::comment));
+if (!CurrentToken || !CurrentToken->is(tok::comment)) {
+  break;
+}
+Start = CurrentToken->getLastLoc();
+  }
   return CurrentToken;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137948: [clang][dataflow] Add widening API and implement it for built-in boolean model.

2022-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 475468.
ymandel added a comment.

Rewrite widen-related comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137948

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -9,7 +9,6 @@
 #include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
@@ -63,7 +62,7 @@
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match,
+  runDataflow(Code, std::move(Match),
   {ApplyBuiltinTransfer ? TransferOptions{}
 : llvm::Optional()},
   Std, TargetFun);
@@ -3255,8 +3254,7 @@
 
 TEST(TransferTest, LoopWithAssignmentConverges) {
   std::string Code = R"(
-
-bool &foo();
+bool foo();
 
 void target() {
do {
@@ -3285,9 +3283,45 @@
   });
 }
 
-TEST(TransferTest, LoopWithReferenceAssignmentConverges) {
+TEST(TransferTest, LoopWithStagedAssignments) {
   std::string Code = R"(
+bool foo();
+
+void target() {
+  bool Bar = false;
+  bool Err = false;
+  while (foo()) {
+if (Bar)
+  Err = true;
+Bar = true;
+/*[[p]]*/
+  }
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+const ValueDecl *ErrDecl = findValueDecl(ASTCtx, "Err");
+ASSERT_THAT(ErrDecl, NotNull());
+
+auto &BarVal = *cast(Env.getValue(*BarDecl, SkipPast::None));
+auto &ErrVal = *cast(Env.getValue(*ErrDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(BarVal));
+// An unsound analysis, for example only evaluating the loop once, can
+// conclude that `Err` is false. So, we test that this conclusion is not
+// reached.
+EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(ErrVal)));
+  });
+}
 
+TEST(TransferTest, LoopWithReferenceAssignmentConverges) {
+  std::string Code = R"(
 bool &foo();
 
 void target() {
@@ -3299,9 +,8 @@
   } while (true);
 }
   )";
-  // The key property that we are verifying is implicit in `runDataflow` --
-  // namely, that the analysis succeeds, rather than hitting the maximum number
-  // of iterations.
+  // The key property that we are verifying is that the analysis succeeds,
+  // rather than hitting the maximum number of iterations.
   runDataflow(
   Code,
   [](const llvm::StringMap> &Results,
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -23,6 +23,7 @@
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "clang/Analysis/FlowSensitive/DataflowWorklist.h"
 #include "clang/Analysis/FlowSensitive/Transfer.h"
 #include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
@@ -69,6 +70,20 @@
   return BlockPos - Pred.succ_begin();
 }
 
+static bool isLoopHead(const CFGBlock &B) {
+  if (const auto *T = B.getTerminatorStmt())
+switch (T->getStmtClass()) {
+  case Stmt::WhileStmtClass:
+  case Stmt::DoStmtClass:
+  case Stmt::ForStmtClass:
+return true;
+  default:
+return false;
+}
+
+  return false;
+}
+
 // The return type of the visit functions in TerminatorVisitor. The first
 // element represents the terminator expression (that is the conditional
 // expression in case of a path split in

[PATCH] D137948: [clang][dataflow] Add widening API and implement it for built-in boolean model.

2022-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 475469.
ymandel marked an inline comment as done.
ymandel added a comment.

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137948

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -9,7 +9,6 @@
 #include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
@@ -63,7 +62,7 @@
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match,
+  runDataflow(Code, std::move(Match),
   {ApplyBuiltinTransfer ? TransferOptions{}
 : llvm::Optional()},
   Std, TargetFun);
@@ -3255,8 +3254,7 @@
 
 TEST(TransferTest, LoopWithAssignmentConverges) {
   std::string Code = R"(
-
-bool &foo();
+bool foo();
 
 void target() {
do {
@@ -3285,9 +3283,45 @@
   });
 }
 
-TEST(TransferTest, LoopWithReferenceAssignmentConverges) {
+TEST(TransferTest, LoopWithStagedAssignments) {
   std::string Code = R"(
+bool foo();
+
+void target() {
+  bool Bar = false;
+  bool Err = false;
+  while (foo()) {
+if (Bar)
+  Err = true;
+Bar = true;
+/*[[p]]*/
+  }
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+const ValueDecl *ErrDecl = findValueDecl(ASTCtx, "Err");
+ASSERT_THAT(ErrDecl, NotNull());
+
+auto &BarVal = *cast(Env.getValue(*BarDecl, SkipPast::None));
+auto &ErrVal = *cast(Env.getValue(*ErrDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(BarVal));
+// An unsound analysis, for example only evaluating the loop once, can
+// conclude that `Err` is false. So, we test that this conclusion is not
+// reached.
+EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(ErrVal)));
+  });
+}
 
+TEST(TransferTest, LoopWithReferenceAssignmentConverges) {
+  std::string Code = R"(
 bool &foo();
 
 void target() {
@@ -3299,9 +,8 @@
   } while (true);
 }
   )";
-  // The key property that we are verifying is implicit in `runDataflow` --
-  // namely, that the analysis succeeds, rather than hitting the maximum number
-  // of iterations.
+  // The key property that we are verifying is that the analysis succeeds,
+  // rather than hitting the maximum number of iterations.
   runDataflow(
   Code,
   [](const llvm::StringMap> &Results,
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -23,6 +23,7 @@
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "clang/Analysis/FlowSensitive/DataflowWorklist.h"
 #include "clang/Analysis/FlowSensitive/Transfer.h"
 #include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
@@ -69,6 +70,20 @@
   return BlockPos - Pred.succ_begin();
 }
 
+static bool isLoopHead(const CFGBlock &B) {
+  if (const auto *T = B.getTerminatorStmt())
+switch (T->getStmtClass()) {
+  case Stmt::WhileStmtClass:
+  case Stmt::DoStmtClass:
+  case Stmt::ForStmtClass:
+return true;
+  default:
+return false;
+}
+
+  return false;
+}
+
 // The return type of the visit functions in TerminatorVisitor. The first
 // element represents the terminator expression (that is the conditional
 // expression in case

[PATCH] D127284: [clang-repl] Support statements on global scope in incremental mode.

2022-11-15 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev updated this revision to Diff 475470.
v.g.vassilev marked an inline comment as not done.
v.g.vassilev added a comment.

Rebase + fix the failure on windows


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

https://reviews.llvm.org/D127284

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DeclNodes.td
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclBase.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTentative.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Serialization/ASTCommon.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/Interpreter/disambiguate-decl-stmt.cpp
  clang/test/Interpreter/execute-stmts.cpp
  clang/test/Interpreter/stmt-serialization.cpp
  clang/unittests/Interpreter/InterpreterTest.cpp

Index: clang/unittests/Interpreter/InterpreterTest.cpp
===
--- clang/unittests/Interpreter/InterpreterTest.cpp
+++ clang/unittests/Interpreter/InterpreterTest.cpp
@@ -124,14 +124,8 @@
   auto *PTU1 = R1->TUPart;
   EXPECT_EQ(2U, DeclsSize(PTU1));
 
-  // FIXME: Add support for wrapping and running statements.
   auto R2 = Interp->Parse("var1++; printf(\"var1 value %d\\n\", var1);");
-  EXPECT_FALSE(!!R2);
-  using ::testing::HasSubstr;
-  EXPECT_THAT(DiagnosticsOS.str(),
-  HasSubstr("error: unknown type name 'var1'"));
-  auto Err = R2.takeError();
-  EXPECT_EQ("Parsing failed.", llvm::toString(std::move(Err)));
+  EXPECT_TRUE(!!R2);
 }
 
 TEST(InterpreterTest, UndoCommand) {
Index: clang/test/Interpreter/stmt-serialization.cpp
===
--- /dev/null
+++ clang/test/Interpreter/stmt-serialization.cpp
@@ -0,0 +1,19 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -std=c++20 -fincremental-extensions -fmodules-cache-path=%t \
+// RUN:-x c++ %s -verify
+// expected-no-diagnostics
+
+#pragma clang module build TopLevelStmt
+module TopLevelStmt { module Statements {} }
+#pragma clang module contents
+
+#pragma clang module begin TopLevelStmt.Statements
+extern "C" int printf(const char*,...);
+int i = 0;
+i++;
+#pragma clang module end /*TopLevelStmt.Statements*/
+#pragma clang module endbuild /*TopLevelStmt*/
+
+#pragma clang module import TopLevelStmt.Statements
+
+printf("Value of i is '%d'", i);
Index: clang/test/Interpreter/execute-stmts.cpp
===
--- /dev/null
+++ clang/test/Interpreter/execute-stmts.cpp
@@ -0,0 +1,33 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl -Xcc -Xclang -Xcc  -verify | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions %s
+
+// expected-no-diagnostics
+
+extern "C" int printf(const char*,...);
+
+template  T call() { printf("called\n"); return T(); }
+call();
+// CHECK: called
+
+int i = 1;
+++i;
+printf("i = %d\n", i);
+// CHECK: i = 2
+
+namespace Ns { void f(){ i++; } }
+Ns::f();
+
+void g() { ++i; }
+g();
+::g();
+
+printf("i = %d\n", i);
+// CHECK-NEXT: i = 5
+
+for (; i > 4; --i) printf("i = %d\n", i);
+// CHECK-NEXT: i = 5
+
+int j = i; printf("j = %d\n", j);
+// CHECK-NEXT: j = 4
Index: clang/test/Interpreter/disambiguate-decl-stmt.cpp
===
--- /dev/null
+++ clang/test/Interpreter/disambiguate-decl-stmt.cpp
@@ -0,0 +1,51 @@
+// REQUIRES: host-supports-jit
+// UNSUPPORTED: system-aix
+// RUN: cat %s | clang-repl -Xcc -std=c++20 -Xcc -Xclang -Xcc -verify | FileCheck %s
+// RUN: %clang_cc1 -fsyntax-only -verify -fincremental-extensions -std=c++20 %s
+
+// expected-no-diagnostics
+
+extern "C" int printf(const char*,...);
+
+// Decls which are hard to disambiguate
+
+// Operators.
+struct S1 { operator int(); };
+S1::operator int() { return 0; }
+
+// Dtors
+using I = int;
+I x = 10;
+x.I::~I();
+x = 20;
+
+// Ctors
+
+// Deduction guide
+template struct A { A(); A(T); };
+A() -> A;
+
+struct S2 { S2(); };
+S2::S2() = default;
+
+namespace N { struct S { S(); }; }
+N::S::S() { printf("N::S::S()\n"); }
+N::S s;
+// CHECK: N::S::S()
+
+namespace Ns {namespace Ns { void Ns(); void Fs();}}
+void Ns::Ns::Ns() { printf("void Ns::Ns::Ns()\n"); }
+void Ns::Ns::Fs() {}
+
+Ns::Ns::Fs();
+Ns::Ns::Ns();
+// CHECK-NEXT: void Ns:

[PATCH] D138010: [AArch64][ARM] add Armv8.9-a/Armv9.4-a identifier support

2022-11-15 Thread Lucas Prates via Phabricator via cfe-commits
pratlucas added inline comments.



Comment at: llvm/unittests/Support/TargetParserTest.cpp:532
+  testARMArch("armv9.4-a", "generic", "v9.4a",
+  ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(

Should this be testing for `ARMBuildAttrs::CPUArch::v9_A` instead?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138010

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


[PATCH] D137948: [clang][dataflow] Add widening API and implement it for built-in boolean model.

2022-11-15 Thread Eric Li via Phabricator via cfe-commits
li.zhe.hua added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:166
+
+  // The second-choice implementation: `transferBranch` is implemented. No-op.
+  template 

I think this is missing a word.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:227
+  /// Widens the environment point-wise, using `PrevEnv` as needed to inform 
the
+  /// approximation. by taking the intersection of storage locations and values
+  /// that are stored in them. Distinct values that are assigned to the same




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137948

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


[PATCH] D138031: Add clang-tidy check for missing move constructors

2022-11-15 Thread Eugene Zelenko via Phabricator via cfe-commits
Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance/missing-move-constructor.rst:6
+
+Warns when a class has a copy constructor but not move constructor.
+

Please synchronize this sentence with Release Notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138031

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


[PATCH] D137698: [include-cleaner] Add self-contained file support for PragmaIncludes.

2022-11-15 Thread Kadir Cetinkaya via Phabricator via cfe-commits
kadircet added inline comments.



Comment at: 
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h:100
 
+  /// Contains all non self-contained files during the parsing.
+  llvm::DenseSet NonSelfContainedFiles;

s/during/detected during/



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:18
+// up the include stack.
+static const FileEntry *getResponsibleHeader(FileID FID,
+ const SourceManager &SM,

what about `selfContainedHeader` instead of `getResponsibleHeader`, 
responsibility is a concept that we rather define between which header should 
include what headers (e.g. a header providing return types of its APIs)



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:24
+  while (FE && FID != SM.getMainFileID() && !PI.isSelfContained(FE)) {
+FID = SM.getFileID(SM.getIncludeLoc(FID));
+FE = SM.getFileEntryForID(FID);

`FID =  SM.getDecomposedIncludedLoc(FID).first;`

which does a cached lookup, so might be faster than `getFileID`, especially 
when performing repeated lookups (e.g. a single file importing multiple 
tablegen'd headers).



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:37
 // FIXME: Handle macro locations.
 // FIXME: Handle non self-contained files.
 FileID FID = SM.getFileID(Loc.physical());

drop fixme



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:38
 // FIXME: Handle non self-contained files.
 FileID FID = SM.getFileID(Loc.physical());
+const auto *FE = getResponsibleHeader(FID, SM, PI);

can you drop `FID` (as it isn't used elsewhere) and just pass `Loc.physical()` 
into helper?



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:39
 FileID FID = SM.getFileID(Loc.physical());
-const auto *FE = SM.getFileEntryForID(FID);
+const auto *FE = getResponsibleHeader(FID, SM, PI);
 if (!FE)

i am not sure if sequencing of this and rest of the IWYU pragma handling is the 
right actually.

e.g. consider a scenario in which:
private.h:
```
// IWYU private, include "public.h"
void foo();
```
private-impl.h:
```
#pragma once
#include "private.h"
```

we'll actually now use `private-impl.h` as provider for `foo` rather than 
`public.h`, a similar argument goes for export pragma. i think intent of the 
developer is a lot more explicit when we have these pragmas around, so we 
should prioritize them and then pick the self contained one, if it's still 
needed. WDYT?



Comment at: clang-tools-extra/include-cleaner/lib/Record.cpp:306
 
+bool PragmaIncludes::isSelfContained(const FileEntry *ID) const {
+  return !NonSelfContainedFiles.contains(ID->getUniqueID());

s/ID/File



Comment at: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp:31
 
+std::string guard(const llvm::StringRef Code) {
+  return "#pragma once\n" + Code.str();

drop the `const`



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:20
 
+std::string guard(const llvm::StringRef Code) {
+  return "#pragma once\n" + Code.str();

again `const`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137698

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


[PATCH] D137714: Do not merge traps in functions annotated optnone

2022-11-15 Thread Dan Liew via Phabricator via cfe-commits
delcypher accepted this revision.
delcypher added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137714

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


[PATCH] D135987: [clangBasic] Refactor StaticAnalyzer to use `clang::SarifDocumentWriter`

2022-11-15 Thread Vaibhav Yenamandra via Phabricator via cfe-commits
vaibhav.y updated this revision to Diff 475475.
vaibhav.y added a comment.

Update FileCheck tests to include now serialized file URIs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135987

Files:
  clang/include/clang/Basic/SourceLocation.h
  clang/lib/Basic/Sarif.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-diagnostics-taint-test.c.sarif
  
clang/test/Analysis/diagnostics/Inputs/expected-sarif/sarif-multi-diagnostic-test.c.sarif
  clang/test/Frontend/sarif-diagnostics.cpp
  clang/unittests/Basic/SarifTest.cpp

Index: clang/unittests/Basic/SarifTest.cpp
===
--- clang/unittests/Basic/SarifTest.cpp
+++ clang/unittests/Basic/SarifTest.cpp
@@ -292,7 +292,7 @@
 TEST_F(SarifDocumentWriterTest, checkSerializingArtifacts) {
   // GIVEN:
   const std::string ExpectedOutput =
-  R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":40,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":14,"startColumn":14,"startLine":3}}}],"message":{"text":"expected ';' after top level declarator"},"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif test runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif test","rules":[{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":"Example rule created during unit tests"},"id":"clang.unittest","name":"clang unit test"}],"version":"1.0.0"}}}],"version":"2.1.0"})";
+  R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":40,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0,"uri":"file:///main.cpp"},"region":{"endColumn":14,"startColumn":14,"startLine":3}}}],"message":{"text":"expected ';' after top level declarator"},"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif test runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif test","rules":[{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":"Example rule created during unit tests"},"id":"clang.unittest","name":"clang unit test"}],"version":"1.0.0"}}}],"version":"2.1.0"})";
 
   SarifDocumentWriter Writer{SourceMgr};
   const SarifRule &Rule =
@@ -332,7 +332,7 @@
 TEST_F(SarifDocumentWriterTest, checkSerializingCodeflows) {
   // GIVEN:
   const std::string ExpectedOutput =
-  R"({"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":27,"location":{"index":1,"uri":"file:///test-header-1.h"},"mimeType":"text/plain","roles":["resultFile"]},{"length":30,"location":{"index":2,"uri":"file:///test-header-2.h"},"mimeType":"text/plain","roles":["resultFile"]},{"length":28,"location":{"index":3,"uri":"file:///test-header-3.h"},"mimeType":"text/plain","roles":["resultFile"]},{"length":41,"location":{"index":0,"uri":"file:///main.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"codeFlows":[{"threadFlows":[{"locations":[{"importance":"essential","location":{"message":{"text":"Message #1"},"physicalLocation":{"artifactLocation":{"index":1},"region":{"endColumn":8,"endLine":2,"startColumn":1,"startLine":1,{"importance":"important","location":{"message":{"text":"Message #2"},"physicalLocation":{"artifactLocation":{"index":2},"region":{"endColumn":8,"endLine":2,"startColumn":1,"startLine":1,{"importance":"unimportant","location":{"message":{"text":"Message #3"},"physicalLocation":{"artifactLocation":{"index":3},"region":{"endColumn":8,"endLine":2,"startColumn":1,"startLine":1]}]}],"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":8,"endLine":2,"startColumn":5,"startLine":2}}}],"message":{"text":"Redefinition of 'foo'"},"ruleId":"clang.unittest","ruleIndex":0}],"tool":{"driver":{"fullName":"sarif test runner","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"sarif test","rules":[{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":"Example rule created during unit tests"},"id":"clang.unittest","name":"clang unit test"}],"version":"1.0.0"}}}],"version":"2.1.0"})";
+   

[PATCH] D135987: [clangBasic] Refactor StaticAnalyzer to use `clang::SarifDocumentWriter`

2022-11-15 Thread Vaibhav Yenamandra via Phabricator via cfe-commits
vaibhav.y added a comment.

In D135987#3906253 , @aaron.ballman 
wrote:

> Thank you for this refactoring, I really like the direction it's going! I've 
> not spotted anything concerning, but if someone can double-check conformance 
> to SARIF in terms of the changes to the tests, that would be appreciated.
>
> It looks like precommit CI found a relevant failure that should be addressed.

Thank you! It was the FileCheck tests for -Wsarif-format-unstable. I think I've 
updated those correctly, but would appreciate another look. I haven't had much 
experience with FileCheck so far.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135987

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


[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

2022-11-15 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Sorry for not reviewing this sooner. This looks very good overall, but I still 
have some NITs and a few major comments. For the major points see:

- the comment about the filler and the syntactic/semantic form for the newly 
added expression,
- the comment about relying on `getFailedCandidateSet().size()`.




Comment at: clang/lib/AST/ExprCXX.cpp:1776
+}
\ No newline at end of file


NIT: add newline



Comment at: clang/lib/AST/ExprConstant.cpp:9988
+  }
+} else
+  llvm_unreachable(

NIT: maybe add braces as the other branches have them? This seems to be in the 
[style 
guide](https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements):
> readability is also harmed if an if/else chain does not use braced bodies for 
> either all or none of its members



Comment at: clang/lib/AST/ExprConstant.cpp:10957
+  unsigned NumElts = InitExprs.size();
+  Result = APValue(APValue::UninitArray(), NumElts, NumElts);
+

Could we add an assertion that the array size and the `InitExprs.size()` are 
the same?
E.g. it can differ in the case of `InitListExpr` (probably due to the filler 
optimization?), it would be nice to add a sanity check here.



Comment at: clang/lib/AST/JSONNodeDumper.cpp:852
 case VarDecl::ListInit: JOS.attribute("init", "list"); break;
+case VarDecl::ParenListInit:
+  JOS.attribute("init", "paren-list");

NIT: maybe use the same formatting as other switch cases for constistency?



Comment at: clang/lib/CodeGen/CGExprAgg.cpp:479
 void AggExprEmitter::EmitArrayInit(Address DestPtr, llvm::ArrayType *AType,
-   QualType ArrayQTy, InitListExpr *E) {
-  uint64_t NumInitElements = E->getNumInits();
+   QualType ArrayQTy, Expr *ExprToVisit,
+   ArrayRef Args) {

NIT: maybe add a comment that `ExprToVisit` is either `InitListExpr` or 
`CXXParenInitListExpr`?



Comment at: clang/lib/CodeGen/CGExprAgg.cpp:581
+  Expr *filler = nullptr;
+  if (auto *ILE = dyn_cast(ExprToVisit))
+filler = ILE->getArrayFiller();

- Should we have a filler for `CXXParenInitListExpr` too?
  It seems like an important optimization and could have large effect on 
compile times if we don't 

- Same question for semantic and syntactic-form (similar to `InitListExpr`): 
should we have it here?
  I am not sure if it's semantically required (probably not?), but that's 
definitely something that `clang-tidy` and other source tools will rely on.

We should probably share the code there, I suggest moving it to a shared base 
class and using it where appropriate instead of the derived classes.



Comment at: clang/lib/Sema/SemaInit.cpp:6169
+  Failure == FK_ConstructorOverloadFailed &&
+  getFailedCandidateSet().size() == 3) {
+// C++20 [dcl.init] 17.6.2.2:

It seems shaky to rely on the size of `getFailedCandidateSet`.
What are we trying to achieve here? Is there a more direct way to specify it?



Comment at: clang/lib/Sema/SemaInit.cpp:6188
+//  (6.7.7), and there is no brace elision.
+TryOrBuildParenListInitialization(S, Entity, Kind, Args, *this,
+  /*VerifyOnly=*/true);

NIT: maybe move the full comment into the body of the function?
It describes in detail what the body of the function does and it would be 
easier to understand whether the code fits the intention in the standard.
Having the first part of the comment here would still be useful, probably.



Comment at: clang/lib/Serialization/ASTReaderStmt.cpp:2174
+  E->NumExprs = Record.readInt();
+  for (unsigned I = 0; I < E->NumExprs; I++)
+E->getTrailingObjects()[I] = Record.readSubExpr();

Could we assign the `CXXParenInitListExpr.NumExprs` on construction and add the 
assertion sanity-checking the numbers here:
```
unsigned NumExprs = Record.readInt();
(void)NumExprs; // to avoid unused warnings in NDEBUG builds.
assert(NumExprs == E->NumExprs);
```

(Looked up how `DesignatedInitExpr` does a similar thing, probably good for 
consistency in addition to added safety)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129531

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


[PATCH] D137948: [clang][dataflow] Add widening API and implement it for built-in boolean model.

2022-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel updated this revision to Diff 475478.
ymandel marked 2 inline comments as done.
ymandel added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137948

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/Value.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -9,7 +9,6 @@
 #include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
-#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
@@ -63,7 +62,7 @@
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match,
+  runDataflow(Code, std::move(Match),
   {ApplyBuiltinTransfer ? TransferOptions{}
 : llvm::Optional()},
   Std, TargetFun);
@@ -3255,8 +3254,7 @@
 
 TEST(TransferTest, LoopWithAssignmentConverges) {
   std::string Code = R"(
-
-bool &foo();
+bool foo();
 
 void target() {
do {
@@ -3285,9 +3283,45 @@
   });
 }
 
-TEST(TransferTest, LoopWithReferenceAssignmentConverges) {
+TEST(TransferTest, LoopWithStagedAssignments) {
   std::string Code = R"(
+bool foo();
+
+void target() {
+  bool Bar = false;
+  bool Err = false;
+  while (foo()) {
+if (Bar)
+  Err = true;
+Bar = true;
+/*[[p]]*/
+  }
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
+const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+const ValueDecl *ErrDecl = findValueDecl(ASTCtx, "Err");
+ASSERT_THAT(ErrDecl, NotNull());
+
+auto &BarVal = *cast(Env.getValue(*BarDecl, SkipPast::None));
+auto &ErrVal = *cast(Env.getValue(*ErrDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(BarVal));
+// An unsound analysis, for example only evaluating the loop once, can
+// conclude that `Err` is false. So, we test that this conclusion is not
+// reached.
+EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(ErrVal)));
+  });
+}
 
+TEST(TransferTest, LoopWithReferenceAssignmentConverges) {
+  std::string Code = R"(
 bool &foo();
 
 void target() {
@@ -3299,9 +,8 @@
   } while (true);
 }
   )";
-  // The key property that we are verifying is implicit in `runDataflow` --
-  // namely, that the analysis succeeds, rather than hitting the maximum number
-  // of iterations.
+  // The key property that we are verifying is that the analysis succeeds,
+  // rather than hitting the maximum number of iterations.
   runDataflow(
   Code,
   [](const llvm::StringMap> &Results,
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -23,6 +23,7 @@
 #include "clang/Analysis/Analyses/PostOrderCFGView.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "clang/Analysis/FlowSensitive/DataflowWorklist.h"
 #include "clang/Analysis/FlowSensitive/Transfer.h"
 #include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
@@ -69,6 +70,20 @@
   return BlockPos - Pred.succ_begin();
 }
 
+static bool isLoopHead(const CFGBlock &B) {
+  if (const auto *T = B.getTerminatorStmt())
+switch (T->getStmtClass()) {
+  case Stmt::WhileStmtClass:
+  case Stmt::DoStmtClass:
+  case Stmt::ForStmtClass:
+return true;
+  default:
+return false;
+}
+
+  return false;
+}
+
 // The return type of the visit functions in TerminatorVisitor. The first
 // element represents the terminator expression (that is the conditional
 // expression

[PATCH] D138037: [analyzer] Remove unjustified assertion from EQClass::simplify

2022-11-15 Thread Balázs Benics via Phabricator via cfe-commits
steakhal created this revision.
steakhal added reviewers: xazax.hun, NoQ, martong, ASDenysPetrov, vabridgers, 
Szelethus.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware.
Herald added a project: All.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

One might think that by merging the equivalence classes (eqclasses) of `Sym1` 
and `Sym2` symbols we would end up with a `State` in which the eqclass of 
`Sym1` and `Sym2` should now be the same. Surprisingly, it's not //always// 
true.

Such an example triggered the assertion enforcing this //unjustified// 
invariant in https://github.com/llvm/llvm-project/issues/58677.

  unsigned a, b;
  #define assert(cond) if (!(cond)) return
  
  void f(unsigned c) {
  /*(1)*/ assert(c == b);
  /*(2)*/ assert((c | a) != a);
  /*(3)*/ assert(a);
  // a = 0  =>  c | 0 != 0  =>  c != 0  =>  b != 0
  }

I believe, that this assertion hold for reasonable cases - where both 
`MemberSym` and `SimplifiedMemberSym` refer to live symbols.
It can only fail if `SimplifiedMemberSym` refers to an already dead symbol. See 
the reasoning at the very end.
In this context, I don't know any way of determining if a symbol is alive/dead, 
so I cannot refine the assertion in that way.
So, I'm proposing to drop this unjustified assertion.

---

Let me elaborate on why I think the assertion is wrong in its current shape.

Here is a quick reminder about equivalence classes in CSA.
We have 4 mappings:

1. `ClassMap`: map, associating `Symbols` with an `EquivalenceClass`.
2. `ClassMembers`: map, associating `EquivalenceClasses` with its members - 
basically an enumeration of the `Symbols` which are known to be equal.
3. `ConstraintRange`: map, associating `EquivalenceClasses` with the range 
constraint which should hold for all the members of the eqclass.
4. `DisequalityMap`: I'm omitting this, as it's irrelevant for our purposes now.

Whenever we encounter/assume that two `SymbolRefs` are equal, we update the 
`ClassMap` so that now both `SymbolRefs` are referring to the same eqclass. 
This operation is known as `merge` or `union`.
Each eqclass is uniquely identified by the `representative` symbol, but it 
could have been just a unique number or anything else - the point is that an 
eqclass is identified by something unique.
Initially, all Symbols form - by itself - a trivial eqclass, as there are no 
other Symbols to which it is assumed to be equal. A trivial eqclass has 
//notionally// exactly one member, the representative symbol.
I'm emphasizing that //notionally// because for such cases we don't store an 
entry in the `ClassMap` nor in the `ClassMembers` map, because it could be 
deduced.

By `merging` two eqclasses, we essentially do what you would think it does. An 
important thing to highlight is that the representative symbol of the resulting 
eqclass will be the same as one of the two eqclasses.
This operation should be commutative, so that `merge(eq1,eq2)` and 
`merge(eq2,eq1)` should result in the same eqclass - except for the 
representative symbol, which is just a unique identifier of the class, a name 
if you will. Using the representative symbol of `eq1` or `eq2` should have no 
visible effect on the analysis overall.

When merging `eq1` into `eq2`, we take each of the `ClassMembers` of `eq1` and 
add them to the `ClassMembers` of `eq2` while we also redirect all the `Symbol` 
members of `eq1` to map to the `eq2` eqclass in the `ClassMap`. This way all 
members of `eq1` will refer to the correct eqclass.
After these, `eq1` key is unreachable in the `ClassMembers`, hence we can drop 
it.

---

Let's get back to the example.
Note that when I refer to symbols `a`, `b`, `c`, I'm referring to the 
`SymbolRegionValue{VarRegion{.}}` - the value of that variable.

After `(1)`, we will have a constraint `c == b : [1,1]` and an eqclass `c,b` 
with the `c` representative symbol.
After `(2)`, we will have an additional constraint `c|b != a : [1,1]` with the 
same eqclass. We will also have disequality info about that `c|a` is disequal 
to `a` - and the other way around.
However, after the full-expression, `c` will become dead. This kicks in the 
garbage collection, which transforms the state into this:

- We no longer have any constraints, because only `a` is alive, for which we 
don't have any constraints.
- We have a single (non-trivial) eqclass with a single element `b` and 
representative symbol `c`. (Dead symbols can be representative symbols.)
- We have the same disequality info as before.

At `(3)` within the false branch, `a` get perfectly constrained to zero. This 
kicks in the simplification, so we try to substitute (simplify) the variable in 
each SymExpr-tree. In our case, it means that the `c|a != a : [1,1]` constraint 
gets re-evaluated as `c|0 != 0 : [1,1]`, which is `c != 0 : [1,1]`.
Under the hood, it means that we will call `

[PATCH] D137948: [clang][dataflow] Add widening API and implement it for built-in boolean model.

2022-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel marked 3 inline comments as done and 2 inline comments as done.
ymandel added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:68
+/// `LatticeT` can optionally provide the following members:
+///  * `bool widen(const LatticeT &Previous)` - chooses a lattice element that
+/// approximates the join of this element with the argument. Widen is 
called

xazax.hun wrote:
> We should document what is the return value for. Also I see 
> `LatticeJoinEffect` instead of bool in the code, but I might be confused.
No, you're right. :) I've completely rewritten the comments -- they were 
outdated and wrong. They now match what we have in TypeErasedDatafllowAnalysis. 
Most importantly, we've dropped any mention of defaulting to `join` which is 
wrong both in fact and conceptually.   Our targeted use of widen means it 
should not default to `join` -- that would be pointless, since it's 
prerequisites would guarantee that `join(prev, cur) = cur`, making the join 
pointless. It instead defaults to an equivalence check.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:108
 
+  LatticeJoinEffect widenTypeErased(TypeErasedLattice &Current,
+const TypeErasedLattice &Previous) final {

xazax.hun wrote:
> I wonder if `LatticeJoinEffect` is the right name if we also use this for 
> widening. (Also, are we in the process of removing these?)
I had the same thought. How about just `LatticeEffect`? Open to other 
suggestions as well. Either way, though, I should update in a separate patch in 
case that breaks something unexpected.

As for removing -- yes, we should remove from join, because we never use it. 
But, it actually makes sense here.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:227
+  /// Widens the environment point-wise, using `PrevEnv` as needed to inform 
the
+  /// approximation. by taking the intersection of storage locations and values
+  /// that are stored in them. Distinct values that are assigned to the same

li.zhe.hua wrote:
> 
I actually just deleted most of the comment -- I don't think the details were 
helpful.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:374-378
+  Environment WidenedEnv(*DACtx);
+
+  WidenedEnv.CallStack = CallStack;
+  WidenedEnv.ReturnLoc = ReturnLoc;
+  WidenedEnv.ThisPointeeLoc = ThisPointeeLoc;

xazax.hun wrote:
> Shouldn't we have a simple copy ctor from PrefEnv that could populate all 
> these fields?
> 
> Also, this makes me wonder if we actually want to move some of these out from 
> environment, since these presumably would not change between basic blocks.
> Shouldn't we have a simple copy ctor from PrefEnv that could populate all 
> these fields?
We *do* have such a copy constructor. The problem is that it's overkill because 
we want to build LocToVal ourselves by point-wise widening of the elements. For 
that matter, I wish we could share, rather than copy, `DeclToLoc` and 
`ExprToLoc` but I don't know of any way to do that. Alternatively: is it 
possible that we can update in place and don't need a separate `WidenedEnv`?

> Also, this makes me wonder if we actually want to move some of these out from 
> environment, since these presumably would not change between basic blocks.
Agreed. Added a fixme with an associated issue to track.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137948

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


[PATCH] D138037: [analyzer] Remove unjustified assertion from EQClass::simplify

2022-11-15 Thread Balázs Benics via Phabricator via cfe-commits
steakhal updated this revision to Diff 475480.
steakhal added a comment.

Add test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138037

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/symbol-simplification-symplify-results-in-dead-symbol.cpp


Index: 
clang/test/Analysis/symbol-simplification-symplify-results-in-dead-symbol.cpp
===
--- /dev/null
+++ 
clang/test/Analysis/symbol-simplification-symplify-results-in-dead-symbol.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+#define assert(cond) if (!(cond)) return
+
+unsigned a, b;
+void f(unsigned c) {
+  assert(c == b);
+  assert((c | a) != a);
+  assert(a); // no-crash
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2616,7 +2616,18 @@
   if (OldState == State)
 continue;
 
-  assert(find(State, MemberSym) == find(State, SimplifiedMemberSym));
+  // Be aware that `SimplifiedMemberSym` might refer to an already dead
+  // symbol. In that case, the eqclass of that might not be the same as the
+  // eqclass of `MemberSym`. This is because the dead symbols are not
+  // preserved in the `ClassMap`, hence
+  // `find(State, SimplifiedMemberSym)` will result in a trivial eqclass
+  // compared to the eqclass of `MemberSym`.
+  // These eqclasses should be the same if `SimplifiedMemberSym` is alive.
+  // --> assert(find(State, MemberSym) == find(State, SimplifiedMemberSym))
+  //
+  // Note that `MemberSym` must be alive here since that is from the
+  // `ClassMembers` where all the symbols are alive.
+
   // Remove the old and more complex symbol.
   State = find(State, MemberSym).removeMember(State, MemberSym);
 


Index: clang/test/Analysis/symbol-simplification-symplify-results-in-dead-symbol.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-simplification-symplify-results-in-dead-symbol.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core -verify %s
+// expected-no-diagnostics
+
+#define assert(cond) if (!(cond)) return
+
+unsigned a, b;
+void f(unsigned c) {
+  assert(c == b);
+  assert((c | a) != a);
+  assert(a); // no-crash
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -2616,7 +2616,18 @@
   if (OldState == State)
 continue;
 
-  assert(find(State, MemberSym) == find(State, SimplifiedMemberSym));
+  // Be aware that `SimplifiedMemberSym` might refer to an already dead
+  // symbol. In that case, the eqclass of that might not be the same as the
+  // eqclass of `MemberSym`. This is because the dead symbols are not
+  // preserved in the `ClassMap`, hence
+  // `find(State, SimplifiedMemberSym)` will result in a trivial eqclass
+  // compared to the eqclass of `MemberSym`.
+  // These eqclasses should be the same if `SimplifiedMemberSym` is alive.
+  // --> assert(find(State, MemberSym) == find(State, SimplifiedMemberSym))
+  //
+  // Note that `MemberSym` must be alive here since that is from the
+  // `ClassMembers` where all the symbols are alive.
+
   // Remove the old and more complex symbol.
   State = find(State, MemberSym).removeMember(State, MemberSym);
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138010: [AArch64][ARM] add Armv8.9-a/Armv9.4-a identifier support

2022-11-15 Thread Ties Stuij via Phabricator via cfe-commits
stuij updated this revision to Diff 475489.
stuij added a comment.

addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138010

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-v89a.c
  clang/test/Driver/aarch64-v94a.c
  clang/test/Driver/arm-cortex-cpus-1.c
  clang/test/Preprocessor/arm-target-features.c
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/Support/AArch64TargetParser.def
  llvm/include/llvm/Support/ARMTargetParser.def
  llvm/lib/Support/ARMTargetParser.cpp
  llvm/lib/Support/ARMTargetParserCommon.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/AArch64/AArch64.td
  llvm/lib/Target/AArch64/AArch64InstrInfo.td
  llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp
  llvm/lib/Target/ARM/ARM.td
  llvm/lib/Target/ARM/ARMSubtarget.h
  llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
  llvm/unittests/Support/TargetParserTest.cpp

Index: llvm/unittests/Support/TargetParserTest.cpp
===
--- llvm/unittests/Support/TargetParserTest.cpp
+++ llvm/unittests/Support/TargetParserTest.cpp
@@ -32,10 +32,12 @@
 "armv8a",   "armv8l",  "armv8.1-a",  "armv8.1a","armv8.2-a",
 "armv8.2a", "armv8.3-a",   "armv8.3a",   "armv8.4-a",   "armv8.4a",
 "armv8.5-a","armv8.5a","armv8.6-a",  "armv8.6a","armv8.7-a",
-"armv8.7a", "armv8.8-a",   "armv8.8a",   "armv8-r", "armv8r",
-"armv8-m.base", "armv8m.base", "armv8-m.main",   "armv8m.main", "iwmmxt",
-"iwmmxt2",  "xscale",  "armv8.1-m.main", "armv9-a", "armv9",
-"armv9a",   "armv9.1-a",   "armv9.1a",   "armv9.2-a",   "armv9.2a",
+"armv8.7a", "armv8.8-a",   "armv8.8a",   "armv8.9-a",   "armv8.9a",
+"armv8-r",  "armv8r",  "armv8-m.base",   "armv8m.base", "armv8-m.main",
+"armv8m.main",  "iwmmxt",  "iwmmxt2","xscale",  "armv8.1-m.main",
+"armv9-a",  "armv9",   "armv9a", "armv9.1-a",   "armv9.1a",
+"armv9.2-a","armv9.2a","armv9.3-a",  "armv9.3a","armv9.4-a",
+"armv9.4a",
 };
 
 template 
@@ -510,6 +512,9 @@
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testARMArch("armv8.8-a", "generic", "v8.8a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(
+  testARMArch("armv8.9-a", "generic", "v8.9a",
+  ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(
   testARMArch("armv9-a", "generic", "v9a",
   ARMBuildAttrs::CPUArch::v9_A));
@@ -522,6 +527,9 @@
   EXPECT_TRUE(
   testARMArch("armv9.3-a", "generic", "v9.3a",
   ARMBuildAttrs::CPUArch::v9_A));
+  EXPECT_TRUE(
+  testARMArch("armv9.4-a", "generic", "v9.4a",
+  ARMBuildAttrs::CPUArch::v9_A));
   EXPECT_TRUE(
   testARMArch("armv8-r", "cortex-r52", "v8r",
   ARMBuildAttrs::CPUArch::v8_R));
@@ -852,10 +860,12 @@
 case ARM::ArchKind::ARMV8_6A:
 case ARM::ArchKind::ARMV8_7A:
 case ARM::ArchKind::ARMV8_8A:
+case ARM::ArchKind::ARMV8_9A:
 case ARM::ArchKind::ARMV9A:
 case ARM::ArchKind::ARMV9_1A:
 case ARM::ArchKind::ARMV9_2A:
 case ARM::ArchKind::ARMV9_3A:
+case ARM::ArchKind::ARMV9_4A:
   EXPECT_EQ(ARM::ProfileKind::A, ARM::parseArchProfile(ARMArch[i]));
   break;
 default:
@@ -1422,12 +1432,18 @@
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv8.8-a", "generic", "v8.8a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv8.9-a", "generic", "v8.9a",
+  ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv9-a", "generic", "v9a",
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv9.1-a", "generic", "v9.1a",
   ARMBuildAttrs::CPUArch::v8_A));
   EXPECT_TRUE(testAArch64Arch("armv9.2-a", "generic", "v9.2a",
   ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv9.3-a", "generic", "v9.3a",
+  ARMBuildAttrs::CPUArch::v8_A));
+  EXPECT_TRUE(testAArch64Arch("armv9.4-a", "generic", "v9.4a",
+  ARMBuildAttrs::CPUArch::v8_A));
 }
 
 bool testAArch64Extension(StringRef CPUName, AArch64::ArchKind AK,
Index: llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
===
--- llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
+++ llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp
@@ -890,10 +890,14 @@
   case ARM::ArchKind::ARMV8_4A:
   case ARM::ArchK

[PATCH] D138010: [AArch64][ARM] add Armv8.9-a/Armv9.4-a identifier support

2022-11-15 Thread Ties Stuij via Phabricator via cfe-commits
stuij marked an inline comment as done.
stuij added inline comments.



Comment at: llvm/include/llvm/Support/ARMTargetParser.def:127
+  ARM::AEK_HWDIVTHUMB | ARM::AEK_DSP | ARM::AEK_CRC | ARM::AEK_RAS |
+  ARM::AEK_DOTPROD | ARM::AEK_BF16 | ARM::AEK_I8MM))
 ARM_ARCH("armv9-a", ARMV9A, "9-A", "v9a",

pratlucas wrote:
> stuij wrote:
> > tmatheson wrote:
> > > No ARM::AEK_SHA2 | ARM::AEK_AES? Or does 8.8 need updated?
> > Yes, I think 8.8 needs update, and some other arches as well.
> > 
> > In the A profile armarm, section A2.3, it is stated that from 8.2 SME(2) 
> > and EAS aren't by default included in the cryptographic extension as the 
> > Cryptographic Extension in an implementation is subject to export license 
> > controls. Inclusion of the extension can be either/or or none, so we should 
> > default to none.
> > 
> > I think this should be handled by  separate patch.
> The v8.9-a entry on AArch64TargetParser.def includes both `AEK_SHA2` and 
> `AEK_AES`. Can you also update it to make sure they are consistent?
Ai, I was mistaken. For v8 we have them enabled by default for historic 
reasons. For v9 we had the opportunity to disable them. So I will actually 
update v8.9 to add them here  :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138010

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


[PATCH] D137851: [OPENMP]Initial support for at clause

2022-11-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 updated this revision to Diff 475490.
jyu2 added a comment.

Thanks Alexey for the review.  This is address his comments.


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

https://reviews.llvm.org/D137851

Files:
  clang/include/clang/AST/OpenMPClause.h
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/OpenMPKinds.def
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/OpenMPClause.cpp
  clang/lib/AST/StmtProfile.cpp
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/OpenMP/error_ast_print.cpp
  clang/test/OpenMP/error_message.cpp
  clang/tools/libclang/CIndex.cpp
  flang/lib/Semantics/check-omp-structure.cpp
  llvm/include/llvm/Frontend/OpenMP/OMP.td

Index: llvm/include/llvm/Frontend/OpenMP/OMP.td
===
--- llvm/include/llvm/Frontend/OpenMP/OMP.td
+++ llvm/include/llvm/Frontend/OpenMP/OMP.td
@@ -301,6 +301,9 @@
   let clangClass = "OMPAtomicDefaultMemOrderClause";
   let flangClass = "OmpAtomicDefaultMemOrderClause";
 }
+def OMPC_At : Clause<"at"> {
+  let clangClass = "OMPAtClause";
+}
 def OMPC_Allocate : Clause<"allocate"> {
   let clangClass = "OMPAllocateClause";
   let flangClass = "OmpAllocateClause";
@@ -527,7 +530,11 @@
 }
 def OMP_TaskYield : Directive<"taskyield"> {}
 def OMP_Barrier : Directive<"barrier"> {}
-def OMP_Error : Directive<"error"> {}
+def OMP_Error : Directive<"error"> {
+  let allowedClauses = [
+VersionedClause
+  ];
+}
 def OMP_TaskWait : Directive<"taskwait"> {
   let allowedClauses = [
 VersionedClause
Index: flang/lib/Semantics/check-omp-structure.cpp
===
--- flang/lib/Semantics/check-omp-structure.cpp
+++ flang/lib/Semantics/check-omp-structure.cpp
@@ -1868,6 +1868,7 @@
 CHECK_SIMPLE_CLAUSE(Use, OMPC_use)
 CHECK_SIMPLE_CLAUSE(Novariants, OMPC_novariants)
 CHECK_SIMPLE_CLAUSE(Nocontext, OMPC_nocontext)
+CHECK_SIMPLE_CLAUSE(At, OMPC_at)
 CHECK_SIMPLE_CLAUSE(Filter, OMPC_filter)
 CHECK_SIMPLE_CLAUSE(When, OMPC_when)
 CHECK_SIMPLE_CLAUSE(AdjustArgs, OMPC_adjust_args)
Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -2443,6 +2443,8 @@
 void OMPClauseEnqueue::VisitOMPAtomicDefaultMemOrderClause(
 const OMPAtomicDefaultMemOrderClause *) {}
 
+void OMPClauseEnqueue::VisitOMPAtClause(const OMPAtClause *) {}
+
 void OMPClauseEnqueue::VisitOMPDeviceClause(const OMPDeviceClause *C) {
   Visitor->AddStmt(C->getDevice());
 }
Index: clang/test/OpenMP/error_message.cpp
===
--- clang/test/OpenMP/error_message.cpp
+++ clang/test/OpenMP/error_message.cpp
@@ -7,19 +7,19 @@
   if (argc)
 #pragma omp error // expected-error {{'#pragma omp error' cannot be an immediate substatement}}
 if (argc) {
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
 }
   while (argc)
 #pragma omp error // expected-error {{'#pragma omp error' cannot be an immediate substatement}}
 while (argc) {
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
 }
   do
 #pragma omp error // expected-error {{'#pragma omp error' cannot be an immediate substatement}}
 while (argc)
   ;
   do {
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
   } while (argc);
   switch (argc)
 #pragma omp error // expected-error {{'#pragma omp error' cannot be an immediate substatement}}
@@ -28,47 +28,75 @@
 #pragma omp error // expected-error {{'#pragma omp error' cannot be an immediate substatement}}
   switch (argc)
   case 1: {
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
   }
   switch (argc) {
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
   case 1:
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
 break;
   default: {
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
   } break;
   }
   for (;;)
 #pragma omp error // expected-error {{'#pragma omp error' cannot be an immediate substatement}}
 for (;;) {
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
 }
 label:
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
 label1 : {
-#pragma omp error
+#pragma omp error // expected-error {{ERROR}}
 }
 if (1)
   label2:
 #pragma omp error // expected-error {{'#pragma omp error' cannot be an immediate substatement}}
 
+// expected-error@+1 {{ERROR}}
+#pragma omp error at() // expected-error {{expected 'compilation' or 'execution' in OpenMP clause 'at'}}
+

[PATCH] D137851: [OPENMP]Initial support for at clause

2022-11-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added inline comments.



Comment at: clang/lib/Parse/ParseOpenMP.cpp:2168
+Actions.ActOnOpenMPErrorDirective(Clauses, StartLoc, SourceLocation(),
+  /*InExcontext = */ false);
+break;

ABataev wrote:
> InExContext
Sorry.  Changed


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

https://reviews.llvm.org/D137851

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


[PATCH] D137851: [OPENMP]Initial support for at clause

2022-11-15 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev accepted this revision.
ABataev added a comment.
This revision is now accepted and ready to land.

LG


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

https://reviews.llvm.org/D137851

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


[PATCH] D136283: [clang-tools-extra] [clangd] Split huge generated CompletionModel.cpp into smaller files

2022-11-15 Thread Sam James via Phabricator via cfe-commits
thesamesam added a comment.

In D136283#3911832 , @sammccall wrote:

> I don't think we should add significant build-system complexity here in order 
> to support the completion model on ppc32.
> Do people actually use clangd on ppc32 machines? (The linked bug calls this a 
> clang build failure, but this code is not part of clang).
>
> If we need to support building on this toolchain, then we should probably 
> just disable the decision forest model entirely and use the heuristic 
> completion scorer instead.

I've had a think about this and I'm not aware of anyone using clangd on ppc32. 
So I agree this patch isn't worth the complexity. But it'd be nice to just 
switch to the heuristic scorer if it's somewhat easy rather than leave it 
broken (or bother having to wire up some "PPC is not supported" message). I've 
got no idea how to do that though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136283

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


[PATCH] D137531: [clang] Add the check of membership in decltype for the issue #58674

2022-11-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

T




Comment at: clang/lib/Sema/SemaExpr.cpp:2671
 
   // Check whether this might be a C++ implicit instance member access.
   // C++ [class.mfct.non-static]p3:

So what I was asking above: You're making a ton of changes to this section, 
without properly updating the comment that is trying to explain what is 
happening.  Please update this to match, particularly the bit around the % 
operands.


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

https://reviews.llvm.org/D137531

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


[PATCH] D137712: Correctly handle Substitution failure in concept specialization.

2022-11-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added inline comments.



Comment at: clang/include/clang/AST/ExprConcepts.h:103
 
+  bool hasSubstitutionFailureInArgs() const {
+return ArgsHasSubstitutionFailure;

usaxena95 wrote:
> erichkeane wrote:
> > Does this really belong here instead of as a part of the 
> > ConceptSpecializationDecl?
> I am not aware of the original goals for this. For example, 
> `ASTTemplateArgumentListInfo` is part of `ConceptReference` while 
> `TemplateArgument` is part of `ImplicitConceptSpecializationDecl`. Both of 
> them are invalid in such a case.
> I am open to suggestion to where to place this.
Yeah, the design of all of these is a little wonky unfortunately.  The 
ConceptReference is so it can be a base of a TypeConstraint as well, so 
anything that also needs to be in type-constraint belongs there.

ConceptSpecializationDecl is a recent split off ConstraintSpecializationExpr 
whose intent is to be something that later instantiation (like instantiating a 
lambda defined in a concept decl) can use to make sure we get template 
arguments correct.  So I suspect the args failure needs to be contained in that 
as well.

IF this is something that can also be invalid in a TypeConstraint, it belongs 
in ConceptReference AND ConceptSpecializationDecl separately I believe (as 
we'll likely need to check that's validity later).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137712

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


[PATCH] D137992: [asan] -fsanitize-address-outline-instrumentation -> -asan-max-inline-poisoning-size=0

2022-11-15 Thread Roy Sundahl via Phabricator via cfe-commits
rsundahl updated this revision to Diff 475497.
rsundahl added a comment.

Removed extraneous blank line.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137992

Files:
  clang/lib/Driver/SanitizerArgs.cpp


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -1256,6 +1256,8 @@
   if (AsanOutlineInstrumentation) {
 CmdArgs.push_back("-mllvm");
 CmdArgs.push_back("-asan-instrumentation-with-call-threshold=0");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-asan-max-inline-poisoning-size=0");
   }
 
   // Only pass the option to the frontend if the user requested,


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -1256,6 +1256,8 @@
   if (AsanOutlineInstrumentation) {
 CmdArgs.push_back("-mllvm");
 CmdArgs.push_back("-asan-instrumentation-with-call-threshold=0");
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-asan-max-inline-poisoning-size=0");
   }
 
   // Only pass the option to the frontend if the user requested,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

2022-11-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

@vitalybuka @rjmccall thank you for taking a look!
Since posting this, there has been a conversation in `#llvm` with @jyknight,
where it was established that the situation is even worse than i though.
Essentially, there are two situations:

1. exception escape out of non-unwinding function is guaranteed to cause 
program termination. That happens for:
  - the C++ `noexcept`
  - `__attribute__((nothrow))` (for some non-obvious reason)
  - OpenMP region lowering (even though it's not mandated by the standard)
2. exception escape out of non-unwinding function is wild UB, this is the 
semantics of
  - `__attribute__((pure))`/`__attribute__((const))`.

It is possible that `__attribute__((nothrow))` should also be in UB category,
otherwise what benefit does it bring over C++'s `noexcept`?

So, i have correctly felt that there is a problem, but the fix is going in the 
wrong way.
While i still think that we should handle case 1. in UBSan, let's first deal 
with 2.

In D137381#3926121 , @rjmccall wrote:

> I don't have a problem with adding a sanitizer like this.  IIRC, though, 
> there's a review process for adding new sanitizers; I don't remember if this 
> is the normal RFC process or something more streamlined.

For the record, i have added the entirety of `-fsanitize=implicit-conversion` 
(part of `-fsanitize=integer`)
and `-fsanitize=alignment`, and this is the first time i'm hearing of an RFC 
process.

> Also, I know @jyknight is interested in a mode that checks `-fno-exceptions` 
> more efficiently, which is of direct relevance to what you're trying here; 
> that was discussed in a forums thread 
> (https://discourse.llvm.org/t/rfc-add-call-unwindabort-to-llvm-ir/62543).

Yes, we've talked in `#llvm` about that a bit.

> The implementation and tests here don't seem to match the stated problem, 
> though.  Trying to throw out of a `noexcept` function is actually 
> well-defined — the function has to catch the exception and call 
> `std::terminate`, and we do actually generate code that way.  Your patch is 
> just adding a check at the start of the handler we emit, which means it's not 
> catching anything we're not already catching; also, again, this is a 
> well-defined execution path, so it doesn't seem appropriate to trigger UBSan 
> on it.
>
> IIRC, there are really only two potential sources of UB with `noexcept`.  The 
> first is that you can declare a function `noexcept(true)` but implement it as 
> `noexcept(false)`, which is in a class of problems (ODR violations) that I 
> think UBSan doesn't normally try to diagnose.  The second is that I think 
> it's possible to get a `noexcept(true)` function pointer for a 
> `noexcept(false)` function, maybe via some chain of casts; that would be a 
> little closer to what UBSan normally diagnoses.
>
> Meanwhile, your original test case has nothing to do with `noexcept`. 
> `__attribute__((pure))` (like `nothrow` and `-fno-exceptions`) really does 
> introduce a novel UB rule which it would be reasonable to ask the compiler to 
> check under UBSan.
>
> I think the right way to handle what you're trying to handle would be to 
> introduce some different kind of scope, a "it's UB to unwind out of this" 
> scope, and then push that scope around calls and implementations of functions 
> that use one of the UB-to-throw attributes.  You could also potentially push 
> that scope around calls to `nounwind` functions if you want to catch those 
> ODR / function-pointer cases.

Yes. Somehow i did not think that those attributes introduce UB, and not are 
just handled incorrectly,
and tried to solve this the wrong way around. That being said, this diff is not 
incorrect.
I would like to have a sanitizer for the "exception escape is guaranteed to 
cause program termination" mode,
because all of the bugs with that mode i had to deal with had rather bad 
diagnostic,
but that can be done afterwards.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137381

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


[PATCH] D137901: [Clang] `nothrow`-implying attributes should actually manifest `nothrow` attribute (PR58798)

2022-11-15 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri abandoned this revision.
lebedev.ri added a comment.

Actually, apparently those attributes really do introduce UB, so it is correct 
that they do not cause program termination.
https://reviews.llvm.org/D137381#3927923


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137901

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


[PATCH] D137570: [Clang][Sema] Refactor category declaration under CheckForIncompatibleAttributes. NFC

2022-11-15 Thread Mike Rice via Phabricator via cfe-commits
mikerice added a comment.

In D137570#3921879 , @erichkeane 
wrote:

> I haven't dealt much with the loop hint attributes, but two of my coworkers 
> have done extensive work in our downstream (@jyu2 and @mikerice), and likely 
> have feedback about how this affects downstream feedback.

Looks good to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137570

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


[PATCH] D137570: [Clang][Sema] Refactor category declaration under CheckForIncompatibleAttributes. NFC

2022-11-15 Thread Jennifer Yu via Phabricator via cfe-commits
jyu2 added a comment.

It is okay with me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137570

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


[PATCH] D137996: Add support for a backdoor driver option that enables emitting header usage information in JSON to a file

2022-11-15 Thread Jan Svoboda via Phabricator via cfe-commits
jansvoboda11 added a comment.

Mostly looks good. Is this expected to work with Clang modules at all?




Comment at: clang/include/clang/Driver/Driver.h:240
 
+  unsigned CCPrintHeadersJson : 1;
+

Please document this.



Comment at: clang/include/clang/Driver/Options.td:5659
   MarshallingInfoString>;
+def header_include_json : Flag<["-"], "header-include-json">,
+  HelpText<"Print header include info in json">,

Could this be an enum of the desired format instead? It would default to 
textual and be overwritten with something like `-header-include-format=json`? I 
think it would result in simpler interface if in the future we decide to add 
another format.



Comment at: clang/lib/Frontend/HeaderIncludeGen.cpp:70
+  bool OwnsOutputFile;
+  std::set IncludedHeaders;
+

Do we really want this to be a set? I can imagine clients wanting the include 
order to be preserved and with potential duplicates (for textual headers 
without guards or `#pragma once`).

If the set semantics are indeed what we want, I'd recommend using 
`llvm::StringSet<>` instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137996

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


[PATCH] D137948: [clang][dataflow] Add widening API and implement it for built-in boolean model.

2022-11-15 Thread Gábor Horváth via Phabricator via cfe-commits
xazax.hun added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:114
   bool isEqualTypeErased(const TypeErasedLattice &E1,
  const TypeErasedLattice &E2) final {
 const Lattice &L1 = llvm::any_cast(E1.Value);

Maybe this is another case where we want to mark the whole class `final` 
instead of the individual methods? It is fine to address this in a separate PR.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:135
+  // between overloads.
+  struct Rank1 {};
+  struct Rank0 : Rank1 {};

Is there ever a reason to instantiate `Rank1`? If no, should we do something 
(like make its ctor private and friends with `Rank0`?). Although possibly not 
important enough to worth the hassle.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:108
 
+  LatticeJoinEffect widenTypeErased(TypeErasedLattice &Current,
+const TypeErasedLattice &Previous) final {

ymandel wrote:
> xazax.hun wrote:
> > I wonder if `LatticeJoinEffect` is the right name if we also use this for 
> > widening. (Also, are we in the process of removing these?)
> I had the same thought. How about just `LatticeEffect`? Open to other 
> suggestions as well. Either way, though, I should update in a separate patch 
> in case that breaks something unexpected.
> 
> As for removing -- yes, we should remove from join, because we never use it. 
> But, it actually makes sense here.
`LatticeEffect` sounds good to me.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:465
+  // FIXME: move the fields `CallStack`, `ReturnLoc` and `ThisPointeeLoc` into 
a
+  // separate call-context object, shared between environment in the same call.
+  // https://github.com/llvm/llvm-project/issues/59005





Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:424-425
+  }
+  if (WidenedEnv.DeclToLoc.size() != PrevEnv.DeclToLoc.size() ||
+  WidenedEnv.ExprToLoc.size() != PrevEnv.ExprToLoc.size() ||
+  WidenedEnv.LocToVal.size() != PrevEnv.LocToVal.size() ||

Isn't some of these tautologically false, as we only read from them?



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:374-378
+  Environment WidenedEnv(*DACtx);
+
+  WidenedEnv.CallStack = CallStack;
+  WidenedEnv.ReturnLoc = ReturnLoc;
+  WidenedEnv.ThisPointeeLoc = ThisPointeeLoc;

ymandel wrote:
> xazax.hun wrote:
> > Shouldn't we have a simple copy ctor from PrefEnv that could populate all 
> > these fields?
> > 
> > Also, this makes me wonder if we actually want to move some of these out 
> > from environment, since these presumably would not change between basic 
> > blocks.
> > Shouldn't we have a simple copy ctor from PrefEnv that could populate all 
> > these fields?
> We *do* have such a copy constructor. The problem is that it's overkill 
> because we want to build LocToVal ourselves by point-wise widening of the 
> elements. For that matter, I wish we could share, rather than copy, 
> `DeclToLoc` and `ExprToLoc` but I don't know of any way to do that. 
> Alternatively: is it possible that we can update in place and don't need a 
> separate `WidenedEnv`?
> 
> > Also, this makes me wonder if we actually want to move some of these out 
> > from environment, since these presumably would not change between basic 
> > blocks.
> Agreed. Added a fixme with an associated issue to track.
> 
> 
> For that matter, I wish we could share, rather than copy, DeclToLoc and 
> ExprToLoc.

One way to maximize sharing (and making copies free) is to use the immutable 
datastructures in LLVM: 
https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/ADT/ImmutableMap.h

Unfortunately, these have their own costs so it might not be easy to decide 
when it is worth to use them. The Clang Static Analyzer was designed from the 
ground up with immutable data structures in mind. 

On the other hand, I see that you want a more ad-hoc/opportunistic sharing. And 
I actually wonder if we need any sharing at all. If we are 100% sure we will do 
the widening, do we actually need to preserve the previous environment? 
Couldn't we "consume" it and move certain fields out instead of doing the more 
expensive copy operation when it makes sense?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137948

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


[PATCH] D138042: [libc++] Trigger both Clang and libc++ CI when there are changes to both

2022-11-15 Thread Louis Dionne via Phabricator via cfe-commits
ldionne created this revision.
ldionne added a reviewer: Mordante.
Herald added a subscriber: arichardson.
Herald added a project: All.
ldionne requested review of this revision.
Herald added projects: clang, libc++.
Herald added subscribers: libcxx-commits, cfe-commits.
Herald added a reviewer: libc++.

Per post-commit comments in https://reviews.llvm.org/D137759.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138042

Files:
  clang/foo
  libcxx/foo
  libcxx/utils/ci/buildkite-pipeline-clang.yml
  libcxx/utils/ci/generate-buildkite-pipeline


Index: libcxx/utils/ci/generate-buildkite-pipeline
===
--- libcxx/utils/ci/generate-buildkite-pipeline
+++ libcxx/utils/ci/generate-buildkite-pipeline
@@ -12,15 +12,9 @@
 #
 
 if git diff --name-only HEAD~1 | grep -q -E 
"^libcxx/|^libcxxabi/|^libunwind/|^runtimes/|^cmake/"; then
-  LIBCXX_CHANGED=true
+  cat libcxx/utils/ci/buildkite-pipeline.yml
 fi
 
 if git diff --name-only HEAD~1 | grep -q -E "^clang/"; then
-  CLANG_CHANGED=true
-fi
-
-if [[ "${CLANG_CHANGED}" == "true" && "${LIBCXX_CHANGED}" != "true" ]]; then
   cat libcxx/utils/ci/buildkite-pipeline-clang.yml
-else
-  cat libcxx/utils/ci/buildkite-pipeline.yml
 fi
Index: libcxx/utils/ci/buildkite-pipeline-clang.yml
===
--- libcxx/utils/ci/buildkite-pipeline-clang.yml
+++ libcxx/utils/ci/buildkite-pipeline-clang.yml
@@ -80,7 +80,7 @@
   limit: 2
 timeout_in_minutes: 120
 
-  - label: "Modules"
+  - label: "Clang Modules"
 commands:
   - "buildkite-agent artifact download 'install/**' ."
   - "export CC=$(pwd)/install/bin/clang"


Index: libcxx/utils/ci/generate-buildkite-pipeline
===
--- libcxx/utils/ci/generate-buildkite-pipeline
+++ libcxx/utils/ci/generate-buildkite-pipeline
@@ -12,15 +12,9 @@
 #
 
 if git diff --name-only HEAD~1 | grep -q -E "^libcxx/|^libcxxabi/|^libunwind/|^runtimes/|^cmake/"; then
-  LIBCXX_CHANGED=true
+  cat libcxx/utils/ci/buildkite-pipeline.yml
 fi
 
 if git diff --name-only HEAD~1 | grep -q -E "^clang/"; then
-  CLANG_CHANGED=true
-fi
-
-if [[ "${CLANG_CHANGED}" == "true" && "${LIBCXX_CHANGED}" != "true" ]]; then
   cat libcxx/utils/ci/buildkite-pipeline-clang.yml
-else
-  cat libcxx/utils/ci/buildkite-pipeline.yml
 fi
Index: libcxx/utils/ci/buildkite-pipeline-clang.yml
===
--- libcxx/utils/ci/buildkite-pipeline-clang.yml
+++ libcxx/utils/ci/buildkite-pipeline-clang.yml
@@ -80,7 +80,7 @@
   limit: 2
 timeout_in_minutes: 120
 
-  - label: "Modules"
+  - label: "Clang Modules"
 commands:
   - "buildkite-agent artifact download 'install/**' ."
   - "export CC=$(pwd)/install/bin/clang"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D133633: [CMake] Add ClangBootstrap configuration

2022-11-15 Thread Amir Ayupov via Phabricator via cfe-commits
Amir updated this revision to Diff 475505.
Amir added a comment.

s/clang_Bootstrap_Add/clang_bootstrap_add


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133633

Files:
  clang/CMakeLists.txt
  clang/cmake/modules/ClangBootstrap.cmake

Index: clang/cmake/modules/ClangBootstrap.cmake
===
--- /dev/null
+++ clang/cmake/modules/ClangBootstrap.cmake
@@ -0,0 +1,41 @@
+include(ExternalProject)
+
+# clang_bootstrap_add(name ...
+#   DEPENDS targets...
+# Targets that this project depends on
+#   CMAKE_ARGS arguments...
+# Optional cmake arguments to pass when configuring the project
+#   BUILD_TOOL_ARGS arguments...
+# Optional arguments to pass to the build tool
+macro(clang_bootstrap_add name)
+  cmake_parse_arguments(ARG "" "LINKER;AR;RANLIB;OBJCOPY;STRIP"
+"DEPENDS;TABLEGEN;CMAKE_ARGS;BUILD_TOOL_ARGS"
+${ARGN})
+  set(STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/${name}-stamps/)
+  set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/${name}-bins/)
+  # Build arguments for native tool used in CMake.
+  set(build_configuration "$")
+
+  ExternalProject_Add(${name}
+DEPENDS ${ARG_DEPENDS}
+PREFIX ${name}
+SOURCE_DIR ${CMAKE_SOURCE_DIR}
+STAMP_DIR ${STAMP_DIR}
+BINARY_DIR ${BINARY_DIR}
+EXCLUDE_FROM_ALL 1
+CMAKE_ARGS
+# We shouldn't need to set this here, but INSTALL_DIR doesn't
+# seem to work, so instead I'm passing this through
+-DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
+${ARG_CMAKE_ARGS}
+BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR}
+   --config ${build_configuration}
+   ${ARG_BUILD_TOOL_ARGS}
+INSTALL_COMMAND ""
+STEP_TARGETS configure build
+USES_TERMINAL_CONFIGURE 1
+USES_TERMINAL_BUILD 1
+USES_TERMINAL_INSTALL 1
+LIST_SEPARATOR |
+  )
+endmacro()
Index: clang/CMakeLists.txt
===
--- clang/CMakeLists.txt
+++ clang/CMakeLists.txt
@@ -785,24 +785,16 @@
   endforeach()
 
   # Build arguments for native tool used in CMake.
-  set(build_configuration "$")
   set(build_tool_args "${LLVM_EXTERNAL_PROJECT_BUILD_TOOL_ARGS}")
   if(NOT build_tool_args STREQUAL "")
 string(PREPEND build_tool_args "-- ")
 separate_arguments(build_tool_args UNIX_COMMAND "${build_tool_args}")
   endif()
 
-  ExternalProject_Add(${NEXT_CLANG_STAGE}
+  include(ClangBootstrap)
+  clang_bootstrap_add(${NEXT_CLANG_STAGE}
 DEPENDS clang-bootstrap-deps
-PREFIX ${NEXT_CLANG_STAGE}
-SOURCE_DIR ${CMAKE_SOURCE_DIR}
-STAMP_DIR ${STAMP_DIR}
-BINARY_DIR ${BINARY_DIR}
-EXCLUDE_FROM_ALL 1
 CMAKE_ARGS
-# We shouldn't need to set this here, but INSTALL_DIR doesn't
-# seem to work, so instead I'm passing this through
--DCMAKE_INSTALL_PREFIX=${CMAKE_INSTALL_PREFIX}
 ${PASSTHROUGH_VARIABLES}
 ${CLANG_BOOTSTRAP_CMAKE_ARGS}
  -DCLANG_STAGE=${NEXT_CLANG_STAGE}
@@ -814,16 +806,8 @@
 ${${CLANG_STAGE}_RANLIB}
 ${${CLANG_STAGE}_OBJCOPY}
 ${${CLANG_STAGE}_STRIP}
-BUILD_COMMAND ${CMAKE_COMMAND} --build ${BINARY_DIR}
-   --config ${build_configuration}
-   ${build_tool_args}
-INSTALL_COMMAND ""
-STEP_TARGETS configure build
-USES_TERMINAL_CONFIGURE 1
-USES_TERMINAL_BUILD 1
-USES_TERMINAL_INSTALL 1
-LIST_SEPARATOR |
-)
+BUILD_TOOL_ARGS ${build_tool_args}
+  )
 
   # exclude really-install from main target
   set_target_properties(${NEXT_CLANG_STAGE} PROPERTIES _EP_really-install_EXCLUDE_FROM_MAIN On)
@@ -904,37 +888,23 @@
   )
 
   # Build specified targets with instrumented Clang to collect the profile
-  set(STAMP_DIR ${CMAKE_CURRENT_BINARY_DIR}/bolt-instrumented-clang-stamps/)
-  set(BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/bolt-instrumented-clang-bins/)
-  set(build_configuration "$")
-  include(ExternalProject)
-  ExternalProject_Add(bolt-instrumentation-profile
+  include(ClangBootstrap)
+  set(COMPILER_OPTIONS
+-DCMAKE_C_COMPILER=${CLANG_INSTRUMENTED}
+-DCMAKE_CXX_COMPILER=${CLANGXX_INSTRUMENTED}
+-DCMAKE_ASM_COMPILER=${CLANG_INSTRUMENTED}
+-DCMAKE_ASM_COMPILER_ID=Clang
+  )
+  clang_bootstrap_add(bolt-instrumentation-profile
 DEPENDS clang++-instrumented
-PREFIX bolt-instrumentation-profile
-SOURCE_DIR ${CMAKE_SOURCE_DIR}
-STAMP_DIR ${STAMP_DIR}
-BINARY_DIR ${BINARY_DIR}
-EXCLUDE_FROM_ALL 1
 CMAKE_ARGS
 ${CLANG_BOLT_INSTRUMENT_EXTRA_CMAKE_FLAGS}
-# We shouldn't need to set this here, but INSTALL_DIR doesn't
-# seem to work, so instead I'm passing this through
--DCMAKE_INSTA

[PATCH] D133633: [CMake] Add ClangBootstrap configuration

2022-11-15 Thread Amir Ayupov via Phabricator via cfe-commits
Amir marked an inline comment as done.
Amir added inline comments.



Comment at: clang/cmake/modules/ClangBootstrap.cmake:10
+# Optional arguments to pass to the build tool
+macro(clang_Bootstrap_Add name)
+  cmake_parse_arguments(ARG "" "LINKER;AR;RANLIB;OBJCOPY;STRIP"

phosek wrote:
> We usually use lowercase names.
Thanks! I was a bit confused by CMake's case use (e.g. `ExternalProject_Add`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133633

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


[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

2022-11-15 Thread Sander de Smalen via Phabricator via cfe-commits
sdesmalen added inline comments.



Comment at: clang/include/clang/AST/Type.h:3940
+/// on declarations and function pointers.
+unsigned AArch64SMEAttributes : 8;
+

erichkeane wrote:
> We seem to be missing all of the modules-storage code for these.  Since this 
> is modifying the AST, we need to increment the 'breaking change' AST code, 
> plus add this to the ASTWriter/ASTReader interface.
> Since this is modifying the AST, we need to increment the 'breaking change' 
> AST code
Could you give me some pointers on what you expect to see changed here? I 
understand your point about adding this to the ASTWriter/Reader interfaces for 
module-storage, but it's not entirely clear what you mean by "increment the 
breaking change AST code". 

I see there is also an ASTImporter, I guess this different from the ASTReader?

Please apologise my ignorance here, I'm not as familiar with the Clang codebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127762

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


[PATCH] D137058: [Driver] [Modules] Support -fsave-std-c++-module-file (1/2)

2022-11-15 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:5541-5542
+  //
+  // FIXME: Do we need to handle the case that the calculated output is
+  // conflicting with the specified output file or the input file?
+  if (!AtTopLevel && isa(JA) &&

Do we do that for `-o` today? (eg: if you try to `-o` and specify the input 
file name, such that the output would overwrite the input, what happens? I'm 
not sure - but I guess it's OK to do whatever that is for this case too)



Comment at: clang/test/Driver/save-std-c++-module-file.cpp:1-11
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang -std=c++20 %t/Hello.cppm -fsave-std-c++-module-file -c
+// RUN: ls %t | FileCheck %t/Hello.cppm --check-prefix=CHECK-DEFAULT
+//

Usually driver tests only invoke the driver and observe the output using `-###` 
to see what subcommands the driver would run. Could this test be phrased in 
that way? (testing whatever command line is expected to be invoked, rather than 
the actual behavior of the frontend)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137058

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


[PATCH] D127762: [Clang][AArch64] Add ACLE attributes for SME.

2022-11-15 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment.

I also don't see anything in this with how it works with function pointer 
conversions, how it affects template instantiation, overload resolution/etc.  
If this is a type attribute, we need to specify all of that, probably in a 
separate document, since this is such a change to all those things.




Comment at: clang/include/clang/AST/Type.h:3940
+/// on declarations and function pointers.
+unsigned AArch64SMEAttributes : 8;
+

sdesmalen wrote:
> erichkeane wrote:
> > We seem to be missing all of the modules-storage code for these.  Since 
> > this is modifying the AST, we need to increment the 'breaking change' AST 
> > code, plus add this to the ASTWriter/ASTReader interface.
> > Since this is modifying the AST, we need to increment the 'breaking change' 
> > AST code
> Could you give me some pointers on what you expect to see changed here? I 
> understand your point about adding this to the ASTWriter/Reader interfaces 
> for module-storage, but it's not entirely clear what you mean by "increment 
> the breaking change AST code". 
> 
> I see there is also an ASTImporter, I guess this different from the ASTReader?
> 
> Please apologise my ignorance here, I'm not as familiar with the Clang 
> codebase.
See VersionMajor here: https://clang.llvm.org/doxygen/ASTBitCodes_8h_source.html

Yes, ASTReader/ASTWriter and ASTImporter are different unfortunately.  I'm not 
completely sure of the difference, but doing this patch changing the type here 
without doing those will break modules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127762

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


[PATCH] D137979: parse: process GNU and standard attributes on top-level decls

2022-11-15 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits
ymandel added inline comments.



Comment at: clang/unittests/Tooling/SourceCodeTest.cpp:249-258
   Visitor.runOverAnnotated(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
   int x;]])cpp");
 
   // Includes attributes and comments together.
   Visitor.runOverAnnotated(R"cpp(
+  $r[[__attribute__((deprecated("message")))

Thanks for trying to fix these! The changed test cases seem to be doing two 
things at once: macros and attributes, and I don't remember why. We should test 
the behavior separately.  So, I think you're new test cases are good 
regardless.  But, we still want the old tests to test the behavior for 
attributes that are hidden behind a macro, since this is used not infrequently 
in my exprience. 

IIUC, the current code isn't properly handling attributes that appear inside 
macros, which is why this fix breaks the code. Specifically, it is not 
considering the case that the location in `Attr->getLocation()` is in a macro, 
while `Range.getBegin()` is in the original file, and hence the locations are 
not comparable. I'm surprised this ever worked, tbh.

My preference would be to update the code, if you know how, so that both the 
old and new tests pass. But, I realize this may be a lot to ask. So, at the 
least, please comment out, but keep, the old tests; or, copy the old tests to a 
new test and mark it disabled. Either way, prefix with a FIXME indicating the 
issue.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137979

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


[PATCH] D137979: parse: process GNU and standard attributes on top-level decls

2022-11-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: erichkeane, elizabethandrews.
aaron.ballman added a comment.

Adding some additional reviewers for more awareness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137979

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


[PATCH] D138031: Add clang-tidy check for missing move constructors

2022-11-15 Thread Nathan James via Phabricator via cfe-commits
njames93 added a comment.

What is the purpose of this check when we have 
cppcoreguidelines-special-member-functions 
.
 
Granted that check won't emit replacements, however the replacements that this 
check generates would often not improve performance or potentially introduce a 
bug.

I know its not nearly the same, but clangd can also fix this interactively with 
its special members 

 refactor


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138031

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


[PATCH] D137979: parse: process GNU and standard attributes on top-level decls

2022-11-15 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd added inline comments.



Comment at: clang/unittests/Tooling/SourceCodeTest.cpp:249-258
   Visitor.runOverAnnotated(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
   int x;]])cpp");
 
   // Includes attributes and comments together.
   Visitor.runOverAnnotated(R"cpp(
+  $r[[__attribute__((deprecated("message")))

ymandel wrote:
> Thanks for trying to fix these! The changed test cases seem to be doing two 
> things at once: macros and attributes, and I don't remember why. We should 
> test the behavior separately.  So, I think you're new test cases are good 
> regardless.  But, we still want the old tests to test the behavior for 
> attributes that are hidden behind a macro, since this is used not 
> infrequently in my exprience. 
> 
> IIUC, the current code isn't properly handling attributes that appear inside 
> macros, which is why this fix breaks the code. Specifically, it is not 
> considering the case that the location in `Attr->getLocation()` is in a 
> macro, while `Range.getBegin()` is in the original file, and hence the 
> locations are not comparable. I'm surprised this ever worked, tbh.
> 
> My preference would be to update the code, if you know how, so that both the 
> old and new tests pass. But, I realize this may be a lot to ask. So, at the 
> least, please comment out, but keep, the old tests; or, copy the old tests to 
> a new test and mark it disabled. Either way, prefix with a FIXME indicating 
> the issue.
> 
> WDYT?
Unfortunately, I couldn't figure out how to track through the expansion so I am 
going to take you up on the commented out case option.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137979

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


[PATCH] D136354: [Driver] Enable nested configuration files

2022-11-15 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay accepted this revision.
MaskRay added a comment.

> To solve such problems, the option --config is allowed inside ...

`--config=`




Comment at: clang/docs/UsersManual.rst:989
+``linux.opts`` is searched for in the directory ``~/.llvm/os``. Another way to
+include a file content is using the command line option ``--config``. It works
+similarly but the included file is searched for using the rules for 
configuration

`--config=`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136354

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


[PATCH] D137473: [vfs] Allow root paths relative to the directory of the vfsoverlay YAML file

2022-11-15 Thread Ben Barham via Phabricator via cfe-commits
bnbarham added inline comments.



Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:1010
 
+  std::error_code makeAbsolute(StringRef WorkingDir,
+   SmallVectorImpl &Path) const;

Should be private IMO



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1369
   // append Path ourselves.
+  if (!sys::path::is_absolute(WorkingDir, sys::path::Style::posix) &&
+  !sys::path::is_absolute(WorkingDir,

Did you find this was needed? It's already checked by the normal `makeAbsolute` 
(which checks this already) and the only other caller is when we're using the 
overlay directory path, which should always be absolute.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1913
+ RedirectingFileSystem::RootRelativeKind::OverlayDir) {
+SmallString<256> FullPath = FS->getOverlayFileDir();
+assert(!FullPath.empty() && "Overlay file directory must exist");

This is unused now. Maybe we should just merge the `else if` and `else` 
branches and just grab either `getOverlayFileDir` or 
`getCurrentWorkingDirectory` depending on `RootRelative`. They should otherwise 
be identical.



Comment at: llvm/unittests/Support/VirtualFileSystemTest.cpp:1875
+
+  IntrusiveRefCntPtr FSOverlayRelative =
+  getFromYAMLString("{\n"

Just override the previous `FS`/`S` IMO - that way we avoid the accidental 
`FS->status` a few lines down 😅


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137473

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


[PATCH] D137835: [ARM] Move ARM::parseBranchProtection into ARMTargetParser

2022-11-15 Thread Sam Elliott via Phabricator via cfe-commits
lenary added a comment.

I'm going to move this into the newly created `ARMTargetParserCommon` files 
which @tmatheson created in https://reviews.llvm.org/D137924 (which has landed).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137835

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


[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

2022-11-15 Thread Tom Praschan via Phabricator via cfe-commits
tom-anders added a comment.

LGTM, but I'll give @kadircet a chance to review as well


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137817

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


[PATCH] D135919: [Clang] Correct when Itanium ABI guard variables are set for non-block variables with static or thread storage duration.

2022-11-15 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment.

@rjmccall, could you please take a look at the updated changes? I think I've 
addressed the issues you raised.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135919

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


[PATCH] D138047: Fix use of dangling stack allocated string in IncludeFixer

2022-11-15 Thread David Goldman via Phabricator via cfe-commits
dgoldman created this revision.
dgoldman added a reviewer: kadircet.
Herald added a subscriber: arphaman.
Herald added a project: All.
dgoldman requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

IncludeFixer uses this BuildDir string later on if given relative paths.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D138047

Files:
  clang-tools-extra/clangd/Headers.h


Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -250,7 +250,7 @@
 private:
   StringRef FileName;
   StringRef Code;
-  StringRef BuildDir;
+  std::string BuildDir;
   HeaderSearch *HeaderSearchInfo = nullptr;
   llvm::StringSet<> IncludedHeaders; // Both written and resolved.
   tooling::HeaderIncludes Inserter;  // Computers insertion replacement.


Index: clang-tools-extra/clangd/Headers.h
===
--- clang-tools-extra/clangd/Headers.h
+++ clang-tools-extra/clangd/Headers.h
@@ -250,7 +250,7 @@
 private:
   StringRef FileName;
   StringRef Code;
-  StringRef BuildDir;
+  std::string BuildDir;
   HeaderSearch *HeaderSearchInfo = nullptr;
   llvm::StringSet<> IncludedHeaders; // Both written and resolved.
   tooling::HeaderIncludes Inserter;  // Computers insertion replacement.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D138047: Fix use of dangling stack allocated string in IncludeFixer

2022-11-15 Thread David Goldman via Phabricator via cfe-commits
dgoldman added a comment.

LMK of the best way to add a test for this, maybe I can somehow make a test 
with relative path arguments?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138047

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


[PATCH] D137760: Add FP8 E4M3 support to APFloat.

2022-11-15 Thread Benjamin Kramer via Phabricator via cfe-commits
bkramer accepted this revision.
bkramer 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/D137760/new/

https://reviews.llvm.org/D137760

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


[PATCH] D137897: Extend the number of case Sema::CheckForIntOverflow covers

2022-11-15 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik updated this revision to Diff 475528.
shafik added a comment.

Add Release note.


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

https://reviews.llvm.org/D137897

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/integer-overflow.cpp


Index: clang/test/Sema/integer-overflow.cpp
===
--- /dev/null
+++ clang/test/Sema/integer-overflow.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -Wno-unused-value -verify -fsyntax-only
+
+namespace GH58944 {
+struct A {
+  A(unsigned long) ;
+};
+
+A a(1024 * 1024 * 1024 * 1024 * 1024ull); // expected-warning {{overflow in 
expression; result is 0 with type 'int'}}
+
+void f() {
+  new int[1024 * 1024 * 1024 * 1024 * 1024ull]; // expected-warning {{overflow 
in expression; result is 0 with type 'int'}}
+
+  int arr[]{1,2,3};
+  arr[1024 * 1024 * 1024 * 1024 * 1024ull]; // expected-warning {{overflow in 
expression; result is 0 with type 'int'}}
+
+  (int){1024 * 1024 * 1024 * 1024 * 1024}; // expected-warning {{overflow in 
expression; result is 0 with type 'int'}}
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -14660,6 +14660,17 @@
   Exprs.append(Call->arg_begin(), Call->arg_end());
 else if (auto Message = dyn_cast(E))
   Exprs.append(Message->arg_begin(), Message->arg_end());
+else if (auto Construct = dyn_cast(E))
+  Exprs.append(Construct->arg_begin(), Construct->arg_end());
+else if (auto Array = dyn_cast(E))
+  Exprs.push_back(Array->getIdx());
+else if (auto Compound = dyn_cast(E))
+  Exprs.push_back(Compound->getInitializer());
+else if (auto New = dyn_cast(E)) {
+  if (New->isArray())
+if (auto ArraySize = New->getArraySize())
+  Exprs.push_back(ArraySize.value());
+}
   } while (!Exprs.empty());
 }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -388,6 +388,8 @@
 - Clang now diagnoses use of invalid or reserved module names in a module
   export declaration. Both are diagnosed as an error, but the diagnostic is
   suppressed for use of reserved names in a system header.
+- ``-Winteger-overflow`` will diagnose overflow in more cases. This fixes 
+  `Issue 58944 `_.
 
 Non-comprehensive list of changes in this release
 -


Index: clang/test/Sema/integer-overflow.cpp
===
--- /dev/null
+++ clang/test/Sema/integer-overflow.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 %s -Wno-unused-value -verify -fsyntax-only
+
+namespace GH58944 {
+struct A {
+  A(unsigned long) ;
+};
+
+A a(1024 * 1024 * 1024 * 1024 * 1024ull); // expected-warning {{overflow in expression; result is 0 with type 'int'}}
+
+void f() {
+  new int[1024 * 1024 * 1024 * 1024 * 1024ull]; // expected-warning {{overflow in expression; result is 0 with type 'int'}}
+
+  int arr[]{1,2,3};
+  arr[1024 * 1024 * 1024 * 1024 * 1024ull]; // expected-warning {{overflow in expression; result is 0 with type 'int'}}
+
+  (int){1024 * 1024 * 1024 * 1024 * 1024}; // expected-warning {{overflow in expression; result is 0 with type 'int'}}
+}
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -14660,6 +14660,17 @@
   Exprs.append(Call->arg_begin(), Call->arg_end());
 else if (auto Message = dyn_cast(E))
   Exprs.append(Message->arg_begin(), Message->arg_end());
+else if (auto Construct = dyn_cast(E))
+  Exprs.append(Construct->arg_begin(), Construct->arg_end());
+else if (auto Array = dyn_cast(E))
+  Exprs.push_back(Array->getIdx());
+else if (auto Compound = dyn_cast(E))
+  Exprs.push_back(Compound->getInitializer());
+else if (auto New = dyn_cast(E)) {
+  if (New->isArray())
+if (auto ArraySize = New->getArraySize())
+  Exprs.push_back(ArraySize.value());
+}
   } while (!Exprs.empty());
 }
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -388,6 +388,8 @@
 - Clang now diagnoses use of invalid or reserved module names in a module
   export declaration. Both are diagnosed as an error, but the diagnostic is
   suppressed for use of reserved names in a system header.
+- ``-Winteger-overflow`` will diagnose overflow in more cases. This fixes 
+  `Issue 58944 `_.
 
 Non-comprehensive list of changes in this release
 --

[clang] ea5be25 - [AVR] Fix use-of-uninitialized-value after D137520

2022-11-15 Thread Fangrui Song via cfe-commits

Author: Fangrui Song
Date: 2022-11-15T11:06:18-08:00
New Revision: ea5be2571dc95edb8ee04d966addd6b58bfe7b85

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

LOG: [AVR] Fix use-of-uninitialized-value after D137520

Added: 


Modified: 
clang/lib/Basic/Targets/AVR.h

Removed: 




diff  --git a/clang/lib/Basic/Targets/AVR.h b/clang/lib/Basic/Targets/AVR.h
index 2e2b13e0ac653..272afdfc03884 100644
--- a/clang/lib/Basic/Targets/AVR.h
+++ b/clang/lib/Basic/Targets/AVR.h
@@ -176,7 +176,7 @@ class LLVM_LIBRARY_VISIBILITY AVRTargetInfo : public 
TargetInfo {
   StringRef ABI;
   StringRef DefineName;
   StringRef Arch;
-  int NumFlashBanks;
+  int NumFlashBanks = 0;
 };
 
 } // namespace targets



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


[PATCH] D137979: parse: process GNU and standard attributes on top-level decls

2022-11-15 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd updated this revision to Diff 475530.
compnerd added a comment.

Add unsupported test cases


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

https://reviews.llvm.org/D137979

Files:
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseHLSL.cpp
  clang/lib/Parse/ParseObjc.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant-varying-return.c
  clang/test/AST/ast-dump-openmp-begin-declare-variant_10.c
  clang/test/AST/ast-dump-openmp-begin-declare-variant_11.c
  clang/test/AST/ast-dump-openmp-begin-declare-variant_12.c
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objc.plist
  clang/test/Analysis/Inputs/expected-plists/retain-release.m.objcpp.plist
  clang/test/Parser/attr-order.cpp
  clang/test/Parser/cxx-attributes.cpp
  clang/test/SemaCXX/attr-unavailable.cpp
  clang/test/SemaObjC/objc-asm-attribute-neg-test.m
  clang/unittests/Tooling/SourceCodeTest.cpp

Index: clang/unittests/Tooling/SourceCodeTest.cpp
===
--- clang/unittests/Tooling/SourceCodeTest.cpp
+++ clang/unittests/Tooling/SourceCodeTest.cpp
@@ -247,16 +247,29 @@
 
   // Includes attributes.
   Visitor.runOverAnnotated(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
   int x;]])cpp");
 
   // Includes attributes and comments together.
   Visitor.runOverAnnotated(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
   // Comment.
   int x;]])cpp");
+
+#if TOOLING_MACRO_EXPANSION_UNSUPPORTED
+  // Includes attributes through macro expansion.
+  Visitor.runOverAnnotate(R"cpp(
+  #define MACRO_EXPANSION __attribute__((deprecated("message")))
+  $r[[MACRO_EXPANSION
+  int x;]])cpp");
+
+  // Includes attributes through macro expansion with comments.
+  Visitor.runOverAnnotate(R"cpp(
+  #define MACRO_EXPANSION __attribute__((deprecated("message")))
+  $r[[MACRO_EXPANSION
+  // Comment.
+  int x;]])cpp");
+#endif
 }
 
 TEST(SourceCodeTest, getAssociatedRangeClasses) {
@@ -402,16 +415,29 @@
 
   // Includes attributes.
   Visit(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
   int x;]])cpp");
 
   // Includes attributes and comments together.
   Visit(R"cpp(
-  #define ATTR __attribute__((deprecated("message")))
-  $r[[ATTR
+  $r[[__attribute__((deprecated("message")))
+  // Comment.
+  int x;]])cpp");
+
+#if TOOLING_MACRO_EXPANSION_UNSUPPORTED
+  // Includes attributes through macro expansion.
+  Visitor.runOverAnnotate(R"cpp(
+  #define MACRO_EXPANSION __attribute__((deprecated("message")))
+  $r[[MACRO_EXPANSION
+  int x;]])cpp");
+
+  // Includes attributes through macro expansion with comments.
+  Visitor.runOverAnnotate(R"cpp(
+  #define MACRO_EXPANSION __attribute__((deprecated("message")))
+  $r[[MACRO_EXPANSION
   // Comment.
   int x;]])cpp");
+#endif
 }
 
 TEST(SourceCodeTest, getAssociatedRangeInvalidForPartialExpansions) {
Index: clang/test/SemaObjC/objc-asm-attribute-neg-test.m
===
--- clang/test/SemaObjC/objc-asm-attribute-neg-test.m
+++ clang/test/SemaObjC/objc-asm-attribute-neg-test.m
@@ -28,7 +28,7 @@
 @end
 
 __attribute__((objc_runtime_name("MySecretNamespace.ForwardClass")))
-@class ForwardClass; // expected-error {{prefix attribute must be followed by an interface, protocol, or implementation}}
+@class ForwardClass; // expected-error@-1 {{prefix attribute must be followed by an interface, protocol, or implementation}}
 
 __attribute__((objc_runtime_name("MySecretNamespace.ForwardProtocol")))
 @protocol ForwardProtocol;
Index: clang/test/SemaCXX/attr-unavailable.cpp
===
--- clang/test/SemaCXX/attr-unavailable.cpp
+++ clang/test/SemaCXX/attr-unavailable.cpp
@@ -42,18 +42,18 @@
 // delayed process for 'deprecated'.
 //  and 
 enum DeprecatedEnum { DE_A, DE_B } __attribute__((deprecated)); // expected-note {{'DeprecatedEnum' has been explicitly marked deprecated here}}
-__attribute__((deprecated)) typedef enum DeprecatedEnum DeprecatedEnum;
 typedef enum DeprecatedEnum AnotherDeprecatedEnum; // expected-warning {{'DeprecatedEnum' is deprecated}}
 
+__attribute__((deprecated)) typedef enum DeprecatedEnum DeprecatedEnum;
 __attribute__((deprecated))
 DeprecatedEnum testDeprecated(DeprecatedEnum X) { return X; }
 
 
 enum UnavailableEnum { UE_A, UE_B } __attribute__((unavailable)); // expected-note {{'UnavailableEnum' has been explicitly marked unavailable here}}
-__attribute__((unavailable)) typedef enum UnavailableEnum UnavailableEnum;
 typedef enum UnavailableEnu

[PATCH] D137992: [asan] -fsanitize-address-outline-instrumentation -> -asan-max-inline-poisoning-size=0

2022-11-15 Thread Vitaly Buka via Phabricator via cfe-commits
vitalybuka added a subscriber: kstoimenov.
vitalybuka added inline comments.



Comment at: clang/lib/Driver/SanitizerArgs.cpp:1256
 
   if (AsanOutlineInstrumentation) {
 CmdArgs.push_back("-mllvm");

CC @kstoimenov 

it's maybe not the best name for flag, but the point of 
AsanOutlineInstrumentation was to reduce binary size.

for -asan-instrumentation-with-call-threshold=0 sizeof(check+asan_report) > 
sizeof(outlined_check_report), so it helps with size
for -asan-max-inline-poisoning-size=0: usually sizeof(small store/load) < 
sizeof(poisoning_callbac). It usually increases the size.

Out internal setup expects the current size saving behavior of this flag.

So to move forward we can do the following instead:
1. Create a new flag for -asan-max-inline-poisoning-size=0, e.g. 
AsanOutlinePoisoning
1. Create a clone flag for -asan-instrumentation-with-call-threshold=0, e.g. 
AsanOutlineChecks
1. We switch out build system to the clone
1. Extend (or delete) AsanOutlineInstrumentation as in this patch






Comment at: clang/lib/Driver/SanitizerArgs.cpp:1260
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-asan-max-inline-poisoning-size=0");
   }

For changes like this we need to update 
llvm-project/clang/test/Driver/fsanitize.c


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137992

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


  1   2   3   >