[PATCH] D126780: [clang-tidy] `bugprone-use-after-move`: Fix handling of moves in lambda captures

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 433972.
mboehme added a comment.

Added release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126780

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -416,6 +416,13 @@
 auto lambda = [&]() { a.foo(); };
 std::move(a);
   }
+  {
+A a;
+auto lambda = [a = std::move(a)] { a.foo(); };
+a.foo();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:24: note: move occurred here
+  }
 }
 
 // Use-after-moves are detected in uninstantiated templates if the moved type
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -209,6 +209,11 @@
   ` when the 
specialization
   template has an unnecessary value paramter. Removed the fix for a template.
 
+- Fixed a bug in :doc:`bugprone-use-after-move
+   where a move in a lambda capture
+  was treated as if it happened within the body of the lambda, not within the
+  function that defines the lambda.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -400,7 +400,8 @@
   auto CallMoveMatcher =
   callExpr(callee(functionDecl(hasName("::std::move"))), 
argumentCountIs(1),
hasArgument(0, declRefExpr().bind("arg")),
-   anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
+   anyOf(hasAncestor(compoundStmt(
+ hasParent(lambdaExpr().bind("containing-lambda",
  hasAncestor(functionDecl().bind("containing-func"))),
unless(inDecltypeOrTemplateArg()),
// try_emplace is a common maybe-moving function that returns a


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -416,6 +416,13 @@
 auto lambda = [&]() { a.foo(); };
 std::move(a);
   }
+  {
+A a;
+auto lambda = [a = std::move(a)] { a.foo(); };
+a.foo();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:24: note: move occurred here
+  }
 }
 
 // Use-after-moves are detected in uninstantiated templates if the moved type
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -209,6 +209,11 @@
   ` when the specialization
   template has an unnecessary value paramter. Removed the fix for a template.
 
+- Fixed a bug in :doc:`bugprone-use-after-move
+   where a move in a lambda capture
+  was treated as if it happened within the body of the lambda, not within the
+  function that defines the lambda.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -400,7 +400,8 @@
   auto CallMoveMatcher =
   callExpr(callee(functionDecl(hasName("::std::move"))), argumentCountIs(1),
hasArgument(0, declRefExpr().bind("arg")),
-   anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
+   anyOf(hasAncestor(compoundStmt(
+ hasParent(lambdaExpr().bind("containing-lambda",
  hasAncestor(functionDecl().bind("containing-func"))),
unless(inDecltypeOrTemplateArg()),
// try_emplace is a common maybe-moving function that returns a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 0d21863 - [Driver] Add multiarch path for RISC-V

[Jonas Hahnfeld via cfe-commits]

Author: Jonas Hahnfeld
Date: 2022-06-03T09:10:34+02:00
New Revision: 0d2186373f73995cfcc45f445024fbc9841c99d6

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

LOG: [Driver] Add multiarch path for RISC-V

This is required to find headers on the Debian port for RISC-V.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Linux.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Linux.cpp 
b/clang/lib/Driver/ToolChains/Linux.cpp
index 43b2bf06e8b4..4309b1060346 100644
--- a/clang/lib/Driver/ToolChains/Linux.cpp
+++ b/clang/lib/Driver/ToolChains/Linux.cpp
@@ -124,6 +124,8 @@ std::string Linux::getMultiarchTriple(const Driver &D,
 return "powerpc64-linux-gnu";
   case llvm::Triple::ppc64le:
 return "powerpc64le-linux-gnu";
+  case llvm::Triple::riscv64:
+return "riscv64-linux-gnu";
   case llvm::Triple::sparc:
 return "sparc-linux-gnu";
   case llvm::Triple::sparcv9:



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

[PATCH] D126672: [Driver] Add multiarch path for RISC-V

[Jonas Hahnfeld via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0d2186373f73: [Driver] Add multiarch path for RISC-V 
(authored by Hahnfeld).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126672

Files:
  clang/lib/Driver/ToolChains/Linux.cpp


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -124,6 +124,8 @@
 return "powerpc64-linux-gnu";
   case llvm::Triple::ppc64le:
 return "powerpc64le-linux-gnu";
+  case llvm::Triple::riscv64:
+return "riscv64-linux-gnu";
   case llvm::Triple::sparc:
 return "sparc-linux-gnu";
   case llvm::Triple::sparcv9:


Index: clang/lib/Driver/ToolChains/Linux.cpp
===
--- clang/lib/Driver/ToolChains/Linux.cpp
+++ clang/lib/Driver/ToolChains/Linux.cpp
@@ -124,6 +124,8 @@
 return "powerpc64-linux-gnu";
   case llvm::Triple::ppc64le:
 return "powerpc64le-linux-gnu";
+  case llvm::Triple::riscv64:
+return "riscv64-linux-gnu";
   case llvm::Triple::sparc:
 return "sparc-linux-gnu";
   case llvm::Triple::sparcv9:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126780: [clang-tidy] `bugprone-use-after-move`: Fix handling of moves in lambda captures

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

In D126780#3552999 , @njames93 wrote:

> LGTM, But please add a note to ReleaseNotes before landing.

Thanks for reminding me! Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126780

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

[PATCH] D126781: [CodeGen] Correctly handle weak symbols in the codegen

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 433976.
junaire added a comment.

Update commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126781

Files:
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ModuleBuilder.cpp
  clang/test/Interpreter/execute.cpp


Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -13,4 +13,8 @@
 
 auto r2 = printf("S[f=%f, m=0x%llx]\n", s.f, reinterpret_cast(s.m));
 // CHECK-NEXT: S[f=1.00, m=0x0]
+
+inline int foo() { return 42; }
+int r3 = foo();
+
 quit
Index: clang/lib/CodeGen/ModuleBuilder.cpp
===
--- clang/lib/CodeGen/ModuleBuilder.cpp
+++ clang/lib/CodeGen/ModuleBuilder.cpp
@@ -133,8 +133,37 @@
 llvm::Module *StartModule(llvm::StringRef ModuleName,
   llvm::LLVMContext &C) {
   assert(!M && "Replacing existing Module?");
+
+  std::unique_ptr OldBuilder;
+  OldBuilder.swap(Builder);
+
   M.reset(new llvm::Module(ExpandModuleName(ModuleName, CodeGenOpts), C));
   Initialize(*Ctx);
+
+  assert(OldBuilder->getDeferredDeclsToEmit().empty() &&
+ "Should have emitted all decls deferred to emit.");
+  assert(Builder->getDeferredDecls().empty() &&
+ "Newly created module should not have deferred decls");
+
+  Builder->getDeferredDecls().swap(OldBuilder->getDeferredDecls());
+
+  assert(Builder->getDeferredVTables().empty() &&
+ "Newly created module should not have deferred vtables");
+
+  Builder->getDeferredVTables().swap(OldBuilder->getDeferredVTables());
+
+  assert(Builder->getMangledDeclNames().empty() &&
+ "Newly created module should not have mangled decl names");
+  assert(Builder->getManglings().empty() &&
+ "Newly created module should not have manglings");
+
+  Builder->getManglings() = std::move(OldBuilder->getManglings());
+
+  assert(OldBuilder->getWeakRefReferences().empty() &&
+ "Not all WeakRefRefs have been applied");
+
+  Builder->getTBAA().swap(OldBuilder->getTBAA());
+
   return M.get();
 }
 
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -1477,6 +1477,36 @@
   void printPostfixForExternalizedDecl(llvm::raw_ostream &OS,
const Decl *D) const;
 
+  llvm::SmallPtrSetImpl &getEmittedDeferredDecls() {
+return EmittedModuleInitializers;
+  }
+
+  llvm::DenseMap &getDeferredDecls() {
+return DeferredDecls;
+  }
+
+  std::vector &getDeferredDeclsToEmit() {
+return DeferredDeclsToEmit;
+  }
+
+  std::vector &getDeferredVTables() {
+return DeferredVTables;
+  }
+
+  llvm::MapVector &getMangledDeclNames() {
+return MangledDeclNames;
+  }
+
+  llvm::StringMap &getManglings() {
+return Manglings;
+  }
+
+  llvm::SmallPtrSetImpl &getWeakRefReferences() {
+return WeakRefReferences;
+  }
+
+  std::unique_ptr &getTBAA() { return TBAA; }
+
 private:
   llvm::Constant *GetOrCreateLLVMFunction(
   StringRef MangledName, llvm::Type *Ty, GlobalDecl D, bool ForVTable,


Index: clang/test/Interpreter/execute.cpp
===
--- clang/test/Interpreter/execute.cpp
+++ clang/test/Interpreter/execute.cpp
@@ -13,4 +13,8 @@
 
 auto r2 = printf("S[f=%f, m=0x%llx]\n", s.f, reinterpret_cast(s.m));
 // CHECK-NEXT: S[f=1.00, m=0x0]
+
+inline int foo() { return 42; }
+int r3 = foo();
+
 quit
Index: clang/lib/CodeGen/ModuleBuilder.cpp
===
--- clang/lib/CodeGen/ModuleBuilder.cpp
+++ clang/lib/CodeGen/ModuleBuilder.cpp
@@ -133,8 +133,37 @@
 llvm::Module *StartModule(llvm::StringRef ModuleName,
   llvm::LLVMContext &C) {
   assert(!M && "Replacing existing Module?");
+
+  std::unique_ptr OldBuilder;
+  OldBuilder.swap(Builder);
+
   M.reset(new llvm::Module(ExpandModuleName(ModuleName, CodeGenOpts), C));
   Initialize(*Ctx);
+
+  assert(OldBuilder->getDeferredDeclsToEmit().empty() &&
+ "Should have emitted all decls deferred to emit.");
+  assert(Builder->getDeferredDecls().empty() &&
+ "Newly created module should not have deferred decls");
+
+  Builder->getDeferredDecls().swap(OldBuilder->getDeferredDecls());
+
+  assert(Builder->getDeferredVTables().empty() &&
+ "Newly created module should not have deferred vtables");
+
+  Builder->getDeferredVTables().swap(OldBuilder->getDeferredVTables());
+
+  assert(Builder->getMangledDeclNames().empty() &&
+ "Newly created module shoul

[clang-tools-extra] 8b90b25 - [clang-tidy] `bugprone-use-after-move`: Fix handling of moves in lambda captures

[Martin Boehme via cfe-commits]

Author: Martin Boehme
Date: 2022-06-03T09:34:09+02:00
New Revision: 8b90b2539048a581052a4b0d7628ffba0cd582a9

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

LOG: [clang-tidy] `bugprone-use-after-move`: Fix handling of moves in lambda 
captures

Previously, we were treating a move in the lambda capture as if it happened
within the body of the lambda, not within the function that defines the lambda.

This fixes the same bug as https://reviews.llvm.org/D119165 (which it appears
may have been abandoned by the author?) but does so more simply.

Reviewed By: njames93

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
index 064b6ae19784e..55f7b87f48a3b 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -400,7 +400,8 @@ void UseAfterMoveCheck::registerMatchers(MatchFinder 
*Finder) {
   auto CallMoveMatcher =
   callExpr(callee(functionDecl(hasName("::std::move"))), 
argumentCountIs(1),
hasArgument(0, declRefExpr().bind("arg")),
-   anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
+   anyOf(hasAncestor(compoundStmt(
+ hasParent(lambdaExpr().bind("containing-lambda",
  hasAncestor(functionDecl().bind("containing-func"))),
unless(inDecltypeOrTemplateArg()),
// try_emplace is a common maybe-moving function that returns a

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index c709f6d35e155..a3e616357398c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -209,6 +209,11 @@ Changes in existing checks
   ` when the 
specialization
   template has an unnecessary value paramter. Removed the fix for a template.
 
+- Fixed a bug in :doc:`bugprone-use-after-move
+   where a move in a lambda capture
+  was treated as if it happened within the body of the lambda, not within the
+  function that defines the lambda.
+
 Removed checks
 ^^
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
index e26db0f6793e0..d6ce626e99d02 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -416,6 +416,13 @@ void lambdas() {
 auto lambda = [&]() { a.foo(); };
 std::move(a);
   }
+  {
+A a;
+auto lambda = [a = std::move(a)] { a.foo(); };
+a.foo();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:24: note: move occurred here
+  }
 }
 
 // Use-after-moves are detected in uninstantiated templates if the moved type



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

[PATCH] D126780: [clang-tidy] `bugprone-use-after-move`: Fix handling of moves in lambda captures

[Martin Böhme via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8b90b2539048: [clang-tidy] `bugprone-use-after-move`: Fix 
handling of moves in lambda captures (authored by mboehme).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126780

Files:
  clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -416,6 +416,13 @@
 auto lambda = [&]() { a.foo(); };
 std::move(a);
   }
+  {
+A a;
+auto lambda = [a = std::move(a)] { a.foo(); };
+a.foo();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:24: note: move occurred here
+  }
 }
 
 // Use-after-moves are detected in uninstantiated templates if the moved type
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -209,6 +209,11 @@
   ` when the 
specialization
   template has an unnecessary value paramter. Removed the fix for a template.
 
+- Fixed a bug in :doc:`bugprone-use-after-move
+   where a move in a lambda capture
+  was treated as if it happened within the body of the lambda, not within the
+  function that defines the lambda.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -400,7 +400,8 @@
   auto CallMoveMatcher =
   callExpr(callee(functionDecl(hasName("::std::move"))), 
argumentCountIs(1),
hasArgument(0, declRefExpr().bind("arg")),
-   anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
+   anyOf(hasAncestor(compoundStmt(
+ hasParent(lambdaExpr().bind("containing-lambda",
  hasAncestor(functionDecl().bind("containing-func"))),
unless(inDecltypeOrTemplateArg()),
// try_emplace is a common maybe-moving function that returns a


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp
@@ -416,6 +416,13 @@
 auto lambda = [&]() { a.foo(); };
 std::move(a);
   }
+  {
+A a;
+auto lambda = [a = std::move(a)] { a.foo(); };
+a.foo();
+// CHECK-NOTES: [[@LINE-1]]:5: warning: 'a' used after it was moved
+// CHECK-NOTES: [[@LINE-3]]:24: note: move occurred here
+  }
 }
 
 // Use-after-moves are detected in uninstantiated templates if the moved type
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -209,6 +209,11 @@
   ` when the specialization
   template has an unnecessary value paramter. Removed the fix for a template.
 
+- Fixed a bug in :doc:`bugprone-use-after-move
+   where a move in a lambda capture
+  was treated as if it happened within the body of the lambda, not within the
+  function that defines the lambda.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
@@ -400,7 +400,8 @@
   auto CallMoveMatcher =
   callExpr(callee(functionDecl(hasName("::std::move"))), argumentCountIs(1),
hasArgument(0, declRefExpr().bind("arg")),
-   anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
+   anyOf(hasAncestor(compoundStmt(
+ hasParent(lambdaExpr().bind("containing-lambda",
  hasAncestor(functionDecl().bind("containing-func"))),
unless(inDecltypeOrTemplateArg()),
// try_emplace is a common maybe-moving function that returns a
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119165: [clang-tidy] Add processing lambda captures at bugprone-use-after-move check

[Martin Böhme via Phabricator via cfe-commits]

mboehme added a comment.

Note: https://reviews.llvm.org/D126780, which fixes the same bug as this patch, 
has now been landed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119165

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

[PATCH] D126672: [Driver] Add multiarch path for RISC-V

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D126672#3546782 , @Hahnfeld wrote:

> In D126672#3546773 , @MaskRay wrote:
>
>> This needs a test.
>
> There are no tests for any of the other architectures.
>
>> Can Debian's riscv GCC be fixed to use a normalized triple for library and 
>> include paths?
>
> This is not specific to RISC-V. As you can see by all the other `case`s, 
> Clang's target triple is actually not normalized wrt to distro layout.

Clang's target triple is normalized, as you can see from its `clang 
-dumpmachine` output.
However, Debian chose to have a different opinion and omit `unknown`/`pc` from 
`-dumpmachine` output.
This added significant overhead to the maintenance of clang driver.
Last year we have this problem that D107799  
has been reverted and nobody gives insight whether relanding it will break 
things, so it just gets stuck.

For Debian, it'd be entirely fine to keep the target triple normalized while 
providing executable symlinks without `known`, e.g. 
`/usr/bin/powerpc64le-linux-gnu-gcc`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126672

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

[PATCH] D124758: [analyzer] Implement assume in terms of assumeDual

[Alex Brachet via Phabricator via cfe-commits]

abrachet added a comment.

As a heads up, because I'm not sure how often folks look at Github Issues. This 
patch causes a stack overflow on some Objective-C++ code. I have filed 
https://github.com/llvm/llvm-project/issues/55851. Could you take a look 
@martong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124758

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

[clang] a459d1e - [clang][sema] Remove unused paramter from VerifyBitField

[Timm Bäder via cfe-commits]

Author: Timm Bäder
Date: 2022-06-03T09:52:37+02:00
New Revision: a459d1eb2c779516652b3e6863cc3973d9bfbbef

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

LOG: [clang][sema] Remove unused paramter from VerifyBitField

The ZeroWidth paramter is unused in every call site of VerifyBitField.

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 43dbf50d2829d..8df7e185cc55a 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -12381,10 +12381,8 @@ class Sema final {
   /// VerifyBitField - verifies that a bit field expression is an ICE and has
   /// the correct width, and that the field type is valid.
   /// Returns false on success.
-  /// Can optionally return whether the bit-field is of width 0
   ExprResult VerifyBitField(SourceLocation FieldLoc, IdentifierInfo *FieldName,
-QualType FieldTy, bool IsMsStruct,
-Expr *BitWidth, bool *ZeroWidth = nullptr);
+QualType FieldTy, bool IsMsStruct, Expr *BitWidth);
 
 private:
   unsigned ForceCUDAHostDeviceDepth = 0;

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index f349a86ec2dac..cfba17d6e23d8 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -17094,17 +17094,12 @@ void Sema::ActOnTagDefinitionError(Scope *S, Decl 
*TagD) {
 
 // Note that FieldName may be null for anonymous bitfields.
 ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
-IdentifierInfo *FieldName,
-QualType FieldTy, bool IsMsStruct,
-Expr *BitWidth, bool *ZeroWidth) {
+IdentifierInfo *FieldName, QualType FieldTy,
+bool IsMsStruct, Expr *BitWidth) {
   assert(BitWidth);
   if (BitWidth->containsErrors())
 return ExprError();
 
-  // Default to true; that shouldn't confuse checks for emptiness
-  if (ZeroWidth)
-*ZeroWidth = true;
-
   // C99 6.7.2.1p4 - verify the field type.
   // C++ 9.6p3: A bit-field shall have integral or enumeration type.
   if (!FieldTy->isDependentType() && !FieldTy->isIntegralOrEnumerationType()) {
@@ -17132,9 +17127,6 @@ ExprResult Sema::VerifyBitField(SourceLocation FieldLoc,
 return ICE;
   BitWidth = ICE.get();
 
-  if (Value != 0 && ZeroWidth)
-*ZeroWidth = false;
-
   // Zero-width bitfield is ok for anonymous field.
   if (Value == 0 && FieldName)
 return Diag(FieldLoc, diag::err_bitfield_has_zero_width) << FieldName;
@@ -17387,17 +17379,15 @@ FieldDecl *Sema::CheckFieldDecl(DeclarationName Name, 
QualType T,
  AbstractFieldType))
 InvalidDecl = true;
 
-  bool ZeroWidth = false;
   if (InvalidDecl)
 BitWidth = nullptr;
   // If this is declared as a bit-field, check the bit-field.
   if (BitWidth) {
-BitWidth = VerifyBitField(Loc, II, T, Record->isMsStruct(Context), 
BitWidth,
-  &ZeroWidth).get();
+BitWidth =
+VerifyBitField(Loc, II, T, Record->isMsStruct(Context), 
BitWidth).get();
 if (!BitWidth) {
   InvalidDecl = true;
   BitWidth = nullptr;
-  ZeroWidth = false;
 }
   }
 



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

[clang] c698189 - [NFC] Format CGBuilder.h

[Guillaume Chatelet via cfe-commits]

Author: Guillaume Chatelet
Date: 2022-06-03T07:54:01Z
New Revision: c698189696d33e7304d94cd4212bd81818ea81a0

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

LOG: [NFC] Format CGBuilder.h

Added: 


Modified: 
clang/lib/CodeGen/CGBuilder.h

Removed: 




diff  --git a/clang/lib/CodeGen/CGBuilder.h b/clang/lib/CodeGen/CGBuilder.h
index c087aa463588..68618df60155 100644
--- a/clang/lib/CodeGen/CGBuilder.h
+++ b/clang/lib/CodeGen/CGBuilder.h
@@ -32,6 +32,7 @@ class CGBuilderInserter final : public 
llvm::IRBuilderDefaultInserter {
   void InsertHelper(llvm::Instruction *I, const llvm::Twine &Name,
 llvm::BasicBlock *BB,
 llvm::BasicBlock::iterator InsertPt) const override;
+
 private:
   CodeGenFunction *CGF = nullptr;
 };
@@ -45,17 +46,18 @@ class CGBuilderTy : public CGBuilderBaseTy {
   /// Storing a reference to the type cache here makes it a lot easier
   /// to build natural-feeling, target-specific IR.
   const CodeGenTypeCache &TypeCache;
+
 public:
   CGBuilderTy(const CodeGenTypeCache &TypeCache, llvm::LLVMContext &C)
-: CGBuilderBaseTy(C), TypeCache(TypeCache) {}
-  CGBuilderTy(const CodeGenTypeCache &TypeCache,
-  llvm::LLVMContext &C, const llvm::ConstantFolder &F,
+  : CGBuilderBaseTy(C), TypeCache(TypeCache) {}
+  CGBuilderTy(const CodeGenTypeCache &TypeCache, llvm::LLVMContext &C,
+  const llvm::ConstantFolder &F,
   const CGBuilderInserterTy &Inserter)
-: CGBuilderBaseTy(C, F, Inserter), TypeCache(TypeCache) {}
+  : CGBuilderBaseTy(C, F, Inserter), TypeCache(TypeCache) {}
   CGBuilderTy(const CodeGenTypeCache &TypeCache, llvm::Instruction *I)
-: CGBuilderBaseTy(I), TypeCache(TypeCache) {}
+  : CGBuilderBaseTy(I), TypeCache(TypeCache) {}
   CGBuilderTy(const CodeGenTypeCache &TypeCache, llvm::BasicBlock *BB)
-: CGBuilderBaseTy(BB), TypeCache(TypeCache) {}
+  : CGBuilderBaseTy(BB), TypeCache(TypeCache) {}
 
   llvm::ConstantInt *getSize(CharUnits N) {
 return llvm::ConstantInt::get(TypeCache.SizeTy, N.getQuantity());
@@ -102,7 +104,8 @@ class CGBuilderTy : public CGBuilderBaseTy {
 
   using CGBuilderBaseTy::CreateAlignedStore;
   llvm::StoreInst *CreateAlignedStore(llvm::Value *Val, llvm::Value *Addr,
-  CharUnits Align, bool IsVolatile = 
false) {
+  CharUnits Align,
+  bool IsVolatile = false) {
 return CreateAlignedStore(Val, Addr, Align.getAsAlign(), IsVolatile);
   }
 
@@ -165,8 +168,8 @@ class CGBuilderTy : public CGBuilderBaseTy {
   Address CreateElementBitCast(Address Addr, llvm::Type *Ty,
const llvm::Twine &Name = "") {
 auto *PtrTy = Ty->getPointerTo(Addr.getAddressSpace());
-return Address(CreateBitCast(Addr.getPointer(), PtrTy, Name),
-   Ty, Addr.getAlignment());
+return Address(CreateBitCast(Addr.getPointer(), PtrTy, Name), Ty,
+   Addr.getAlignment());
   }
 
   using CGBuilderBaseTy::CreatePointerBitCastOrAddrSpaceCast;
@@ -193,10 +196,10 @@ class CGBuilderTy : public CGBuilderBaseTy {
 const llvm::StructLayout *Layout = DL.getStructLayout(ElTy);
 auto Offset = CharUnits::fromQuantity(Layout->getElementOffset(Index));
 
-return Address(CreateStructGEP(Addr.getElementType(),
-   Addr.getPointer(), Index, Name),
-   ElTy->getElementType(Index),
-   Addr.getAlignment().alignmentAtOffset(Offset));
+return Address(
+CreateStructGEP(Addr.getElementType(), Addr.getPointer(), Index, Name),
+ElTy->getElementType(Index),
+Addr.getAlignment().alignmentAtOffset(Offset));
   }
 
   /// Given
@@ -264,10 +267,10 @@ class CGBuilderTy : public CGBuilderBaseTy {
 CharUnits EltSize =
 CharUnits::fromQuantity(DL.getTypeAllocSize(Addr.getElementType()));
 
-return Address(CreateGEP(Addr.getElementType(), Addr.getPointer(), Index,
- Name),
-   Addr.getElementType(),
-   Addr.getAlignment().alignmentOfArrayElement(EltSize));
+return Address(
+CreateGEP(Addr.getElementType(), Addr.getPointer(), Index, Name),
+Addr.getElementType(),
+Addr.getAlignment().alignmentOfArrayElement(EltSize));
   }
 
   /// Given a pointer to i8, adjust it by a given constant offset.
@@ -342,8 +345,7 @@ class CGBuilderTy : public CGBuilderBaseTy {
   }
 
   using CGBuilderBaseTy::CreatePreserveStructAccessIndex;
-  Address CreatePreserveStructAccessIndex(Address Addr,
-  unsigned Index,
+  Address CreatePreserveStructAccessIndex(Address Addr, unsigned Index

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 433978.
gchatelet added a comment.

- Rebase over formatted CGBuilder.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126903

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-memset-inline.c
  clang/test/Sema/builtins-memset-inline.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Analysis/Lint.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
  llvm/test/CodeGen/X86/memset-inline.ll
  llvm/test/Other/lint.ll
  llvm/test/Verifier/intrinsic-immarg.ll
  llvm/test/Verifier/memset-inline.ll

Index: llvm/test/Verifier/memset-inline.ll
===
--- /dev/null
+++ llvm/test/Verifier/memset-inline.ll
@@ -0,0 +1,9 @@
+; RUN: not opt -verify < %s 2>&1 | FileCheck %s
+
+; CHECK: alignment is not a power of two 
+
+define void @foo(i8* %P, i8 %value) {
+  call void @llvm.memset.inline.p0i8.i32(i8* align 3 %P, i8 %value, i32 4, i1 false)
+  ret void
+}
+declare void @llvm.memset.inline.p0i8.i32(i8* nocapture, i8, i32, i1) nounwind
Index: llvm/test/Verifier/intrinsic-immarg.ll
===
--- llvm/test/Verifier/intrinsic-immarg.ll
+++ llvm/test/Verifier/intrinsic-immarg.ll
@@ -62,6 +62,23 @@
   ret void
 }
 
+declare void @llvm.memset.inline.p0i8.i32(i8* nocapture, i8, i32, i1)
+define void @memset_inline_is_volatile(i8* %dest, i8 %value, i1 %is.volatile) {
+  ; CHECK: immarg operand has non-immediate parameter
+  ; CHECK-NEXT: i1 %is.volatile
+  ; CHECK-NEXT: call void @llvm.memset.inline.p0i8.i32(i8* %dest, i8 %value, i32 8, i1 %is.volatile)
+  call void @llvm.memset.inline.p0i8.i32(i8* %dest, i8 %value, i32 8, i1 %is.volatile)
+  ret void
+}
+
+define void @memset_inline_variable_size(i8* %dest, i8 %value, i32 %size) {
+  ; CHECK: immarg operand has non-immediate parameter
+  ; CHECK-NEXT: i32 %size
+  ; CHECK-NEXT: call void @llvm.memset.inline.p0i8.i32(i8* %dest, i8 %value, i32 %size, i1 true)
+  call void @llvm.memset.inline.p0i8.i32(i8* %dest, i8 %value, i32 %size, i1 true)
+  ret void
+}
+
 
 declare i64 @llvm.objectsize.i64.p0i8(i8*, i1, i1, i1)
 define void @objectsize(i8* %ptr, i1 %a, i1 %b, i1 %c) {
Index: llvm/test/Other/lint.ll
===
--- llvm/test/Other/lint.ll
+++ llvm/test/Other/lint.ll
@@ -6,6 +6,8 @@
 declare void @llvm.stackrestore(i8*)
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i1) nounwind
 declare void @llvm.memcpy.inline.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i1) nounwind
+declare void @llvm.memset.p0i8.i8.i64(i8* nocapture, i8, i64, i1) nounwind
+declare void @llvm.memset.inline.p0i8.i8.i64(i8* nocapture, i8, i64, i1) nounwind
 declare void @has_sret(i8* sret(i8) %p)
 declare void @has_noaliases(i32* noalias %p, i32* %q)
 declare void @one_arg(i32)
@@ -87,6 +89,11 @@
 ; CHECK: Unusual: noalias argument aliases another argument
 call void @llvm.memcpy.p0i8.p0i8.i64(i8* bitcast (i32* @CG to i8*), i8* bitcast (i32* @CG to i8*), i64 1, i1 0)
 
+; CHECK: Write to read-only memory
+call void @llvm.memset.p0i8.i8.i64(i8* bitcast (i32* @CG to i8*), i8 1, i64 1, i1 0)
+; CHECK: Write to read-only memory
+call void @llvm.memset.inline.p0i8.i8.i64(i8* bitcast (i32* @CG to i8*), i8 1, i64 1, i1 0)
+
 ; CHECK: Undefined behavior: Buffer overflow
   %wider = bitcast i8* %buf to i16*
   store i16 0, i16* %wider
Index: llvm/test/CodeGen/X86/memset-inline.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/memset-inline.ll
@@ -0,0 +1,56 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=core2 | FileCheck %s
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i1) nounwind
+declare void @llvm.memset.inline.p0i8.i64(i8* nocapture, i8, i64, i1) nounwind
+
+define void @test1(i8* %a, i8 %value) nounwind {
+; CHECK-LABEL: test1:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:# kill: def $esi killed $esi def $rsi
+; CHECK-NEXT:movzbl %sil, %eax
+; CHECK-NEXT:movabsq $72340172838076673, %rcx # imm = 0x101010101010101
+; CHECK-NEXT:imulq %rax, %rcx
+; CHECK-NEXT:movq %rcx, (%rdi)
+; CHECK-NEXT:retq
+  tail call void @llvm.memset.inline.p0i8.i64(i8* %a, i8 %value, i64 8, i1 0)
+  ret void
+}
+
+define void @regular_memset_calls_external_function(i8* %a, 

[PATCH] D126781: [CodeGen] Keep track info of lazy-emitted symbols in ModuleBuilder

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added a comment.

In D126781#3554845 , @rjmccall wrote:

> Okay, I understand.  So, first off, I wouldn't really call that a "weak" 
> symbol rather than, say, a lazily-emitted symbol; "weak" already has plenty 
> of different senses, and we should try to avoid coining more.  Also, your 
> patch description makes it sound like there's a general bug you're fixing 
> rather than specifically an issue with re-using a single `CodeGenerator` to 
> generate a succession of modules; please remember that people reading your 
> commit messages don't necessarily know you or contextualize your patches by 
> knowing that you work on Cling.

Yes, you are right. We will update the patch title and summary.

> On a more technical note, it's now clear that the main thrust of your patch 
> is the state persistence in `StartModule`.  Your patch is effectively adding 
> a feature where `StartModule` can be invoked multiple times (assuming it's 
> been appropriately finalized from earlier invocations).

Yes!

> I think that's fine, although I imagine it will need a lot of further changes 
> to allow linkage between these modules.

Currently, we do that outside of CodeGen in a separate module pass which looks 
like this:

  
  bool runOnGlobal(GlobalValue& GV) {
if (GV.isDeclaration())
  return false; // no change.
  
// GV is a definition.
  
// It doesn't make sense to keep unnamed constants, we wouldn't know how
// to reference them anyway.
if (!GV.hasName())
  return false;
  
if (GV.getName().startswith(".str"))
  return false;
  
llvm::GlobalValue::LinkageTypes LT = GV.getLinkage();
if (!GV.isDiscardableIfUnused(LT))
  return false;
  
if (LT == llvm::GlobalValue::InternalLinkage
|| LT == llvm::GlobalValue::PrivateLinkage) {
  // We want to keep this GlobalValue around, but have to tell the JIT
  // linker that it should not error on duplicate symbols.
  // FIXME: Ideally the frontend would never emit duplicate symbols and
  // we could just use the old version of saying:
  // GV.setLinkage(llvm::GlobalValue::ExternalLinkage);
  GV.setLinkage(llvm::GlobalValue::WeakAnyLinkage);
  return true; // a change!
}
return false;
  }

The new JIT infrastructure does not ignore duplicate symbols anymore which 
makes it more challenging to adjust linkage. Ideally, we should CodeGen know 
what's needed to make code in two different llvm::Modules work... I do not have 
a feeling if that requires a lot of changes or even redesign. I think this 
might become clearer when we start upstreaming the code "undo" patches where 
CodeGen's state should be reverted to a previous partial translation unit. 
@junaire, maybe it is a good time to make your patch in that area public and 
add the current reviewers.

> That doesn't really explain why you needed to add a new field to the 
> CodeGenModule, though.

Good question. The original idea of `EmittedDeferredDecls` (patch was developed 
in 2017) was to track deferred decls that were emitted in a previous 
llvm::Module. We checked and it is not needed to fix the current test case. We 
also are checking on the bigger infrastructure if we can drop this field.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126781

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

[PATCH] D126891: [clang-tidy] The check should ignore final classes

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

steakhal added a comment.

In D126891#3554039 , @carlosgalvezp 
wrote:

> Hmm, `MostDerived` **does** have a public virtual destructor in your example 
> already - if the base class destructor is virtual, the child class destructor 
> is virtual. In that sense the check should not warn.
>
> Seems like there's some deeper problem in the check?

Not quite. None of the destructors are virtual in the example. In fact, the 
`MostDerived` has a //public// and //non-virtual// destructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126891

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

[PATCH] D126891: [clang-tidy] The check should ignore final classes

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D126891#3555456 , @steakhal wrote:

> In D126891#3554039 , @carlosgalvezp 
> wrote:
>
>> Hmm, `MostDerived` **does** have a public virtual destructor in your example 
>> already - if the base class destructor is virtual, the child class 
>> destructor is virtual. In that sense the check should not warn.
>>
>> Seems like there's some deeper problem in the check?
>
> Not quite. None of the destructors are virtual in the example. In fact, the 
> `MostDerived` has a //public// and //non-virtual// destructor.

Doesn't `Base` have a virtual destructor? As per the standard 
:

> If a class has a base class with a virtual destructor, its destructor 
> (whether user- or implicitly-declared) is virtual.

To clarify: I'm not opposed to the patch, I'm just trying to understand if the 
AST is indeed implementing the Standard correctly and classifying the `Derived` 
destructor as virtual.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126891

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

[PATCH] D126891: [clang-tidy] The check should ignore final classes

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Oh, I see the unit test now, indeed `Base` does not have a virtual destructor. 
LGTM then!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126891

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

[PATCH] D126891: [clang-tidy] The check should ignore final classes

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:170
+  ` involving
+  `final` classes. The check will not diagnose `final` marked classes, since
+  those cannot be used as base classes, consequently they can not violate the

I believe the convention is to use double-backtick for C++ keywords and 
functions, see line 160.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126891

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

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

[Guillaume Chatelet via Phabricator via cfe-commits]

gchatelet updated this revision to Diff 433982.
gchatelet added a comment.

- Fix sema checking


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126903

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/Builtins.def
  clang/lib/CodeGen/CGBuilder.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/builtins-memset-inline.c
  clang/test/Sema/builtins-memcpy-inline.cpp
  clang/test/Sema/builtins-memset-inline.cpp
  llvm/docs/LangRef.rst
  llvm/include/llvm/CodeGen/SelectionDAG.h
  llvm/include/llvm/IR/IRBuilder.h
  llvm/include/llvm/IR/IntrinsicInst.h
  llvm/include/llvm/IR/Intrinsics.td
  llvm/lib/Analysis/Lint.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
  llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
  llvm/lib/IR/IRBuilder.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
  llvm/test/CodeGen/X86/memset-inline.ll
  llvm/test/Other/lint.ll
  llvm/test/Verifier/intrinsic-immarg.ll
  llvm/test/Verifier/memset-inline.ll

Index: llvm/test/Verifier/memset-inline.ll
===
--- /dev/null
+++ llvm/test/Verifier/memset-inline.ll
@@ -0,0 +1,9 @@
+; RUN: not opt -verify < %s 2>&1 | FileCheck %s
+
+; CHECK: alignment is not a power of two 
+
+define void @foo(i8* %P, i8 %value) {
+  call void @llvm.memset.inline.p0i8.i32(i8* align 3 %P, i8 %value, i32 4, i1 false)
+  ret void
+}
+declare void @llvm.memset.inline.p0i8.i32(i8* nocapture, i8, i32, i1) nounwind
Index: llvm/test/Verifier/intrinsic-immarg.ll
===
--- llvm/test/Verifier/intrinsic-immarg.ll
+++ llvm/test/Verifier/intrinsic-immarg.ll
@@ -62,6 +62,23 @@
   ret void
 }
 
+declare void @llvm.memset.inline.p0i8.i32(i8* nocapture, i8, i32, i1)
+define void @memset_inline_is_volatile(i8* %dest, i8 %value, i1 %is.volatile) {
+  ; CHECK: immarg operand has non-immediate parameter
+  ; CHECK-NEXT: i1 %is.volatile
+  ; CHECK-NEXT: call void @llvm.memset.inline.p0i8.i32(i8* %dest, i8 %value, i32 8, i1 %is.volatile)
+  call void @llvm.memset.inline.p0i8.i32(i8* %dest, i8 %value, i32 8, i1 %is.volatile)
+  ret void
+}
+
+define void @memset_inline_variable_size(i8* %dest, i8 %value, i32 %size) {
+  ; CHECK: immarg operand has non-immediate parameter
+  ; CHECK-NEXT: i32 %size
+  ; CHECK-NEXT: call void @llvm.memset.inline.p0i8.i32(i8* %dest, i8 %value, i32 %size, i1 true)
+  call void @llvm.memset.inline.p0i8.i32(i8* %dest, i8 %value, i32 %size, i1 true)
+  ret void
+}
+
 
 declare i64 @llvm.objectsize.i64.p0i8(i8*, i1, i1, i1)
 define void @objectsize(i8* %ptr, i1 %a, i1 %b, i1 %c) {
Index: llvm/test/Other/lint.ll
===
--- llvm/test/Other/lint.ll
+++ llvm/test/Other/lint.ll
@@ -6,6 +6,8 @@
 declare void @llvm.stackrestore(i8*)
 declare void @llvm.memcpy.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i1) nounwind
 declare void @llvm.memcpy.inline.p0i8.p0i8.i64(i8* nocapture, i8* nocapture, i64, i1) nounwind
+declare void @llvm.memset.p0i8.i8.i64(i8* nocapture, i8, i64, i1) nounwind
+declare void @llvm.memset.inline.p0i8.i8.i64(i8* nocapture, i8, i64, i1) nounwind
 declare void @has_sret(i8* sret(i8) %p)
 declare void @has_noaliases(i32* noalias %p, i32* %q)
 declare void @one_arg(i32)
@@ -87,6 +89,11 @@
 ; CHECK: Unusual: noalias argument aliases another argument
 call void @llvm.memcpy.p0i8.p0i8.i64(i8* bitcast (i32* @CG to i8*), i8* bitcast (i32* @CG to i8*), i64 1, i1 0)
 
+; CHECK: Write to read-only memory
+call void @llvm.memset.p0i8.i8.i64(i8* bitcast (i32* @CG to i8*), i8 1, i64 1, i1 0)
+; CHECK: Write to read-only memory
+call void @llvm.memset.inline.p0i8.i8.i64(i8* bitcast (i32* @CG to i8*), i8 1, i64 1, i1 0)
+
 ; CHECK: Undefined behavior: Buffer overflow
   %wider = bitcast i8* %buf to i16*
   store i16 0, i16* %wider
Index: llvm/test/CodeGen/X86/memset-inline.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/memset-inline.ll
@@ -0,0 +1,56 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=core2 | FileCheck %s
+
+declare void @llvm.memset.p0i8.i64(i8* nocapture, i8, i64, i1) nounwind
+declare void @llvm.memset.inline.p0i8.i64(i8* nocapture, i8, i64, i1) nounwind
+
+define void @test1(i8* %a, i8 %value) nounwind {
+; CHECK-LABEL: test1:
+; CHECK:   # %bb.0:
+; CHECK-NEXT:# kill: def $esi killed $esi def $rsi
+; CHECK-NEXT:movzbl %sil, %eax
+; CHECK-NEXT:movabsq $72340172838076673, %rcx # imm = 0x101010101010101
+; CHECK-NEXT:imulq %rax, %rcx
+; CHECK-NEXT:movq %rcx, (%rdi)
+; CHECK-NEXT:retq
+  tail call void @llvm.memset.inline.p0i8.i64(i8* %a, i8 %value, i64 8, i1 0)
+  ret void
+}
+
+define void @regular_memset_cal

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

[Guillaume Chatelet via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaChecking.cpp:2293-2315
+  case Builtin::BI__builtin_memset_inline: {
+if (checkArgCount(*this, TheCall, 3))
+  return ExprError();
+auto ArgArrayConversionFailed = [&](unsigned Arg) {
+  ExprResult ArgExpr =
+  DefaultFunctionArrayLvalueConversion(TheCall->getArg(Arg));
+  if (ArgExpr.isInvalid())

rsmith wrote:
> This custom checking seems unnecessary and also insufficient: you're not 
> enforcing that the arguments have the right types. We should convert the 
> arguments to this builtin in the same way we'd convert arguments for a 
> normally-declared function, which you can achieve by removing the "t" flag 
> from the builtin definition and removing all of this logic (except for the 
> nonnull checking, which I don't think we have a way to model in Builtins.def 
> yet).
> 
> Please do the same to `__builtin_memcpy_inline`, which is wrong in the same 
> way. (For example, `__builtin_memcpy_inline(1, 2, 3)` crashes in code 
> generation because it fails to perform proper conversions and type-checking 
> on its arguments.)
Thx!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126903

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

[clang-tools-extra] b50542f - [clang-tidy] Add missing close quote in release notes.

[Martin Boehme via cfe-commits]

Author: Martin Boehme
Date: 2022-06-03T10:33:57+02:00
New Revision: b50542f21e95800ca1d49b50bddd8e91e0f256fc

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

LOG: [clang-tidy] Add missing close quote in release notes.

Sorry for the breakage.

Added: 


Modified: 
clang-tools-extra/docs/ReleaseNotes.rst

Removed: 




diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index a3e616357398..8ae33ab0a072 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -210,7 +210,7 @@ Changes in existing checks
   template has an unnecessary value paramter. Removed the fix for a template.
 
 - Fixed a bug in :doc:`bugprone-use-after-move
-   where a move in a lambda capture
+  ` where a move in a lambda capture
   was treated as if it happened within the body of the lambda, not within the
   function that defines the lambda.
 



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

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

[David Friberg via Phabricator via cfe-commits]

dfrib added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-const-or-ref-data-members.cpp:25
+
+struct RefMember {
+  int &r;

Differentiate between lvalue reference and rvalue reference members using these 
terms instead of "ref" and "refref". E.g.:

- `RefMember` -> `LvalueRefMember`
- `RefRefMember` -> `RvalueRefMember`

(apply to all cases below).



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-const-or-ref-data-members.cpp:40
+
+struct ConstAndRefMember {
+  int const c;

`ConstAndRefMembers`



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-const-or-ref-data-members.cpp:100
+};
+
+template 

Should we test for array types also? E.g.:

```
template using Array = int[N];

struct ConstArrayMember {
  const Array<1> c;
};

struct LvalueRefArrayMember {
  Array<2>& lvr;   
};

struct ConstRefArrayMember {
  Array<3> const& clvr;
};

struct LvalueRefArrayMember {
  Array<4>&& rvr;   
};
```



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-const-or-ref-data-members.cpp:121
+TemplatedRef t3{123};
+TemplatedOk t4{};

Consider expanding with the the non-const lvalue ref case, e.g.

```
TemplatedOk t4{};
TemplateRef t5{t4.t};
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

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

[PATCH] D126891: [clang-tidy] The check should ignore final classes

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

steakhal added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:170
+  ` involving
+  `final` classes. The check will not diagnose `final` marked classes, since
+  those cannot be used as base classes, consequently they can not violate the

carlosgalvezp wrote:
> I believe the convention is to use double-backtick for C++ keywords and 
> functions, see line 160.
Thanks, I'll fix it. I should learn rst for once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126891

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

[PATCH] D126903: [clang] Add support for __builtin_memset_inline

[Clement Courbet via Phabricator via cfe-commits]

courbet added inline comments.



Comment at: clang/test/Sema/builtins-memcpy-inline.cpp:11
+void test_memcpy_inline_invalid_arg_types() {
+  __builtin_memcpy_inline(1, 2, 3); // expected-error {{cannot initialize a 
parameter of type 'void *' with an rvalue of type 'int'}}
+}

can you split this off (and the corresponding code) to a separate patch ?



Comment at: llvm/docs/LangRef.rst:13898
+Note that, unlike the standard libc function, the ``llvm.memset.inline.*``
+intrinsics do not return a value, takes extra isvolatile
+arguments and the pointers can be in specified address spaces.

courbet wrote:
> "an extra `isvolatile` argument"
take*



Comment at: llvm/docs/LangRef.rst:13898-13899
+Note that, unlike the standard libc function, the ``llvm.memset.inline.*``
+intrinsics do not return a value, takes extra isvolatile
+arguments and the pointers can be in specified address spaces.
+

"an extra `isvolatile` argument"



Comment at: llvm/docs/LangRef.rst:13899
+intrinsics do not return a value, takes extra isvolatile
+arguments and the pointers can be in specified address spaces.
+

pointer*



Comment at: llvm/include/llvm/IR/IntrinsicInst.h:993
 
 /// This class wraps the llvm.memset intrinsic.
 class MemSetInst : public MemSetBase {

update comment



Comment at: llvm/include/llvm/IR/Intrinsics.td:654
 
+// Memset semantic that is guaranteed to be inlined.
+// In particular this means that the generated code is not allowed to call any

"version" ? or "semantics"



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7334
   if (TSI) {
 SDValue Result = TSI->EmitTargetCodeForMemset(
 *this, dl, Chain, Dst, Src, Size, Alignment, isVol, DstPtrInfo);

There is nothing preveting a target from emitting a call to `memset` here when 
`AlwaysInline` is `true`. Actually, `X86SelectionDAGInfo` does just that (this 
very patch is adding `/* AlwaysInline */ false,` to the `getMemset` call that 
handles the trailing bytes). It happens that because trailing bytes are 
typically small and therefore inline. it happens to work, but this should be 
verified somehow (or, maybe easier,  `AlwaysInline` should be passed to 
`EmitTargetCodeForMemset` so it can do the right thing).



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:7344
+assert(ConstantSize && "AlwaysInline requires a constant size!");
+return getMemsetStores(*this, dl, Chain, Dst, Src,
+   ConstantSize->getZExtValue(), Alignment, isVol, 
true,

Not that this is not strictly required to return a valid `SDValue`: Even with 
an "infinite" `Limit` in `TLI.findOptimalMemOpLowering`, a target that 
overrides this function could decide to just return false. I'm not sure what we 
want to do in this case. 
So I think we should document `findOptimalMemOpLowering` to mention that the 
default implementation always returns a valid memop lowering if `Limit` is 
`UINT_MAX` and that target that decide to not provide a memop lowering *must* 
emit a valid one in `EmitTargetCodeForMemset`. Another option would be to call 
the generic `TargetLowering::findOptimalMemOpLowering` when the target declines 
to generate eithe ra memop lowering or target-specific code.



Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5928
+bool isTC = I.isTailCall() && isInTailCallPosition(I, DAG.getTarget());
+// FIXME: Support passing different dest/src alignments to the memcpy DAG
+// node.

memset



Comment at: llvm/test/CodeGen/X86/memset-inline.ll:1
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mcpu=core2 | FileCheck %s

Add tests for other targets ?



Comment at: llvm/test/CodeGen/X86/memset-inline.ll:21
+define void @regular_memset_calls_external_function(i8* %a, i8 %value) 
nounwind {
+; CHECK-LABEL: regular_memset_calls_external_function:
+; CHECK:   # %bb.0:

I could see memset deciding to inline 129 bytes in the future. What about using 
a more absurdly large number ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126903

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Diana Picus via Phabricator via cfe-commits]

rovka updated this revision to Diff 433988.
rovka added a comment.

- Update for MinGW.
- Add `/subsystem:console` to help `link.exe` understand what's going on.

Thanks for all the comments. I don't have a MinGW environment readily available 
for testing. @mmuetzel Could you test this? Alternatively, do you know whether 
we have any buildbots that would test this?


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

https://reviews.llvm.org/D126291

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/lib/Driver/ToolChains/Gnu.cpp
  clang/lib/Driver/ToolChains/MSVC.cpp
  flang/test/Driver/linker-flags-windows.f90
  flang/test/Driver/linker-flags.f90

Index: flang/test/Driver/linker-flags.f90
===
--- flang/test/Driver/linker-flags.f90
+++ flang/test/Driver/linker-flags.f90
@@ -5,9 +5,9 @@
 ! NOTE: The additional linker flags tested here are currently only specified for
 ! GNU and Darwin. The following line will make sure that this test is skipped on
 ! Windows. If you are running this test on a yet another platform and it is
-! failing for you, please either update the following or (preferably) update the
-! linker invocation accordingly.
-! UNSUPPORTED: system-windows
+! failing for you, please either update the following or (preferably)
+! create a similar test for your platform.
+! UNSUPPORTED: windows-msvc
 
 !
 ! RUN COMMAND
Index: flang/test/Driver/linker-flags-windows.f90
===
--- /dev/null
+++ flang/test/Driver/linker-flags-windows.f90
@@ -0,0 +1,25 @@
+! Verify that the Fortran runtime libraries are present in the linker
+! invocation. These libraries are added on top of other standard runtime
+! libraries that the Clang driver will include.
+
+! NOTE: The additional linker flags tested here are currently specified in
+! clang/lib/Driver/Toolchains/MSVC.cpp.
+! REQUIRES: windows-msvc
+
+! RUN: %flang -### %S/Inputs/hello.f90 2>&1 | FileCheck %s
+
+! Compiler invocation to generate the object file
+! CHECK-LABEL: {{.*}} "-emit-obj"
+! CHECK-SAME:  "-o" "[[object_file:.*]]" {{.*}}Inputs/hello.f90
+
+! Linker invocation to generate the executable
+! NOTE: This check should also match if the default linker is lld-link.exe
+! CHECK-LABEL: link.exe
+! CHECK-NOT: libcmt
+! CHECK-NOT: oldnames
+! CHECK-SAME: Fortran_main.lib
+! CHECK-SAME: FortranRuntime.lib
+! CHECK-SAME: FortranDecimal.lib
+! CHECK-SAME: /subsystem:console
+! CHECK-SAME: "[[object_file]]"
+
Index: clang/lib/Driver/ToolChains/MSVC.cpp
===
--- clang/lib/Driver/ToolChains/MSVC.cpp
+++ clang/lib/Driver/ToolChains/MSVC.cpp
@@ -81,7 +81,7 @@
 Args.MakeArgString(std::string("-out:") + Output.getFilename()));
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles) &&
-  !C.getDriver().IsCLMode()) {
+  !C.getDriver().IsCLMode() && !C.getDriver().IsFlangMode()) {
 CmdArgs.push_back("-defaultlib:libcmt");
 CmdArgs.push_back("-defaultlib:oldnames");
   }
@@ -130,6 +130,17 @@
   Args.MakeArgString(std::string("-libpath:") + WindowsSdkLibPath));
   }
 
+  if (C.getDriver().IsFlangMode()) {
+tools::addFortranRuntimeLibraryPath(TC, Args, CmdArgs);
+tools::addFortranRuntimeLibs(TC, CmdArgs);
+
+// Inform the MSVC linker that we're generating a console application, i.e.
+// one with `main` as the "user-defined" entry point. The `main` function is
+// defined in flang's runtime libraries.
+if (TC.getTriple().isKnownWindowsMSVCEnvironment())
+  CmdArgs.push_back("/subsystem:console");
+  }
+
   // Add the compiler-rt library directories to libpath if they exist to help
   // the linker find the various sanitizer, builtin, and profiling runtimes.
   for (const auto &LibPath : TC.getLibraryPaths()) {
Index: clang/lib/Driver/ToolChains/Gnu.cpp
===
--- clang/lib/Driver/ToolChains/Gnu.cpp
+++ clang/lib/Driver/ToolChains/Gnu.cpp
@@ -599,7 +599,7 @@
   // TODO: Make this work unconditionally once Flang is mature enough.
   if (D.IsFlangMode() && Args.hasArg(options::OPT_flang_experimental_exec)) {
 addFortranRuntimeLibraryPath(ToolChain, Args, CmdArgs);
-addFortranRuntimeLibs(CmdArgs);
+addFortranRuntimeLibs(ToolChain, CmdArgs);
 CmdArgs.push_back("-lm");
   }
 
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -644,7 +644,7 @@
   if (getToolChain().getDriver().IsFlangMode() &&
   Args.hasArg(options::OPT_flang_experimental_exec)) {
 addFortranRuntimeLibraryPath(getToolChain(), Args, CmdArgs);
-addFortranRuntimeLibs(CmdArgs);
+addFortranRun

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 433989.
carlosgalvezp marked 4 inline comments as done.
carlosgalvezp added a comment.

Fix review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidConstOrRefDataMembersCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-avoid-const-or-ref-data-members.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-const-or-ref-data-members.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-const-or-ref-data-members.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-avoid-const-or-ref-data-members.cpp
@@ -0,0 +1,152 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-const-or-ref-data-members %t
+namespace std {
+template 
+struct reference_wrapper {};
+} // namespace std
+
+namespace gsl {
+template 
+struct not_null {};
+} // namespace gsl
+
+struct Ok {
+  int i;
+  int *p;
+  const int *pc;
+  std::reference_wrapper r;
+  gsl::not_null n;
+};
+
+struct ConstMember {
+  const int c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
+};
+
+struct LvalueRefMember {
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+};
+
+struct ConstRefMember {
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+};
+
+struct RvalueRefMember {
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct ConstAndRefMembers {
+  int const c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  int &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+  const int &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+  int &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct Foo {};
+
+struct Ok2 {
+  Foo i;
+  Foo *p;
+  const Foo *pc;
+  std::reference_wrapper r;
+  gsl::not_null n;
+};
+
+struct ConstMember2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified [cppcoreguidelines-avoid-const-or-ref-data-members]
+};
+
+struct LvalueRefMember2 {
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+};
+
+struct ConstRefMember2 {
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+};
+
+struct RvalueRefMember2 {
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+struct ConstAndRefMembers2 {
+  const Foo c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  Foo &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: member 'lr' is a reference
+  const Foo &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'cr' is a reference
+  Foo &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: member 'rr' is a reference
+};
+
+using ConstType = const int;
+using RefType = int &;
+using ConstRefType = const int &;
+using RefRefType = int &&;
+
+struct WithAlias {
+  ConstType c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'c' is const qualified
+  RefType lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: member 'lr' is a reference
+  ConstRefType cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: member 'cr' is a reference
+  RefRefType rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' is a reference
+};
+
+template 
+using Array = int[N];
+
+struct ConstArrayMember {
+  const Array<1> c;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: member 'c' is const qualified
+};
+
+struct LvalueRefArrayMember {
+  Array<2> &lr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: member 'lr' is a reference
+};
+
+struct ConstLvalueRefArrayMember {
+  Array<3> const &cr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:19: warning: member 'cr' is a reference
+};
+
+struct RvalueRefArrayMember {
+  Array<4> &&rr;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: member 'rr' is a reference
+};
+
+template 
+struct TemplatedOk {
+  T t;
+};
+
+template 
+struct TemplatedConst {
+  T t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' is const qualified
+};
+
+template 
+struct TemplatedRef {
+  T t;
+  // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: member 't' is a reference
+};
+
+TemplatedOk t1{};
+TemplatedConst t2{123};
+TemplatedRef t3{123};
+Templa

[PATCH] D126880: [clang-tidy] Add cppcoreguidelines-avoid-const-or-ref-data-members check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Fixed comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126880

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

[PATCH] D112916: Confusable identifiers detection

[serge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb94db7ed7eaf: [clang-tidy] Confusable identifiers detection 
(authored by serge-sans-paille).

Changed prior to commit:
  https://reviews.llvm.org/D112916?vs=410776&id=433991#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112916

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/ConfusableTable/build_confusable_table.cpp
  clang-tools-extra/clang-tidy/misc/ConfusableTable/confusables.txt
  clang-tools-extra/clang-tidy/misc/Homoglyph.cpp
  clang-tools-extra/clang-tidy/misc/Homoglyph.h
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc-homoglyph.rst
  clang-tools-extra/test/clang-tidy/checkers/misc-homoglyph.cpp

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Markus Mützel via Phabricator via cfe-commits]

mmuetzel added a comment.

In D126291#356 , @rovka wrote:

> - Update for MinGW.
> - Add `/subsystem:console` to help `link.exe` understand what's going on.
>
> Thanks for all the comments. I don't have a MinGW environment readily 
> available for testing. @mmuetzel Could you test this? Alternatively, do you 
> know whether we have any buildbots that would test this?

Sorry. I don't know how to compile flang in MinGW. (ISTR, I somewhere read that 
Windows isn't a supported host currently. Is this no longer the case?)
I also know nothing about the available buildbots.

If you could give some pointers to available instructions, I could potentially 
try to set up an MSYS2 environment for this.


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

https://reviews.llvm.org/D126291

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

[PATCH] D125479: [pseudo] Fix the incorrect parameters-and-qualifiers rule.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

friendly ping.




Comment at: clang-tools-extra/pseudo/test/glr.cpp:37
+
+void foo2(int, ...);
+// CHECK:  declaration~simple-declaration := decl-specifier-seq 
init-declarator-list ;

since we have the builtin pseudoCXX library now, we could use it to write a cxx 
unittest, so that we can get rid of this cxx lit test. what do you think? (I 
have a feeling we might add more in the future, I just discovered a new 
oversight in our cxx.bnf grammar).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125479

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

[PATCH] D112916: Confusable identifiers detection

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

I haven't checked, but this probably doesn't do the right thing in cross builds 
(eg building a Mac/arm binary on a Mac/Intel machine - i think the 
gen_confusables binary will likely end up being an arm binary then, and it 
won't be able to run during the build). See https://reviews.llvm.org/D126397 
and discussion there for how to handle that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112916

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

[PATCH] D126956: [tbaa] Handle base classes in struct tbaa

[Bruno De Fraine via Phabricator via cfe-commits]

brunodf created this revision.
brunodf added reviewers: jeroen.dobbelaere, eli.friedman.
Herald added a subscriber: kosarev.
Herald added a project: All.
brunodf requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is a fix for the miscompilation reported in 
https://github.com/llvm/llvm-project/issues/55384

Not adding a new test case since existing test cases already cover base classes 
(including new-struct-path tbaa).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126956

Files:
  clang/lib/CodeGen/CodeGenTBAA.cpp
  clang/test/CodeGen/tbaa-class.cpp
  clang/unittests/CodeGen/TBAAMetadataTest.cpp


Index: clang/unittests/CodeGen/TBAAMetadataTest.cpp
===
--- clang/unittests/CodeGen/TBAAMetadataTest.cpp
+++ clang/unittests/CodeGen/TBAAMetadataTest.cpp
@@ -968,13 +968,10 @@
   MConstInt(0)),
 MConstInt(0));
 
-  auto ClassDerived = MMTuple(
-MMString("_ZTS7Derived"),
-MMTuple(
-  MMString("short"),
-  OmnipotentCharCXX,
-  MConstInt(0)),
-MConstInt(4));
+  auto ClassDerived =
+  MMTuple(MMString("_ZTS7Derived"), ClassBase, MConstInt(0),
+  MMTuple(MMString("short"), OmnipotentCharCXX, MConstInt(0)),
+  MConstInt(4));
 
   const Instruction *I = match(BB,
   MInstruction(Instruction::Store,
@@ -1047,13 +1044,10 @@
   MConstInt(0)),
 MConstInt(Compiler.PtrSize));
 
-  auto ClassDerived = MMTuple(
-MMString("_ZTS7Derived"),
-MMTuple(
-  MMString("short"),
-  OmnipotentCharCXX,
-  MConstInt(0)),
-MConstInt(Compiler.PtrSize + 4));
+  auto ClassDerived =
+  MMTuple(MMString("_ZTS7Derived"), ClassBase, MConstInt(0),
+  MMTuple(MMString("short"), OmnipotentCharCXX, MConstInt(0)),
+  MConstInt(Compiler.PtrSize + 4));
 
   const Instruction *I = match(BB,
   MInstruction(Instruction::Store,
Index: clang/test/CodeGen/tbaa-class.cpp
===
--- clang/test/CodeGen/tbaa-class.cpp
+++ clang/test/CodeGen/tbaa-class.cpp
@@ -222,7 +222,7 @@
 // OLD-PATH: [[TYPE_S]] = !{!"_ZTS7StructS", [[TYPE_SHORT]], i64 0, 
[[TYPE_INT]], i64 4}
 // OLD-PATH: [[TAG_S_f16]] = !{[[TYPE_S]], [[TYPE_SHORT]], i64 0}
 // OLD-PATH: [[TAG_S2_f32_2]] = !{[[TYPE_S2:!.*]], [[TYPE_INT]], i64 12}
-// OLD-PATH: [[TYPE_S2]] = !{!"_ZTS8StructS2", [[TYPE_SHORT]], i64 8, 
[[TYPE_INT]], i64 12}
+// OLD-PATH: [[TYPE_S2]] = !{!"_ZTS8StructS2", [[TYPE_S]], i64 0, 
[[TYPE_SHORT]], i64 8, [[TYPE_INT]], i64 12}
 // OLD-PATH: [[TAG_C_b_a_f32]] = !{[[TYPE_C:!.*]], [[TYPE_INT]], i64 12}
 // OLD-PATH: [[TYPE_C]] = !{!"_ZTS7StructC", [[TYPE_SHORT]], i64 0, 
[[TYPE_B]], i64 4, [[TYPE_INT]], i64 28}
 // OLD-PATH: [[TAG_D_b_a_f32]] = !{[[TYPE_D:!.*]], [[TYPE_INT]], i64 12}
@@ -244,7 +244,7 @@
 // NEW-PATH: [[TYPE_S]] = !{[[TYPE_CHAR]], i64 8, !"_ZTS7StructS", 
[[TYPE_SHORT]], i64 0, i64 2, [[TYPE_INT]], i64 4, i64 4}
 // NEW-PATH: [[TAG_S_f16]] = !{[[TYPE_S]], [[TYPE_SHORT]], i64 0, i64 2}
 // NEW-PATH: [[TAG_S2_f32_2]] = !{[[TYPE_S2:!.*]], [[TYPE_INT]], i64 12, i64 4}
-// NEW-PATH: [[TYPE_S2]] = !{[[TYPE_CHAR]], i64 16, !"_ZTS8StructS2", 
[[TYPE_SHORT]], i64 8, i64 2, [[TYPE_INT]], i64 12, i64 4}
+// NEW-PATH: [[TYPE_S2]] = !{[[TYPE_CHAR]], i64 16, !"_ZTS8StructS2", 
[[TYPE_S]], i64 0, i64 8, [[TYPE_SHORT]], i64 8, i64 2, [[TYPE_INT]], i64 12, 
i64 4}
 // NEW-PATH: [[TAG_C_b_a_f32]] = !{[[TYPE_C:!.*]], [[TYPE_INT]], i64 12, i64 4}
 // NEW-PATH: [[TYPE_C]] = !{[[TYPE_CHAR]], i64 32, !"_ZTS7StructC", 
[[TYPE_SHORT]], i64 0, i64 2, [[TYPE_B]], i64 4, i64 24, [[TYPE_INT]], i64 28, 
i64 4}
 // NEW-PATH: [[TAG_D_b_a_f32]] = !{[[TYPE_D:!.*]], [[TYPE_INT]], i64 12, i64 4}
Index: clang/lib/CodeGen/CodeGenTBAA.cpp
===
--- clang/lib/CodeGen/CodeGenTBAA.cpp
+++ clang/lib/CodeGen/CodeGenTBAA.cpp
@@ -336,6 +336,30 @@
 const RecordDecl *RD = TTy->getDecl()->getDefinition();
 const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
 SmallVector Fields;
+if (const CXXRecordDecl *CXXRD = dyn_cast(RD)) {
+  // Handle C++ base classes. Non-virtual bases can treated a a kind of
+  // field. Virtual bases are more complex and omitted, but avoid an
+  // incomplete view for NewStructPathTBAA.
+  if (CodeGenOpts.NewStructPathTBAA && CXXRD->getNumVBases() != 0)
+return BaseTypeMetadataCache[Ty] = nullptr;
+  for (const CXXBaseSpecifier &B : CXXRD->bases()) {
+if (B.isVirtual())
+  continue;
+QualType BaseQTy = B.getType();
+const CXXRecordDecl *BaseRD = BaseQTy->getAsCXXRecordDecl();
+if (BaseRD->isEmpty())
+  continue;
+llvm::MDNode *TypeNode = isValidBaseType(BaseQTy)
+ ? getBaseTypeInfo(BaseQTy)
+ : getTypeInfo(BaseQTy);
+   

[PATCH] D126291: [flang][Driver] Update link job on windows

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D126291#391 , @mmuetzel wrote:

> ISTR, I somewhere read that Windows isn't a supported host currently. Is this 
> no longer the case?)

Windows is supported: https://lab.llvm.org/buildbot/#/builders/172 :)

> If you could give some pointers to available instructions, I could 
> potentially try to set up an MSYS2 environment for this.

The only thing that's different to other sub-projects is the value of 
`LLVM_ENABLE_PROJECTS`, i.e. use `-DLLVM_ENABLE_PROJECTS=clang;mlir;flang;llvm` 
when running CMake. Keep in mind that Flang is slow to build and consumes lots 
of memory, sadly.

If we can't test on MinGW, then we could also just add a note in the docs.


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

https://reviews.llvm.org/D126291

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

[PATCH] D125919: Drop qualifiers from return types in C (DR423)

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D125919#3554599 , @rjmccall wrote:

> Ah, true, the standard doesn't describe it as a normal qualifier.  From the 
> DR, it sounds like the committee certainly expected that this reasoning would 
> also apply to `_Atomic`, even if that's not quite what they've written, but 
> that the DR submitter seems to not want that behavior.  Have you been able to 
> track down the solicited paper mentioned in that DR?  In the absence of 
> further indication, I think dropping `_Atomic` like anything else is the 
> right thing to do; like the other qualifiers, it isn't meaningful for 
> r-values.

I think it's less clear to me that the committee expected to drop `_Atomic` 
because the standard basically makes `_Atomic` introduce a new type rather than 
a qualified use like `const`; it's more akin to `_Complex` in that regard. 
e.g., it's not clear to me that these are redeclarations:

  int func(void);
  _Atomic(int) func(void);

When I survey non-Clang C compilers which support `_Atomic`, it's evenly split 
between thinking these are valid redeclarations or not (GCC and ICC say they're 
not valid redeclarations, ICC and TCC say they are) but when I use `const int` 
and `int` as the return types, only ICC claims they're incompatible types.

Further, this seems like a gratuitous incompatibility with C++ where the 
qualifiers are *not* dropped, given that function call expressions explicitly 
drop the qualifiers anyway. (So you're right, they are effectively useless on 
the return type, but why should C and C++ differ in what's a valid 
redeclaration?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125919

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

[PATCH] D125479: [pseudo] Fix the incorrect parameters-and-qualifiers rule.

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/pseudo/test/glr.cpp:37
+
+void foo2(int, ...);
+// CHECK:  declaration~simple-declaration := decl-specifier-seq 
init-declarator-list ;

hokein wrote:
> since we have the builtin pseudoCXX library now, we could use it to write a 
> cxx unittest, so that we can get rid of this cxx lit test. what do you think? 
> (I have a feeling we might add more in the future, I just discovered a new 
> oversight in our cxx.bnf grammar).
For detailed coverage of particular parts, t's not obvious to me whether a unit 
test or these tests will be more readable. (Consider writing a bunch of 
matchers and dealing with the messages from those, vs textual diffs here).



Comment at: clang-tools-extra/pseudo/test/glr.cpp:37
+
+void foo2(int, ...);
+// CHECK:  declaration~simple-declaration := decl-specifier-seq 
init-declarator-list ;

sammccall wrote:
> hokein wrote:
> > since we have the builtin pseudoCXX library now, we could use it to write a 
> > cxx unittest, so that we can get rid of this cxx lit test. what do you 
> > think? (I have a feeling we might add more in the future, I just discovered 
> > a new oversight in our cxx.bnf grammar).
> For detailed coverage of particular parts, t's not obvious to me whether a 
> unit test or these tests will be more readable. (Consider writing a bunch of 
> matchers and dealing with the messages from those, vs textual diffs here).
can we place this test in another file? (glr/varargs.cpp or so)
I missed this with `operator<` but that should probably be its own file too.

One of the things that feels painful with clang's lit tests is having to reduce 
a failure in a huge file with many related tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125479

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

[clang-tools-extra] 8df2b1a - [pp-trace] Print HashLoc in InclusionDirective callback

[via cfe-commits]

Author: CHIANG, YU-HSUN (Tommy Chiang, oToToT)
Date: 2022-06-03T19:29:59+08:00
New Revision: 8df2b1a866800b41984bd7721b244a9821810764

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

LOG: [pp-trace] Print HashLoc in InclusionDirective callback

The HashLoc in InclusionDirective callback is an unused parameter.
Since pp-trace is also used as a test of Clang’s PPCallbacks interface,
add it to the output of pp-trace could avoid some unintended change on
it.

This shuold resolves PR52673

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/pp-trace.rst
clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
clang-tools-extra/test/pp-trace/pp-trace-include.cpp

Removed: 




diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 7e8ef092fad1..b8321dd0c365 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -239,7 +239,7 @@ The improvements are...
 Improvements to pp-trace
 
 
-The improvements are...
+- Added `HashLoc` information to `InclusionDirective` callback output.
 
 Clang-tidy Visual Studio plugin
 ---

diff  --git a/clang-tools-extra/docs/pp-trace.rst 
b/clang-tools-extra/docs/pp-trace.rst
index 84a5ae6ed6c6..7339d8084c34 100644
--- a/clang-tools-extra/docs/pp-trace.rst
+++ b/clang-tools-extra/docs/pp-trace.rst
@@ -223,6 +223,7 @@ Imported ((module name)|(null)) 
  const Modu
 Example:::
 
   - Callback: InclusionDirective
+HashLoc: 
"D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:4:1"
 IncludeTok: include
 FileName: "Input/Level1B.h"
 IsAngled: false

diff  --git a/clang-tools-extra/pp-trace/PPCallbacksTracker.cpp 
b/clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
index 63a07914f207..c1434558698d 100644
--- a/clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
+++ b/clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
@@ -137,6 +137,7 @@ void PPCallbacksTracker::InclusionDirective(
 llvm::StringRef SearchPath, llvm::StringRef RelativePath,
 const Module *Imported, SrcMgr::CharacteristicKind FileType) {
   beginCallback("InclusionDirective");
+  appendArgument("HashLoc", HashLoc);
   appendArgument("IncludeTok", IncludeTok);
   appendFilePathArgument("FileName", FileName);
   appendArgument("IsAngled", IsAngled);

diff  --git a/clang-tools-extra/test/pp-trace/pp-trace-include.cpp 
b/clang-tools-extra/test/pp-trace/pp-trace-include.cpp
index d5578cf16f59..96b4014025b7 100644
--- a/clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ b/clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -51,6 +51,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (getFileEntryForID failed)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:3:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -65,6 +66,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -95,6 +97,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:4:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1B.h"
 // CHECK-NEXT:   IsAngled: false
@@ -109,6 +112,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1B.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2B.h"
 // CHECK-NEXT:   IsAngled: false



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

[PATCH] D125373: [pp-trace] Print HashLoc in InclusionDirective callback

[Tommy Chiang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8df2b1a86680: [pp-trace] Print HashLoc in InclusionDirective 
callback (authored by oToToT).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125373

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


Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -51,6 +51,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (getFileEntryForID failed)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:3:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -65,6 +66,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -95,6 +97,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:4:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1B.h"
 // CHECK-NEXT:   IsAngled: false
@@ -109,6 +112,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1B.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2B.h"
 // CHECK-NEXT:   IsAngled: false
Index: clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
===
--- clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
+++ clang-tools-extra/pp-trace/PPCallbacksTracker.cpp
@@ -137,6 +137,7 @@
 llvm::StringRef SearchPath, llvm::StringRef RelativePath,
 const Module *Imported, SrcMgr::CharacteristicKind FileType) {
   beginCallback("InclusionDirective");
+  appendArgument("HashLoc", HashLoc);
   appendArgument("IncludeTok", IncludeTok);
   appendFilePathArgument("FileName", FileName);
   appendArgument("IsAngled", IsAngled);
Index: clang-tools-extra/docs/pp-trace.rst
===
--- clang-tools-extra/docs/pp-trace.rst
+++ clang-tools-extra/docs/pp-trace.rst
@@ -223,6 +223,7 @@
 Example:::
 
   - Callback: InclusionDirective
+HashLoc: 
"D:/Clang/llvmnewmod/clang-tools-extra/test/pp-trace/pp-trace-include.cpp:4:1"
 IncludeTok: include
 FileName: "Input/Level1B.h"
 IsAngled: false
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -239,7 +239,7 @@
 Improvements to pp-trace
 
 
-The improvements are...
+- Added `HashLoc` information to `InclusionDirective` callback output.
 
 Clang-tidy Visual Studio plugin
 ---


Index: clang-tools-extra/test/pp-trace/pp-trace-include.cpp
===
--- clang-tools-extra/test/pp-trace/pp-trace-include.cpp
+++ clang-tools-extra/test/pp-trace/pp-trace-include.cpp
@@ -51,6 +51,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (getFileEntryForID failed)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:3:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -65,6 +66,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Level1A.h:1:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Level2A.h"
 // CHECK-NEXT:   IsAngled: false
@@ -95,6 +97,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: "{{.*}}{{[/\\]}}Inputs/Level1A.h"
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}pp-trace-include.cpp:4:1"
 // CHECK-NEXT:   IncludeTok: include
 // CHECK-NEXT:   FileName: "Inputs/Level1B.h"
 // CHECK-NEXT:   IsAngled: false
@@ -109,6 +112,7 @@
 // CHECK-NEXT:   FileType: C_User
 // CHECK-NEXT:   PrevFID: (invalid)
 // CHECK-NEXT: - Callback: InclusionDirective
+// CHECK-NEXT:   HashLoc: "{{.*}}{{[/\\]}}Inputs/Lev

[PATCH] D112916: Confusable identifiers detection

[Nico Weber via Phabricator via cfe-commits]

thakis added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/ConfusableTable/build_confusable_table.cpp:55
+  std::sort(Entries.begin(), Entries.end());
+  errs() << "Parsed " << Entries.size() << " Entries\n";
+

Could we not print this line? The build is supposed to be silent if nothing's 
broken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112916

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

[clang-tools-extra] 180bae0 - [gn build] (manually) port b94db7ed7eaf (Confusables.inc)

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2022-06-03T07:49:28-04:00
New Revision: 180bae08a04d4dc724cb5e6f2ea9df8641a3f657

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

LOG: [gn build] (manually) port b94db7ed7eaf (Confusables.inc)

Added: 

llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn

Modified: 
clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/BUILD.gn

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
index 5d6be0c8548fa..05eecddd18603 100644
--- a/clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/misc/ConfusableTable/CMakeLists.txt
@@ -1 +1,3 @@
-add_llvm_executable(make_confusable_table build_confusable_table.cpp)
+add_llvm_executable(make_confusable_table
+  build_confusable_table.cpp
+  )

diff  --git 
a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/BUILD.gn 
b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/BUILD.gn
index 5463d61e5ff84..a99b254a1b547 100644
--- a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/BUILD.gn
+++ b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/BUILD.gn
@@ -1,7 +1,26 @@
+action("Confusables.inc") {
+  gen_target = "ConfusableTable:make_confusable_table($host_toolchain)"
+  gen_executable = get_label_info(gen_target, "root_out_dir") +
+"/bin/" + get_label_info(gen_target, "name")
+  deps = [ gen_target ]
+
+  # FIXME: Rename this script, now that it's used for other things.
+  script = "//llvm/utils/gn/build/run_tablegen.py"
+  sources = [ "ConfusableTable/confusables.txt" ]
+  outputs = [ "$target_gen_dir/$target_name" ]
+  args = [
+rebase_path(gen_executable, root_build_dir),
+rebase_path(sources[0], root_build_dir),
+rebase_path(outputs[0], root_build_dir),
+  ]
+}
+
 static_library("misc") {
   output_name = "clangTidyMiscModule"
   configs += [ "//llvm/utils/gn/build:clang_code" ]
+  include_dirs = [ target_gen_dir ]
   deps = [
+":Confusables.inc",
 "//clang-tools-extra/clang-tidy",
 "//clang-tools-extra/clang-tidy/utils",
 "//clang/lib/AST",
@@ -15,6 +34,7 @@ static_library("misc") {
   ]
   sources = [
 "DefinitionsInHeadersCheck.cpp",
+"Homoglyph.cpp",
 "MiscTidyModule.cpp",
 "MisleadingBidirectional.cpp",
 "MisleadingIdentifier.cpp",

diff  --git 
a/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
 
b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
new file mode 100644
index 0..e07cd98f824ad
--- /dev/null
+++ 
b/llvm/utils/gn/secondary/clang-tools-extra/clang-tidy/misc/ConfusableTable/BUILD.gn
@@ -0,0 +1,4 @@
+executable("make_confusable_table") {
+  deps = [ "//llvm/lib/Support" ]
+  sources = [ "build_confusable_table.cpp" ]
+}



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

[PATCH] D126759: [clang][dataflow] Model calls returning optionals

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev updated this revision to Diff 434000.
sgatev added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126759

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

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2150,6 +2150,70 @@
   UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+$ns::$optional MakeOpt();
+
+void target() {
+  $ns::$optional opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-1]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:9:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+const $ns::$optional& MakeOpt();
+
+void target() {
+  $ns::$optional opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-2]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-2", "unsafe: input.cc:9:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using IntOpt = $ns::$optional;
+IntOpt MakeOpt();
+
+void target() {
+  IntOpt opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-3]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:10:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using IntOpt = $ns::$optional;
+const IntOpt& MakeOpt();
+
+void target() {
+  IntOpt opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-4]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
+}
+
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -45,6 +45,11 @@
 
 auto hasOptionalType() { return hasType(optionalClass()); }
 
+auto hasOptionalOrAliasType() {
+  return hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(optionalClass(;
+}
+
 auto isOptionalMemberCallWithName(
 llvm::StringRef MemberName,
 llvm::Optional Ignorable = llvm::None) {
@@ -158,6 +163,12 @@
ComparesToSame(integerLiteral(equals(0);
 }
 
+auto isCallReturningOptional() {
+  return callExpr(callee(functionDecl(
+  returns(anyOf(hasOptionalOrAliasType(),
+referenceType(pointee(hasOptionalOrAliasType(;
+}
+
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
 /// symbolic value of its "has_value" property.
 StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
@@ -322,6 +333,18 @@
   });
 }
 
+void transferCallReturningOptional(const CallExpr *E,
+   const MatchFinder::MatchResult &Result,
+   LatticeTransferState &State) {
+  if (State.Env.getStorageLocation(*E, SkipPast::None) != nullptr)
+return;
+
+  auto &Loc = State.Env.createStorageLocation(*E);
+  State.Env.setStorageLocation(*E, Loc);
+  State.Env.setValue(
+  Loc, createOptionalValue(State.Env, State.Env.makeAtomicBoolValue()));
+}
+
 void assignOptionalValue(const Expr &E, LatticeTransferState &State,
  BoolValue &HasValueVal) {
   if (auto *OptionalLoc =
@@ -547,6 +570,10 @@
   // opt.value_or(X) != X
   .CaseOf(isValueOrNotEqX(), transferValueOrNotEqX)
 
+  // returns optional
+  .CaseOf(isCallReturningOptional(),
+transferCallReturningOptional)
+
   .Build();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 166f9be - Update old mailing list link in the nullability doc

[Hans Wennborg via cfe-commits]

Author: Hans Wennborg
Date: 2022-06-03T14:23:41+02:00
New Revision: 166f9be330dd36e2ef27d4c0023b78b8257f0909

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

LOG: Update old mailing list link in the nullability doc

Added: 


Modified: 
clang/docs/analyzer/developer-docs/nullability.rst

Removed: 




diff  --git a/clang/docs/analyzer/developer-docs/nullability.rst 
b/clang/docs/analyzer/developer-docs/nullability.rst
index d54a2e9369c7a..f4be0b57719ee 100644
--- a/clang/docs/analyzer/developer-docs/nullability.rst
+++ b/clang/docs/analyzer/developer-docs/nullability.rst
@@ -4,7 +4,8 @@ Nullability Checks
 
 This document is a high level description of the nullablility checks.
 These checks intended to use the annotations that is described in this
-RFC: http://lists.cs.uiuc.edu/pipermail/cfe-dev/2015-March/041798.html.
+RFC: https://discourse.llvm.org/t/rfc-nullability-qualifiers/35672
+(`Mailman `_)
 
 Let's consider the following 2 categories:
 



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

[PATCH] D126189: [C++20][Modules] Build module static initializers per P1874R1.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 434003.
iains edited the summary of this revision.
iains added a comment.

rebased and updated

previously, this was not doing the right thing for module implementation units
which were being processed as if they were interfaces, so we've introduced a 
module
implemetation type, and that is now checked for in the emitting of the inits.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126189

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/Parse/ParseAST.cpp
  clang/test/CodeGen/module-intializer-pmf.cpp
  clang/test/CodeGen/module-intializer.cpp

Index: clang/test/CodeGen/module-intializer.cpp
===
--- /dev/null
+++ clang/test/CodeGen/module-intializer.cpp
@@ -0,0 +1,186 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.cpp \
+// RUN:-emit-module-interface -o N.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 N.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-N
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.cpp \
+// RUN:-emit-module-interface -o O.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 O.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-O
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.cpp \
+// RUN:-emit-module-interface -o M-part.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-part.pcm -S \
+// RUN: -emit-llvm -o - | FileCheck %s --check-prefix=CHECK-P
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.cpp \
+// RUN: -fmodule-file=N.pcm -fmodule-file=O.pcm -fmodule-file=M-part.pcm \
+// RUN:-emit-module-interface -o M.pcm
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M.pcm -S -emit-llvm \
+// RUN:  -o - | FileCheck %s --check-prefix=CHECK-M
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 useM.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-USE
+
+// RUN: %clang_cc1 -triple %itanium_abi_triple -std=c++20 M-impl.cpp \
+// RUN: -fmodule-file=M.pcm -S -emit-llvm  -o - \
+// RUN: | FileCheck %s --check-prefix=CHECK-IMPL
+
+//--- N-h.h
+
+struct Oink {
+  Oink(){};
+};
+
+Oink Hog;
+
+//--- N.cpp
+
+module;
+#include "N-h.h"
+
+export module N;
+
+export struct Quack {
+  Quack(){};
+};
+
+export Quack Duck;
+
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZN4OinkC1Ev
+// CHECK-N: define internal void @__cxx_global_var_init
+// CHECK-N: call void @_ZNW1N5QuackC1Ev
+// CHECK-N: define void @_ZGIW1N
+// CHECK-N: store i8 1, ptr @_ZGIW1N__in_chrg
+// CHECK-N: call void @__cxx_global_var_init
+// CHECK-N: call void @__cxx_global_var_init
+
+//--- O-h.h
+
+struct Meow {
+  Meow(){};
+};
+
+Meow Cat;
+
+//--- O.cpp
+
+module;
+#include "O-h.h"
+
+export module O;
+
+export struct Bark {
+  Bark(){};
+};
+
+export Bark Dog;
+
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZN4MeowC2Ev
+// CHECK-O: define internal void @__cxx_global_var_init
+// CHECK-O: call void @_ZNW1O4BarkC1Ev
+// CHECK-O: define void @_ZGIW1O
+// CHECK-O: store i8 1, ptr @_ZGIW1O__in_chrg
+// CHECK-O: call void @__cxx_global_var_init
+// CHECK-O: call void @__cxx_global_var_init
+
+//--- P-h.h
+
+struct Croak {
+  Croak(){};
+};
+
+Croak Frog;
+
+//--- M-part.cpp
+
+module;
+#include "P-h.h"
+
+module M:Part;
+
+struct Squawk {
+  Squawk(){};
+};
+
+Squawk parrot;
+
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZN5CroakC1Ev
+// CHECK-P: define internal void @__cxx_global_var_init
+// CHECK-P: call void @_ZNW1M6SquawkC1Ev
+// CHECK-P: define void @_ZGIW1MWP4Part
+// CHECK-P: store i8 1, ptr @_ZGIW1MWP4Part__in_chrg
+// CHECK-P: call void @__cxx_global_var_init
+// CHECK-P: call void @__cxx_global_var_init
+
+//--- M-h.h
+
+struct Moo {
+  Moo(){};
+};
+
+Moo Cow;
+
+//--- M.cpp
+
+module;
+#include "M-h.h"
+
+export module M;
+import N;
+export import O;
+import :Part;
+
+export struct Baa {
+  int x;
+  Baa(){};
+  Baa(int x) : x(x) {}
+  int getX() { return x; }
+};
+
+export Baa Sheep(10);
+
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZN3MooC1Ev
+// CHECK-M: define internal void @__cxx_global_var_init
+// CHECK-M: call void @_ZNW1M3BaaC1Ei
+// CHECK-M: declare void @_ZGIW1O()
+// CHECK-M: declare void @_ZGIW1N()
+// CHECK-M: declare void @_ZGIW1MWP4Part()
+// CHECK-M: define void @_ZGIW1M
+// CHECK-M: store i8 1, ptr @_ZGIW1M__in_chrg
+// CHECK-M: call void @_ZGIW1O()
+// CHECK-M: call void @_ZGIW1N()
+// CHECK-M: call void @

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

[Iain Sandoe via Phabricator via cfe-commits]

iains created this revision.
Herald added a project: All.
iains added reviewers: urnathan, ChuanqiXu.
iains added a subscriber: clang-modules.
iains published this revision for review.
iains added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

we need to distinguish implementation and interface in code-gen.


During code-gen (for initializers) we need to process module implementation
units differently from interfaces.  At present, this is not possible since
the interface module is loaded as representing the "current module scope".

We cannot treat an implementation unit as if it was a regular source, since we
need to track module purview and associate definitions with the module.

The solution here is to create a module type for the implementation which
imports the interface per C++20:
[module.unit/8] 'A module-declaration that contains neither an export-keyword
nor a module-partition implicitly imports the primary module interface unit of
the module as if by a module-import-declaration.

Implementation modules are never serialized (-emit-module-interface for an
implementation unit is diagnosed and rejected).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126959

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Lex/ModuleMap.h
  clang/lib/AST/Decl.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaModule.cpp

Index: clang/lib/Sema/SemaModule.cpp
===
--- clang/lib/Sema/SemaModule.cpp
+++ clang/lib/Sema/SemaModule.cpp
@@ -256,6 +256,7 @@
   auto &Map = PP.getHeaderSearchInfo().getModuleMap();
   Module *Mod;
 
+  ImportDecl *Import = nullptr;
   switch (MDK) {
   case ModuleDeclKind::Interface:
   case ModuleDeclKind::PartitionInterface: {
@@ -288,6 +289,8 @@
 // keyword nor a module-partition implicitly imports the primary
 // module interface unit of the module as if by a module-import-
 // declaration.
+
+// First find the interface we need to import.
 Mod = getModuleLoader().loadModule(ModuleLoc, {ModuleNameLoc},
Module::AllVisible,
/*IsInclusionDirective=*/false);
@@ -296,6 +299,13 @@
   // Create an empty module interface unit for error recovery.
   Mod = Map.createModuleForInterfaceUnit(ModuleLoc, ModuleName,
  GlobalModuleFragment);
+} else {
+  // Make the decl.
+  Import = ImportDecl::Create(Context, CurContext, ModuleLoc, Mod,
+  Path[0].second);
+  // now create the implementation module
+  Mod = Map.createModuleForImplementationUnit(ModuleLoc, ModuleName,
+  GlobalModuleFragment);
 }
   } break;
 
@@ -335,8 +345,16 @@
   // statements, so imports are allowed.
   ImportState = ModuleImportState::ImportAllowed;
 
+  // We already potentially made an implicit import (in the case of a module
+  // implementation unit importing its interface).  Make this module visible
+  // and return the import decl to be added to the current TU.
+  if (Import)
+VisibleModules.setVisible(Import->getImportedModule(), ModuleLoc);
+
   // FIXME: Create a ModuleDecl.
-  return nullptr;
+  // If we made an implicit import of the module interface, then return the
+  // imported module decl.
+  return Import ? ConvertDeclToDeclGroup(Import) : nullptr;
 }
 
 Sema::DeclGroupPtrTy
@@ -360,19 +378,17 @@
 Diag(ModuleScopes.back().BeginLoc, diag::note_previous_definition);
 return nullptr;
 
-  case Module::ModuleInterfaceUnit:
-break;
-  }
-
-  if (!ModuleScopes.back().ModuleInterface) {
+  case Module::ModuleImplementationUnit:
 Diag(PrivateLoc, diag::err_private_module_fragment_not_module_interface);
 Diag(ModuleScopes.back().BeginLoc,
  diag::note_not_module_interface_add_export)
 << FixItHint::CreateInsertion(ModuleScopes.back().BeginLoc, "export ");
 return nullptr;
+
+  case Module::ModuleInterfaceUnit:
+break;
   }
 
-  // FIXME: Check this isn't a module interface partition.
   // FIXME: Check that this translation unit does not import any partitions;
   // such imports would violate [basic.link]/2's "shall be the only module unit"
   // restriction.
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -1641,6 +1641,13 @@
   if (NewM == OldM)
 return false;
 
+  // A module implementation unit has visibility of the decls in its implicitly
+  // imported interface.
+  if (getLangOpts().CPlusPlusModules && NewM && OldM &&
+  NewM->Kind == Module::ModuleImplementationUnit &&
+  OldM->Kind == Module::ModuleInterfaceUnit)
+return NewM->Name == OldM->Name;
+
   bool NewIsModuleInterface =

[clang] 3472b6e - Updating more entries in the C DR Status page

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-06-03T08:29:06-04:00
New Revision: 3472b6eb0a70f6b3ae45078d79d1c5b350da9c24

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

LOG: Updating more entries in the C DR Status page

Adds test coverage or information for ~25 more C DRs.

Added: 


Modified: 
clang/test/C/drs/dr1xx.c
clang/www/c_dr_status.html

Removed: 




diff  --git a/clang/test/C/drs/dr1xx.c b/clang/test/C/drs/dr1xx.c
index 58c4386b3fe3b..10497f90c2db9 100644
--- a/clang/test/C/drs/dr1xx.c
+++ b/clang/test/C/drs/dr1xx.c
@@ -13,6 +13,24 @@
  *
  * WG14 DR104: dup 084
  * Incomplete tag types in a parameter list
+ *
+ * WG14 DR109: yes
+ * Are undefined values and undefined behavior the same?
+ *
+ * WG14 DR110: dup 047
+ * Formal parameters having array-of-non-object types
+ *
+ * WG14 DR117: yes
+ * Abstract semantics, sequence points, and expression evaluation
+ *
+ * WG14 DR121: yes
+ * Conversions of pointer values to integral types
+ *
+ * WG14 DR122: dup 015
+ * Conversion/widening of bit-fields
+ *
+ * WG14 DR125: yes
+ * Using things declared as 'extern (qualified) void'
  */
 
 
@@ -71,3 +89,137 @@ void dr105(void) {
   extern int i;   /* expected-note {{previous declaration is here}} */
   extern float i; /* expected-error {{redeclaration of 'i' with a 
diff erent type: 'float' vs 'int'}} */
 }
+
+/* WG14 DR106: yes
+ * When can you dereference a void pointer?
+ *
+ * NB: This is a partial duplicate of DR012.
+ */
+void dr106(void *p, int i) {
+  /* The behavior changed between C89 and C99. */
+  (void)&*p; /* c89only-warning {{ISO C forbids taking the address of an 
expression of type 'void'}} */
+  /* The behavior of all three of these is undefined. */
+  (void)*p;
+  (void)(i ? *p : *p);
+  (void)(*p, *p); /* expected-warning {{left operand of comma operator has no 
effect}} */
+}
+
+/* WG14 DR108: yes
+ * Can a macro identifier hide a keyword?
+ */
+void dr108(void) {
+#define const
+  const int i = 12;
+#undef const
+  const int j = 12; /* expected-note {{variable 'j' declared const here}} */
+
+  i = 100; /* Okay, the keyword was hidden by the macro. */
+  j = 100; /* expected-error {{cannot assign to variable 'j' with 
const-qualified type 'const int'}} */
+}
+
+/* WG14 DR111: yes
+ * Conversion of pointer-to-qualified type values to type (void*) values
+ */
+void dr111(const char *ccp, void *vp) {
+  vp = ccp; /* expected-warning {{assigning to 'void *' from 'const char *' 
discards qualifiers}} */
+}
+
+/* WG14 DR112: yes
+ * Null pointer constants and relational comparisons
+ */
+void dr112(void *vp) {
+  /* The behavior of this expression is pedantically undefined.
+   * FIXME: should we diagnose under -pedantic?
+   */
+  (void)(vp > (void*)0);
+}
+
+/* WG14 DR113: yes
+ * Return expressions in functions declared to return qualified void
+ */
+volatile void dr113_v(volatile void *vvp) { /* expected-warning {{function 
cannot return qualified void type 'volatile void'}} */
+  return *vvp; /* expected-warning {{void function 'dr113_v' should not return 
void expression}} */
+}
+const void dr113_c(const void *cvp) { /* expected-warning {{function cannot 
return qualified void type 'const void'}} */
+  return *cvp; /* expected-warning {{void function 'dr113_c' should not return 
void expression}} */
+}
+
+/* WG14 DR114: yes
+ * Initialization of multi-dimensional char array objects
+ */
+void dr114(void) {
+  char array[2][5] = { "defghi" }; /* expected-warning {{initializer-string 
for char array is too long}} */
+}
+
+/* WG14 DR115: yes
+ * Member declarators as declarators
+ */
+void dr115(void) {
+  struct { int mbr; }; /* expected-warning {{declaration does not declare 
anything}} */
+  union { int mbr; };  /* expected-warning {{declaration does not declare 
anything}} */
+}
+
+/* WG14 DR116: yes
+ * Implicit unary & applied to register arrays
+ */
+void dr116(void) {
+  register int array[5] = { 0, 1, 2, 3, 4 };
+  (void)array;   /* expected-error {{address of register variable 
requested}} */
+  (void)array[3];/* expected-error {{address of register variable 
requested}} */
+  (void)(array + 3); /* expected-error {{address of register variable 
requested}} */
+}
+
+/* WG14 DR118: yes
+ * Completion point for enumerated types
+ */
+void dr118(void) {
+  enum E {
+   /* The enum isn't a complete type until the closing }, but an
+* implementation may complete the type earlier if it has sufficient 
type
+* information to calculate size or alignment, etc.
+*/
+Val = sizeof(enum E)
+  };
+}
+
+/* WG14 DR119: yes
+ * Initialization of multi-dimensional array objects
+ */
+void dr119(void) {
+  static int array[][] = { { 1, 2, 3 }, { 4, 5, 6 }, { 7, 8, 9 } }; /* 
expected-error {{array has incomplete element type 'int[]'

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

Luca,

Can you expand the description to give a better intuition as to what this check 
is finding that the diagnostic does not? The example is good, but it would be 
helpful if you could phrase the general rule being enforced.  Similarly, that 
would be helpful in the documentation file -- before diving into the formal 
specification, explain what the intuition. For that matter, why does this merit 
a separate clang-tidy vs. integeration into the existing code for the 
diagnostic (or a similar, parallel one)? I'm not advocating one way or another, 
I'd just like to understand your decision.

Separately, can you provide some data as to how much of a problem this is in 
practice? The code here is quite complex and I think that sets a high bar (in 
terms of benefit to users) for committing it.

Thanks!




Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h:52-55
+  llvm::SmallVector TypesByBase;
+  llvm::SmallVector RawTypes;
+  llvm::SmallVector TemplateTypes;
+  llvm::SmallVector SmartPointerTypes;

Please add comments explaining the roles of these fields (either individually 
or collectively)



Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h:56
+  llvm::SmallVector SmartPointerTypes;
+  // Map of function name to bitmask of arguments that are not read from.
+  llvm::StringMap> FunctionAllowList;

can you expand or rephrase? is the idea that these are argument positions that 
should be ignored for the purposes of this check? If so, what does the name of 
the field mean?



Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h:58-59
+  llvm::StringMap> FunctionAllowList;
+  // TODO(veluca): The FixIts don't really fix everything and can break code.
+  static constexpr bool EmitFixits = false;
+};

Might this be better in the `.cpp` file?



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:116
`bugprone-virtual-near-miss `_, "Yes"
-   `cert-dcl21-cpp `_, "Yes"
+   `cert-dcl21-cpp `_,
`cert-dcl50-cpp `_,

why the change to this line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D126944: [Clang] Fix memory leak due to TemplateArgumentListInfo used in AST node.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/AST/DeclTemplate.h:2776
   void setTemplateArgsInfo(const TemplateArgumentListInfo &ArgsInfo);
+  void setTemplateArgsInfo(const ASTTemplateArgumentListInfo *ArgsInfo);
 

Can we make it a prerequisite that the argument has to be nonnull and thus we 
can pass in a const reference instead?



Comment at: clang/include/clang/AST/DeclTemplate.h:2778-2779
 
-  const TemplateArgumentListInfo &getTemplateArgsInfo() const {
+  const ASTTemplateArgumentListInfo *getTemplateArgsInfo() const {
 return TemplateArgsInfo;
   }

Do we want to assert that `TemplateArgsInfo` is nonnull and return a const 
reference instead?

Basically, I'm wondering if we can remove a bunch of the pointers and go with 
references as much as possible; it seems to me that a null pointer is a sign of 
an error, same as with `TemplateArgs`.



Comment at: clang/lib/AST/ASTImporter.cpp:6021
 TemplateArgumentListInfo ToTAInfo;
-if (Error Err = ImportTemplateArgumentListInfo(
-D->getTemplateArgsInfo(), ToTAInfo))
-  return std::move(Err);
+if (auto *Args = D->getTemplateArgsInfo()) {
+  if (Error Err = ImportTemplateArgumentListInfo(

You should spell out the type name here instead of using `auto` (per coding 
standard).



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:5560
+TemplateArgumentListInfo TemplateArgInfo;
+if (auto *ArgInfo = VarSpec->getTemplateArgsInfo()) {
+  TemplateArgInfo.setLAngleLoc(ArgInfo->getLAngleLoc());

You should spell out the type name here as well.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:5563-5565
+  for (const TemplateArgumentLoc& arg : ArgInfo->arguments()) {
+TemplateArgInfo.addArgument(arg);
+  }

More coding standard changes -- btw, it looks like you need to run the patch 
through clang-format 
(https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126944

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

[PATCH] D126937: Fix memleak in VarTemplateSpecializationDecl

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D126937#3555243 , @browneee wrote:

> I was also looking into fixing this: https://reviews.llvm.org/D126944
>
> I'm not yet sure if my changes are correct.

I did a review on https://reviews.llvm.org/D126944, but I'll let the two of you 
decide which review to move forward with rather than review them both in 
tandem. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126937

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

[PATCH] D126960: [clang][sema] Unary not boolean to int conversion

[Ashley Roll via Phabricator via cfe-commits]

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

I have modifyed GetExprRange() to capture the promotion
of boolean values to ints when applying unary not to ensure
-Wsign-compare identifies unsigned/signed comparisons as
identified in:

https://github.com/llvm/llvm-project/issues/18878


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126960

Files:
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/compare.c


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -419,3 +419,25 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int warn_on_different_sign_after_unary_operator(unsigned a, int b) {
+  return
+  // unary not promotes boolean to int
+  (a > ~(!b)) // expected-warning {{comparison of integers of different 
signs: 'unsigned int' and 'int'}}
+  &&
+  (a > -(b)) // expected-warning {{comparison of integers of different 
signs: 'unsigned int' and 'int'}}
+  &&
+  (a > ++b) // expected-warning {{comparison of integers of different 
signs: 'unsigned int' and 'int'}}
+  &&
+  (a > --b) // expected-warning {{comparison of integers of different 
signs: 'unsigned int' and 'int'}}
+  &&
+  // unary not promotes boolean to int
+  (b > ~(!a)) // no warning
+  &&
+  (b > -(a)) // expected-warning {{comparison of integers of different 
signs: 'int' and 'unsigned int'}}
+  &&
+  (b > ++a) // expected-warning {{comparison of integers of different 
signs: 'int' and 'unsigned int'}}
+  &&
+  (b > --a) // expected-warning {{comparison of integers of different 
signs: 'int' and 'unsigned int'}}
+  ;
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12326,6 +12326,13 @@
 case UO_AddrOf: // should be impossible
   return IntRange::forValueOfType(C, GetExprType(E));
 
+case UO_Not:
+  // unary not promotes boolean to integer
+  if (UO->getSubExpr()->isKnownToHaveBooleanValue())
+return IntRange(MaxWidth, false);
+
+  LLVM_FALLTHROUGH;
+
 default:
   return GetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
   Approximate);


Index: clang/test/Sema/compare.c
===
--- clang/test/Sema/compare.c
+++ clang/test/Sema/compare.c
@@ -419,3 +419,25 @@
   if (x == y) x = y; // no warning
   if (y == x) y = x; // no warning
 }
+
+int warn_on_different_sign_after_unary_operator(unsigned a, int b) {
+  return
+  // unary not promotes boolean to int
+  (a > ~(!b)) // expected-warning {{comparison of integers of different signs: 'unsigned int' and 'int'}}
+  &&
+  (a > -(b)) // expected-warning {{comparison of integers of different signs: 'unsigned int' and 'int'}}
+  &&
+  (a > ++b) // expected-warning {{comparison of integers of different signs: 'unsigned int' and 'int'}}
+  &&
+  (a > --b) // expected-warning {{comparison of integers of different signs: 'unsigned int' and 'int'}}
+  &&
+  // unary not promotes boolean to int
+  (b > ~(!a)) // no warning
+  &&
+  (b > -(a)) // expected-warning {{comparison of integers of different signs: 'int' and 'unsigned int'}}
+  &&
+  (b > ++a) // expected-warning {{comparison of integers of different signs: 'int' and 'unsigned int'}}
+  &&
+  (b > --a) // expected-warning {{comparison of integers of different signs: 'int' and 'unsigned int'}}
+  ;
+}
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -12326,6 +12326,13 @@
 case UO_AddrOf: // should be impossible
   return IntRange::forValueOfType(C, GetExprType(E));
 
+case UO_Not:
+  // unary not promotes boolean to integer
+  if (UO->getSubExpr()->isKnownToHaveBooleanValue())
+return IntRange(MaxWidth, false);
+
+  LLVM_FALLTHROUGH;
+
 default:
   return GetExprRange(C, UO->getSubExpr(), MaxWidth, InConstantContext,
   Approximate);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D126818#3554617 , @tahonermann 
wrote:

> I wonder if I'm reading (temp.friend)p9` sentence 2 
>  correctly. Which of these 
> should it be parsed as?
>
> 1. A (friend function template with a constraint) that depends on a template 
> parameter from an enclosing template shall be a definition.
> 2. A friend function template (with a constraint that depends on a template 
> parameter from an enclosing template) shall be a definition.
>
> I'm guessing that the first interpretation is the intent.

I believe it is the 2nd one actually.  A friend function with a constraint, 
where said constraint depends on a template param from an enclosing template, 
shall be a definition.  And only THOSE are protected against being a 
'duplicate' of another in a different scope. I essentially implemented the 
former in this (as it is a superset of the cases) for the purposes of mangling, 
but I think that was the wrong idea. The result would be that when the 
constraint does NOT depend on a enclosing template, we end up emitting 
different names for the same function in different TUs (potentially).  
Initially I thought that was fine, but it ends up with extra duplication.

Note we might be confused, the parens there aren't completely clear as to what 
your intent is.


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

https://reviews.llvm.org/D126818

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

[clang] 1896df1 - Correct the behavior of this test for non-Windows targets

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-06-03T09:00:05-04:00
New Revision: 1896df18cc5b588760f75cc2c21d64c772cf0e4c

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

LOG: Correct the behavior of this test for non-Windows targets

This should address build failures like:
https://lab.llvm.org/buildbot/#/builders/188/builds/14980
https://lab.llvm.org/buildbot/#/builders/171/builds/15515
https://lab.llvm.org/buildbot/#/builders/91/builds/9877

Added: 


Modified: 
clang/test/C/drs/dr1xx.c

Removed: 




diff  --git a/clang/test/C/drs/dr1xx.c b/clang/test/C/drs/dr1xx.c
index 10497f90c2db..e7343c0e3573 100644
--- a/clang/test/C/drs/dr1xx.c
+++ b/clang/test/C/drs/dr1xx.c
@@ -177,8 +177,16 @@ void dr118(void) {
/* The enum isn't a complete type until the closing }, but an
 * implementation may complete the type earlier if it has sufficient 
type
 * information to calculate size or alignment, etc.
+*
+* On Microsoft targets, an enum is always implicit int sized, so the 
type
+* is sufficiently complete there. On other platforms, it is an 
incomplete
+* type at this point.
 */
 Val = sizeof(enum E)
+#ifndef _WIN32
+/* expected-error@-2 {{invalid application of 'sizeof' to an incomplete 
type 'enum E'}} */
+/* expected-note@-12 {{definition of 'enum E' is not complete until the 
closing '}'}} */
+#endif
   };
 }
 



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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Markus Mützel via Phabricator via cfe-commits]

mmuetzel added a comment.

I checked out the LLVM repository from https://github.com/llvm/llvm-project.git 
and applied your patch with `patch -Np0 -i D126291.433988.patch`.

After some failing attempts, I finally found a configuration for which building 
succeeded. I struggled with duplicate symbols and two many symbols when linking 
some libraries.
I ended up using these switches:

  cmake \
-Sllvm \
-Bbuild \
-GNinja \
-DCMAKE_INSTALL_PREFIX=pkg \
-DCMAKE_C_COMPILER=clang \
-DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_C_COMPILER_LAUNCHER=ccache \
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
-DLLVM_ENABLE_PROJECTS="clang;mlir;flang;llvm" \
-DLLVM_TARGETS_TO_BUILD="X86" \
-DCMAKE_BUILD_TYPE=Release \
-DLLVM_ENABLE_LIBCXX=ON \
-DCLANG_DEFAULT_RTLIB=compiler-rt \
-DCLANG_DEFAULT_UNWINDLIB=libunwind \
-DCLANG_DEFAULT_LINKER=lld \
-DLLVM_BUILD_LLVM_DYLIB=OFF \
-DLLVM_BUILD_STATIC=OFF \
-DLLVM_ENABLE_ASSERTIONS=OFF \
-DLLVM_ENABLE_FFI=ON \
-DLLVM_ENABLE_THREADS=ON \
-DLLVM_INCLUDE_EXAMPLES=OFF \
-DLLVM_INSTALL_UTILS=ON

I hope those make sense.

With those, `flang-new` was built in a CLANG64 build environment of MSYS2. 
After installation, I got:

  $ PATH=./pkg/bin:$PATH flang-new --version
  flang-new version 15.0.0
  Target: x86_64-w64-windows-gnu
  Thread model: posix
  InstalledDir: D:/llvm-project/pkg/bin

However, when trying to build a simple "Hello World" program with it, I got:

  $ PATH=./pkg/bin:$PATH flang-new hello.f90 -L/clang64/lib
  ld.lld: error: could not open 
'D:/llvm-project/pkg/lib/clang/15.0.0/lib/windows/libclang_rt.builtins-x86_64.a':
 No such file or directory
  ld.lld: error: could not open 
'D:/llvm-project/pkg/lib/clang/15.0.0/lib/windows/libclang_rt.builtins-x86_64.a':
 No such file or directory
  flang-new: error: linker command failed with exit code 1 (use -v to see 
invocation)

Those files don't exist indeed.
Is that expected? Did I do something wrong? Incorrect configuration? Do I also 
need to enable `compiler-rt`?


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

https://reviews.llvm.org/D126291

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Diana Picus via Phabricator via cfe-commits]

rovka added a comment.

@mmuetzel Thanks for looking into this! I think since you're passing 
`-DCLANG_DEFAULT_RTLIB=compiler-rt`, you might indeed need to build compiler-rt 
(or at least the builtins part of it).

FYI, I'll be out of office until Wednesday, but I'll definitely check all the 
comments here when I get back. Thanks again for all the help.


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

https://reviews.llvm.org/D126291

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

[PATCH] D112916: Confusable identifiers detection

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This also makes clang-tidy crash for fairly normal inputs, see below.




Comment at: clang-tools-extra/clang-tidy/misc/Homoglyph.cpp:84
+  if (const auto *ND = Result.Nodes.getNodeAs("nameddecl")) {
+StringRef NDName = ND->getName();
+auto &Mapped = Mapper[skeleton(NDName)];

This call is not correct, it's only valid for simple identifiers:

```
% cat t.cc
namespace ns { struct Foo {}; }
auto f = ns::Foo();
% ../llvm-build-2/bin/clang-tidy t.cc  -fix '--checks=-*,misc-homoglyph'  
-config={} --
argh Foo
Assertion failed: (Name.isIdentifier() && "Name is not a simple identifier"), 
function getName, file Decl.h, line 278.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and 
include the crash backtrace.
```

This crash breaks check-clangd in some build configs (t.cc is reduced form 
`clang-tools-extra/clangd/test/check.test`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112916

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

[clang-tools-extra] 88052fd - check_clang_tidy.py: Update run line to python3

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2022-06-03T09:28:09-04:00
New Revision: 88052fd241267a09a4e612505f589ae371b91398

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

LOG: check_clang_tidy.py: Update run line to python3

`python` no longer exists on several systems, and the script
runs under python3 when run as part of lit.

Added: 


Modified: 
clang-tools-extra/test/clang-tidy/check_clang_tidy.py

Removed: 




diff  --git a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py 
b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
index e6a4a8e50549..4ce6cd4e65de 100755
--- a/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
+++ b/clang-tools-extra/test/clang-tidy/check_clang_tidy.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 #
 #===- check_clang_tidy.py - ClangTidy Test Helper *- python 
-*--===#
 #



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

[PATCH] D112916: Confusable identifiers detection

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Reverted in 371e6f8b7fb94c444083ba115fd8edf17d6ba05c 
 for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112916

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Thanks for checking @mmuetzel !

In D126291#3555780 , @mmuetzel wrote:

> I ended up using these switches:
>
>   cmake \
> -Sllvm \
> -Bbuild \
> -GNinja \
> -DCMAKE_INSTALL_PREFIX=pkg \
> -DCMAKE_C_COMPILER=clang \
> -DCMAKE_CXX_COMPILER=clang++ \
> -DCMAKE_C_COMPILER_LAUNCHER=ccache \
> -DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
> -DLLVM_ENABLE_PROJECTS="clang;mlir;flang;llvm" \
> -DLLVM_TARGETS_TO_BUILD="X86" \
> -DCMAKE_BUILD_TYPE=Release \
> -DLLVM_ENABLE_LIBCXX=ON \
> -DCLANG_DEFAULT_RTLIB=compiler-rt \
> -DCLANG_DEFAULT_UNWINDLIB=libunwind \
> -DCLANG_DEFAULT_LINKER=lld \
> -DLLVM_BUILD_LLVM_DYLIB=OFF \
> -DLLVM_BUILD_STATIC=OFF \
> -DLLVM_ENABLE_ASSERTIONS=OFF \
> -DLLVM_ENABLE_FFI=ON \
> -DLLVM_ENABLE_THREADS=ON \
> -DLLVM_INCLUDE_EXAMPLES=OFF \
> -DLLVM_INSTALL_UTILS=ON

That's actually quite complex. How about this (that's what I'd normally use):

  cmake \
-Sllvm \
-Bbuild \
-GNinja \
-DLLVM_ENABLE_PROJECTS="clang;mlir;flang;llvm" \
-DLLVM_TARGETS_TO_BUILD="X86" \
-DCMAKE_BUILD_TYPE=Release \

Like @rovka pointed out, skipping `CLANG_DEFAULT_RTLIB` should solve your issue 
with missing libs, In general, most of the CMake options have "sane" defaults.


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

https://reviews.llvm.org/D126291

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

[PATCH] D126759: [clang][dataflow] Model calls returning optionals

[Stanislav Gatev via Phabricator via cfe-commits]

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



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:161
 
+auto possiblyAliasedOptionalType() {
+  return hasUnqualifiedDesugaredType(

xazax.hun wrote:
> Does alias refers to a type alias in this case? I first parsed this as if 
> this has something to do with pointers being aliased. I wonder if 
> `possiblyOptionalAliasType` would read better (considering the optional type 
> its own alias). Feel free to ignore. 
I opted for `hasOptionalOrAliasType`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126759

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D126291#3555805 , @awarzynski 
wrote:

> Like @rovka pointed out, skipping `CLANG_DEFAULT_RTLIB` should solve your 
> issue with missing libs

Skipping that woulnd't help/work in the clang64 environment in msys2; there's 
no libgcc there but it's expected to use compiler-rt in general.

The preexisting systemwide corresponding files do exist in 
`/clang64/lib/clang/14.0.0`, but your toolchain would want similar ones in 
`/lib/clang/15.0.0`. For testing, just copy over the 
clang//lib directory to your testroot.

Adding compiler-rt to `LLVM_ENABLE_RUNTIMES` might fix it properly, but at 
least personally, I build compiler-rt in a separate step afterwards (and I 
think msys2 builds it in that way too).


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

https://reviews.llvm.org/D126291

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

[clang] 65e710c - [clang][dataflow] Model calls returning optionals

[Stanislav Gatev via cfe-commits]

Author: Stanislav Gatev
Date: 2022-06-03T13:38:22Z
New Revision: 65e710c3fc036706ec20b357a1bfce9cbadf5705

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

LOG: [clang][dataflow] Model calls returning optionals

Model calls returning optionals

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

Reviewed-by: ymandel, xazax.hun

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Removed: 




diff  --git 
a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp 
b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index c797e02cec990..22a33c397a644 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -45,6 +45,11 @@ DeclarationMatcher optionalClass() {
 
 auto hasOptionalType() { return hasType(optionalClass()); }
 
+auto hasOptionalOrAliasType() {
+  return hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(optionalClass(;
+}
+
 auto isOptionalMemberCallWithName(
 llvm::StringRef MemberName,
 llvm::Optional Ignorable = llvm::None) {
@@ -158,6 +163,12 @@ auto isValueOrNotEqX() {
ComparesToSame(integerLiteral(equals(0);
 }
 
+auto isCallReturningOptional() {
+  return callExpr(callee(functionDecl(
+  returns(anyOf(hasOptionalOrAliasType(),
+referenceType(pointee(hasOptionalOrAliasType(;
+}
+
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
 /// symbolic value of its "has_value" property.
 StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
@@ -322,6 +333,18 @@ void transferValueOrNotEqX(const Expr *ComparisonExpr,
   });
 }
 
+void transferCallReturningOptional(const CallExpr *E,
+   const MatchFinder::MatchResult &Result,
+   LatticeTransferState &State) {
+  if (State.Env.getStorageLocation(*E, SkipPast::None) != nullptr)
+return;
+
+  auto &Loc = State.Env.createStorageLocation(*E);
+  State.Env.setStorageLocation(*E, Loc);
+  State.Env.setValue(
+  Loc, createOptionalValue(State.Env, State.Env.makeAtomicBoolValue()));
+}
+
 void assignOptionalValue(const Expr &E, LatticeTransferState &State,
  BoolValue &HasValueVal) {
   if (auto *OptionalLoc =
@@ -547,6 +570,10 @@ auto buildTransferMatchSwitch(
   // opt.value_or(X) != X
   .CaseOf(isValueOrNotEqX(), transferValueOrNotEqX)
 
+  // returns optional
+  .CaseOf(isCallReturningOptional(),
+transferCallReturningOptional)
+
   .Build();
 }
 

diff  --git 
a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index a501f549851f9..a4ea2bf5e5abc 100644
--- 
a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ 
b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2150,6 +2150,70 @@ TEST_P(UncheckedOptionalAccessTest, 
UniquePtrToStructWithOptionalField) {
   UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+$ns::$optional MakeOpt();
+
+void target() {
+  $ns::$optional opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-1]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:9:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+const $ns::$optional& MakeOpt();
+
+void target() {
+  $ns::$optional opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-2]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-2", "unsafe: input.cc:9:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using IntOpt = $ns::$optional;
+IntOpt MakeOpt();
+
+void target() {
+  IntOpt opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-3]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:10:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using IntOpt = $ns::$optional;
+const IntOpt& MakeOpt();
+
+void target() {
+  IntOpt opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-4]]*/
+}
+  )",
+  UnorderedElementsAre

[PATCH] D126759: [clang][dataflow] Model calls returning optionals

[Stanislav Gatev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sgatev marked an inline comment as done.
Closed by commit rG65e710c3fc03: [clang][dataflow] Model calls returning 
optionals (authored by sgatev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126759

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

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2150,6 +2150,70 @@
   UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+$ns::$optional MakeOpt();
+
+void target() {
+  $ns::$optional opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-1]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:9:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+const $ns::$optional& MakeOpt();
+
+void target() {
+  $ns::$optional opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-2]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-2", "unsafe: input.cc:9:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using IntOpt = $ns::$optional;
+IntOpt MakeOpt();
+
+void target() {
+  IntOpt opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-3]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:10:7")));
+
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using IntOpt = $ns::$optional;
+const IntOpt& MakeOpt();
+
+void target() {
+  IntOpt opt = 0;
+  opt = MakeOpt();
+  opt.value();
+  /*[[check-4]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
+}
+
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -45,6 +45,11 @@
 
 auto hasOptionalType() { return hasType(optionalClass()); }
 
+auto hasOptionalOrAliasType() {
+  return hasUnqualifiedDesugaredType(
+  recordType(hasDeclaration(optionalClass(;
+}
+
 auto isOptionalMemberCallWithName(
 llvm::StringRef MemberName,
 llvm::Optional Ignorable = llvm::None) {
@@ -158,6 +163,12 @@
ComparesToSame(integerLiteral(equals(0);
 }
 
+auto isCallReturningOptional() {
+  return callExpr(callee(functionDecl(
+  returns(anyOf(hasOptionalOrAliasType(),
+referenceType(pointee(hasOptionalOrAliasType(;
+}
+
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
 /// symbolic value of its "has_value" property.
 StructValue &createOptionalValue(Environment &Env, BoolValue &HasValueVal) {
@@ -322,6 +333,18 @@
   });
 }
 
+void transferCallReturningOptional(const CallExpr *E,
+   const MatchFinder::MatchResult &Result,
+   LatticeTransferState &State) {
+  if (State.Env.getStorageLocation(*E, SkipPast::None) != nullptr)
+return;
+
+  auto &Loc = State.Env.createStorageLocation(*E);
+  State.Env.setStorageLocation(*E, Loc);
+  State.Env.setValue(
+  Loc, createOptionalValue(State.Env, State.Env.makeAtomicBoolValue()));
+}
+
 void assignOptionalValue(const Expr &E, LatticeTransferState &State,
  BoolValue &HasValueVal) {
   if (auto *OptionalLoc =
@@ -547,6 +570,10 @@
   // opt.value_or(X) != X
   .CaseOf(isValueOrNotEqX(), transferValueOrNotEqX)
 
+  // returns optional
+  .CaseOf(isCallReturningOptional(),
+transferCallReturningOptional)
+
   .Build();
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126902: [Attributes] Remove AttrSyntax and migrate uses to AttributeCommonInfo::Syntax (NFC)

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a request to switch to an assert. Thanks for this cleanup!




Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:3368
+  fn("C2x", C2x);
+  OS << "case AttributeCommonInfo::Syntax::AS_Keyword:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_ContextSensitiveKeyword:\n";

lgrey wrote:
> Not sure if this should be an assert or if 0 is fine.
I think this should probably be an assertion -- keyword attributes are 
sufficiently weird that I'd rather we not accidentally support this interface; 
we can intentionally support it later if we find a need.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126902

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Markus Mützel via Phabricator via cfe-commits]

mmuetzel added a comment.

Thank you for the suggestions. In the meantime, I added 
`-DLLVM_ENABLE_PROJECTS="clang;compiler-rt;mlir;flang;llvm" 
-DCOMPILER_RT_USE_BUILTINS_LIBRARY=ON` to the compiler flags which seems to 
have advanced a bit further.
Now, I get the following when trying to compile a simple "Hello World":

  $ PATH=./pkg/bin:$PATH flang-new hello.f90 -L/clang64/lib -v
  flang-new version 15.0.0
  Target: x86_64-w64-windows-gnu
  Thread model: posix
  InstalledDir: D:/llvm-project/pkg/bin
   "D:/llvm-project/pkg/bin/flang-new" -fc1 -triple x86_64-w64-windows-gnu 
-emit-obj -o C:/msys64/tmp/hello-076f00.o hello.f90
   "C:/msys64/clang64/bin/ld.lld.exe" -m i386pep -Bdynamic -o a.exe crt2.o 
crtbegin.o -LC:/msys64/clang64/lib -LD:/llvm-project/pkg/x86_64-w64-mingw32/lib 
-LD:/llvm-project/pkg/lib 
-LD:/llvm-project/pkg/x86_64-w64-mingw32/sys-root/mingw/lib 
-LD:/llvm-project/pkg/lib/clang/15.0.0/lib/windows C:/msys64/tmp/hello-076f00.o 
-lmingw32 
D:/llvm-project/pkg/lib/clang/15.0.0/lib/windows/libclang_rt.builtins-x86_64.a 
-lunwind -lmoldname -lmingwex -lmsvcrt -ladvapi32 -lshell32 -luser32 -lkernel32 
-lmingw32 
D:/llvm-project/pkg/lib/clang/15.0.0/lib/windows/libclang_rt.builtins-x86_64.a 
-lunwind -lmoldname -lmingwex -lmsvcrt -lkernel32 crtend.o
  ld.lld: error: undefined symbol: _FortranAioBeginExternalListOutput
  >>> referenced by D:/llvm-project/./hello.f90:2
  >>>   C:/msys64/tmp/hello-6d9e30.o:(_QQmain)
  
  ld.lld: error: undefined symbol: _FortranAioOutputAscii
  >>> referenced by D:/llvm-project/./hello.f90:2
  >>>   C:/msys64/tmp/hello-6d9e30.o:(_QQmain)
  
  ld.lld: error: undefined symbol: _FortranAioEndIoStatement
  >>> referenced by D:/llvm-project/./hello.f90:2
  >>>   C:/msys64/tmp/hello-6d9e30.o:(_QQmain)
  
  ld.lld: error: undefined symbol: WinMain
  >>> referenced by 
C:/M/mingw-w64-crt-git/src/mingw-w64/mingw-w64-crt/crt/crt0_c.c:18
  >>>   libmingw32.a(lib64_libmingw32_a-crt0_c.o):(main)
  flang-new: error: linker command failed with exit code 1 (use -v to see 
invocation)

Is this still a configuration error?


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

https://reviews.llvm.org/D126291

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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Andy Soffer via Phabricator via cfe-commits]

asoffer added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp:90
+if (!Op->isAssignmentOp()) {
+  markSideEffectFree(Op->getRHS());
+}

Perhaps I'm not understanding precisely what `markSideEffectFree` means, but 
this doesn't seem right to me:

```
int *data = ...;
auto getNextEntry = [&] () -> int& { return *++data; };
int n = (getNextDataEntry() + 17); // We can't mark RHS as side-effect free.
*data = n;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm a bit uneasy that this is implementing something that's not yet been 
accepted into the Itanium ABI document. That runs the risk of requiring an ABI 
break if the Itanium document changes directions before finalizing. Also, what 
should we be doing for the Microsoft mangling, or do we already handle that 
properly?




Comment at: clang/include/clang/AST/Decl.h:2495
+  // constraints on it, so that we can decide to mangle this with its 
containing
+  // scope and the 'F' for itanium.
+  bool isConstrainedFriend() const;





Comment at: clang/lib/AST/ItaniumMangle.cpp:668-669
+// different functions.  See [temp.friend]p9.
+if (FD->isConstrainedFriend())
+return FD->getLexicalDeclContext();
+  }

Formatting looks off here -- should clang-format the patch.



Comment at: clang/lib/AST/ItaniumMangle.cpp:672-674
+  // If this is a friend, and has constraints, mangle it in the decl context
+  // of its lexical context, since in different scopes, they are considered
+  // different functions.  See [temp.friend]p9.

I don't know that repeating the comment here is helpful.



Comment at: clang/lib/AST/ItaniumMangle.cpp:1733
   // 
-  //   ::= N [] []   E
+  //   ::= N [] []  [F} 
+  //E




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

https://reviews.llvm.org/D126818

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

Nice!

In D126291#3555835 , @mmuetzel wrote:

> Is this still a configuration error?

No :) Clearly the following `if` block from `tools::addFortranRuntimeLibs` is 
not entered:

  if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
  CmdArgs.push_back("Fortran_main.lib");
  CmdArgs.push_back("FortranRuntime.lib");
  CmdArgs.push_back("FortranDecimal.lib");
} 

The missing symbols reported by the linker are from thes Fortran runtime 
libraries listed above. These libs should be included in the linker invocation 
generated by `flang-new`. I'm assuming that this makes sense - given the 
definition of isKnownWindowsMSVCEnvironment 
.
 If you use isOSWindows 

 instead then it should work, right?


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

https://reviews.llvm.org/D126291

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Markus Mützel via Phabricator via cfe-commits]

mmuetzel added a comment.

In D126291#3555848 , @awarzynski 
wrote:

> Nice!
>
> In D126291#3555835 , @mmuetzel 
> wrote:
>
>> Is this still a configuration error?
>
> No :) Clearly the following `if` block from `tools::addFortranRuntimeLibs` is 
> not entered:
>
>   if (TC.getTriple().isKnownWindowsMSVCEnvironment()) {
>   CmdArgs.push_back("Fortran_main.lib");
>   CmdArgs.push_back("FortranRuntime.lib");
>   CmdArgs.push_back("FortranDecimal.lib");
> } 
>
> The missing symbols reported by the linker are from thes Fortran runtime 
> libraries listed above. These libs should be included in the linker 
> invocation generated by `flang-new`. I'm assuming that this makes sense - 
> given the definition of isKnownWindowsMSVCEnvironment 
> .
>  If you use isOSWindows 
> 
>  instead then it should work, right?

These libraries are named like this in MinGW:

  $ ls ./pkg/lib/*Fortran*
  ./pkg/lib/libFortran_main.a   ./pkg/lib/libFortranDecimal.a   
./pkg/lib/libFortranLower.a   ./pkg/lib/libFortranRuntime.a
  ./pkg/lib/libFortranCommon.a  ./pkg/lib/libFortranEvaluate.a  
./pkg/lib/libFortranParser.a  ./pkg/lib/libFortranSemantics.a


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

https://reviews.llvm.org/D126291

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

[clang] efbf013 - Only issue warning for subtraction involving null pointers on live code paths

[Jamie Schmeiser via cfe-commits]

Author: Jamie Schmeiser
Date: 2022-06-03T10:10:37-04:00
New Revision: efbf0136b4108692ddd1a852b3f5b232c10d2097

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

LOG: Only issue warning for subtraction involving null pointers on live code 
paths

Summary:
Change the warning produced for subtraction from (or with) a null pointer
to only be produced when the code path is live.
https://github.com/llvm/llvm-project/issues/54570

Author: Jamie Schmeiser 
Reviewed By: anarazel (Andres Freund)
Differential Revision: https://reviews.llvm.org/D126816

Added: 


Modified: 
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/pointer-subtraction.c
clang/test/Sema/pointer-subtraction.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 3e8bd63e89ae9..6627aaf3e6d88 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -10845,8 +10845,10 @@ static void diagnoseSubtractionOnNullPointer(Sema &S, 
SourceLocation Loc,
   if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
 return;
 
-  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
-  << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+  S.DiagRuntimeBehavior(Loc, Pointer,
+S.PDiag(diag::warn_pointer_sub_null_ptr)
+<< S.getLangOpts().CPlusPlus
+<< Pointer->getSourceRange());
 }
 
 /// Diagnose invalid arithmetic on two function pointers.

diff  --git a/clang/test/Sema/pointer-subtraction.c 
b/clang/test/Sema/pointer-subtraction.c
index c3dbbd47459fa..79d18a0f0a306 100644
--- a/clang/test/Sema/pointer-subtraction.c
+++ b/clang/test/Sema/pointer-subtraction.c
@@ -11,6 +11,16 @@ void a(void) {
   f = (char *)(f - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}} 
expected-warning {{performing pointer subtraction with a null pointer has 
undefined behavior}}
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else

diff  --git a/clang/test/Sema/pointer-subtraction.cpp 
b/clang/test/Sema/pointer-subtraction.cpp
index efbb3255e5d31..28063a1fea6ef 100644
--- a/clang/test/Sema/pointer-subtraction.cpp
+++ b/clang/test/Sema/pointer-subtraction.cpp
@@ -11,6 +11,16 @@ void a() {
   f = (char *)(f - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // valid in C++
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else



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

[PATCH] D126816: Only issue warning for subtraction involving null pointers on live code paths

[Jamie Schmeiser 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 rGefbf0136b410: Only issue warning for subtraction involving 
null pointers on live code paths (authored by jamieschmeiser).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126816

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/pointer-subtraction.c
  clang/test/Sema/pointer-subtraction.cpp


Index: clang/test/Sema/pointer-subtraction.cpp
===
--- clang/test/Sema/pointer-subtraction.cpp
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // valid in C++
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer may have undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/test/Sema/pointer-subtraction.c
===
--- clang/test/Sema/pointer-subtraction.c
+++ clang/test/Sema/pointer-subtraction.c
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}} 
expected-warning {{performing pointer subtraction with a null pointer has 
undefined behavior}}
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing 
pointer subtraction with a null pointer has undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -10845,8 +10845,10 @@
   if (S.Diags.getSuppressSystemWarnings() && S.SourceMgr.isInSystemMacro(Loc))
 return;
 
-  S.Diag(Loc, diag::warn_pointer_sub_null_ptr)
-  << S.getLangOpts().CPlusPlus << Pointer->getSourceRange();
+  S.DiagRuntimeBehavior(Loc, Pointer,
+S.PDiag(diag::warn_pointer_sub_null_ptr)
+<< S.getLangOpts().CPlusPlus
+<< Pointer->getSourceRange());
 }
 
 /// Diagnose invalid arithmetic on two function pointers.


Index: clang/test/Sema/pointer-subtraction.cpp
===
--- clang/test/Sema/pointer-subtraction.cpp
+++ clang/test/Sema/pointer-subtraction.cpp
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // valid in C++
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer may have undefined behavior}}
+
 #ifndef SYSTEM_WARNINGS
   SYSTEM_MACRO(f);
 #else
Index: clang/test/Sema/pointer-subtraction.c
===
--- clang/test/Sema/pointer-subtraction.c
+++ clang/test/Sema/pointer-subtraction.c
@@ -11,6 +11,16 @@
   f = (char *)(f - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
   f = (char *)((char *)0 - (char *)0); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}} expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
 
+  if (1)
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+  else
+f = (char *)((char *)0 - f);
+
+  if (0)
+f = (char *)((char *)0 - f);
+  else
+f = (char *)((char *)0 - f); // expected-warning {{performing pointer subtraction with a null pointer has undefined behavior}}
+
 #ifndef SYSTE

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

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

xbolva00 added inline comments.



Comment at: clang/docs/ClangCommandLineReference.rst:2624
 
+.. option:: -fstrict-flex-array
+

arrays


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126864

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

[PATCH] D126818: Itanium ABI: Implement mangling for constrained friends

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D126818#3555843 , @aaron.ballman 
wrote:

> I'm a bit uneasy that this is implementing something that's not yet been 
> accepted into the Itanium ABI document. That runs the risk of requiring an 
> ABI break if the Itanium document changes directions before finalizing. Also, 
> what should we be doing for the Microsoft mangling, or do we already handle 
> that properly?

I'm a touch uneasy as well, but there at least seems to be agreement in this 
solution (at least according to Richard).  I'm hopeful that @rjmccall can 
comment as to whether this is the 'final' version here, or what we can do to 
finalize it.

The Microsoft mangling IS a problem as well, the Microsoft compiler has the 
same issue (https://godbolt.org/z/cnf6nM1aj), but I don't want to fix it there 
until MSVC does so that we don't diverge on their implementation.  In the 
meantime, these would just not be callable in that situation (as they are now).


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

https://reviews.llvm.org/D126818

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

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz updated this revision to Diff 434020.
sgraenitz added a comment.

Fix unchecked nullptr compiler crash and assertion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124762

Files:
  clang/lib/CodeGen/CGCall.cpp
  llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
  llvm/lib/CodeGen/WinEHPrepare.cpp


Index: llvm/lib/CodeGen/WinEHPrepare.cpp
===
--- llvm/lib/CodeGen/WinEHPrepare.cpp
+++ llvm/lib/CodeGen/WinEHPrepare.cpp
@@ -963,7 +963,7 @@
 if (auto BU = CB->getOperandBundle(LLVMContext::OB_funclet))
   FuncletBundleOperand = BU->Inputs.front();
 
-if (FuncletBundleOperand == FuncletPad)
+if (!FuncletPad || FuncletBundleOperand == FuncletPad)
   continue;
 
 // Skip call sites which are nounwind intrinsics or inline asm.
Index: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
===
--- llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -107,7 +107,9 @@
 
 IRBuilder<> Builder(CI->getParent(), CI->getIterator());
 SmallVector Args(CI->args());
-CallInst *NewCI = Builder.CreateCall(FCache, Args);
+SmallVector BundleList;
+CI->getOperandBundlesAsDefs(BundleList);
+CallInst *NewCI = Builder.CreateCall(FCache, Args, BundleList);
 NewCI->setName(CI->getName());
 
 // Try to set the most appropriate TailCallKind based on both the current
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -25,11 +25,13 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/CodeGen/SwiftCallingConv.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Analysis/ObjCARCInstKind.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/IR/Attributes.h"
@@ -4465,16 +4467,37 @@
 CodeGenFunction::getBundlesForFunclet(llvm::Value *Callee) {
   SmallVector BundleList;
   // There is no need for a funclet operand bundle if we aren't inside a
-  // funclet.
+  // funclet or the callee is not a function.
   if (!CurrentFuncletPad)
 return BundleList;
-
-  // Skip intrinsics which cannot throw.
   auto *CalleeFn = dyn_cast(Callee->stripPointerCasts());
-  if (CalleeFn && CalleeFn->isIntrinsic() && CalleeFn->doesNotThrow())
+  if (!CalleeFn)
 return BundleList;
 
-  BundleList.emplace_back("funclet", CurrentFuncletPad);
+  // Skip intrinsics which cannot throw.
+  bool InsertFuncletOp = true;
+  if (CalleeFn->isIntrinsic() && CalleeFn->doesNotThrow())
+InsertFuncletOp = false;
+
+  // Most ObjC ARC intrinics are lowered in PreISelIntrinsicLowering. Thus,
+  // WinEHPrepare will see them as regular calls. We need to set the funclet
+  // operand explicitly in this case to avoid accidental truncation of EH
+  // funclets on Windows.
+  if (CalleeFn->isIntrinsic() && CalleeFn->doesNotThrow()) {
+if (CGM.getTarget().getTriple().isOSWindows()) {
+  assert(CGM.getLangOpts().ObjCRuntime.getKind() == ObjCRuntime::GNUstep &&
+ "Only reproduced with GNUstep so far, but likely applies to other 
"
+ "ObjC runtimes on Windows");
+  using namespace llvm::objcarc;
+  ARCInstKind CalleeKind = GetFunctionClass(CalleeFn);
+  if (!IsUser(CalleeKind) && CalleeKind != ARCInstKind::None)
+InsertFuncletOp = true;
+}
+  }
+
+  if (InsertFuncletOp)
+BundleList.emplace_back("funclet", CurrentFuncletPad);
+
   return BundleList;
 }
 


Index: llvm/lib/CodeGen/WinEHPrepare.cpp
===
--- llvm/lib/CodeGen/WinEHPrepare.cpp
+++ llvm/lib/CodeGen/WinEHPrepare.cpp
@@ -963,7 +963,7 @@
 if (auto BU = CB->getOperandBundle(LLVMContext::OB_funclet))
   FuncletBundleOperand = BU->Inputs.front();
 
-if (FuncletBundleOperand == FuncletPad)
+if (!FuncletPad || FuncletBundleOperand == FuncletPad)
   continue;
 
 // Skip call sites which are nounwind intrinsics or inline asm.
Index: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
===
--- llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -107,7 +107,9 @@
 
 IRBuilder<> Builder(CI->getParent(), CI->getIterator());
 SmallVector Args(CI->args());
-CallInst *NewCI = Builder.CreateCall(FCache, Args);
+SmallVector BundleList;
+CI->getOperandBundlesAsDefs(BundleList);
+CallInst *NewCI = Builder.CreateCall(FCache, Args, B

[PATCH] D126291: [flang][Driver] Update link job on windows

[Markus Mützel via Phabricator via cfe-commits]

mmuetzel added a comment.

I made this additional change:

  From 26cee469225e80ac9bae22ebb6e60d47373fc19d Mon Sep 17 00:00:00 2001
  From: =?UTF-8?q?Markus=20M=C3=BCtzel?= 
  Date: Fri, 3 Jun 2022 16:23:47 +0200
  Subject: [PATCH] MinGW
  
  ---
   clang/lib/Driver/ToolChains/MinGW.cpp | 7 +++
   1 file changed, 7 insertions(+)
  
  diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp 
b/clang/lib/Driver/ToolChains/MinGW.cpp
  index ceeaa79bc202..eba8e9db82ea 100644
  --- a/clang/lib/Driver/ToolChains/MinGW.cpp
  +++ b/clang/lib/Driver/ToolChains/MinGW.cpp
  @@ -138,6 +138,13 @@ void tools::MinGW::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
   llvm_unreachable("Unsupported target architecture.");
 }
   
  +  if (C.getDriver().IsFlangMode()) {
  +tools::addFortranRuntimeLibraryPath(TC, Args, CmdArgs);
  +tools::addFortranRuntimeLibs(TC, CmdArgs);
  +  }
  +
  +  // Add the compiler-rt library directories to libpath if they exist to help
  +
 Arg *SubsysArg =
 Args.getLastArg(options::OPT_mwindows, options::OPT_mconsole);
 if (SubsysArg && SubsysArg->getOption().matches(options::OPT_mwindows)) {
  -- 
  2.35.3.windows.1

No the error message changed to the following:

  $ PATH=./pkg/bin:$PATH flang-new hello.f90 -L/clang64/lib -v
  flang-new version 15.0.0
  Target: x86_64-w64-windows-gnu
  Thread model: posix
  InstalledDir: D:/llvm-project/pkg/bin
   "D:/llvm-project/pkg/bin/flang-new" -fc1 -triple x86_64-w64-windows-gnu 
-emit-obj -o C:/msys64/tmp/hello-f38fa1.o hello.f90
   "C:/msys64/clang64/bin/ld.lld.exe" -m i386pep -LD:/llvm-project/pkg/lib 
-lFortran_main -lFortranRuntime -lFortranDecimal -Bdynamic -o a.exe crt2.o 
crtbegin.o -LC:/msys64/clang64/lib -LD:/llvm-project/pkg/x86_64-w64-mingw32/lib 
-LD:/llvm-project/pkg/lib 
-LD:/llvm-project/pkg/x86_64-w64-mingw32/sys-root/mingw/lib 
-LD:/llvm-project/pkg/lib/clang/15.0.0/lib/windows C:/msys64/tmp/hello-f38fa1.o 
-lmingw32 
D:/llvm-project/pkg/lib/clang/15.0.0/lib/windows/libclang_rt.builtins-x86_64.a 
-lunwind -lmoldname -lmingwex -lmsvcrt -ladvapi32 -lshell32 -luser32 -lkernel32 
-lmingw32 
D:/llvm-project/pkg/lib/clang/15.0.0/lib/windows/libclang_rt.builtins-x86_64.a 
-lunwind -lmoldname -lmingwex -lmsvcrt -lkernel32 crtend.o
  ld.lld: error: undefined symbol: std::__1::mutex::lock()
  >>> referenced by 
libFortranRuntime.a(io-api.cpp.obj):(Fortran::runtime::io::IoStatementState* 
Fortran::runtime::io::BeginExternalListIO<(Fortran::runtime::io::Direction)0, 
Fortran::runtime::io::ExternalListIoStatementState>(int, char const*, int))
  >>> referenced by 
libFortranRuntime.a(io-api.cpp.obj):(Fortran::runtime::io::IoStatementState* 
Fortran::runtime::io::BeginExternalListIO<(Fortran::runtime::io::Direction)0, 
Fortran::runtime::io::ExternalListIoStatementState>(int, char const*, int))
  >>> referenced by 
libFortranRuntime.a(io-api.cpp.obj):(Fortran::runtime::io::IoStatementState* 
Fortran::runtime::io::BeginExternalListIO<(Fortran::runtime::io::Direction)1, 
Fortran::runtime::io::ExternalListIoStatementState>(int, char const*, int))
  >>> referenced 36 more times
  
  ld.lld: error: undefined symbol: std::__1::mutex::unlock()
  >>> referenced by 
libFortranRuntime.a(unit.cpp.obj):(Fortran::runtime::io::FlushOutputOnCrash(Fortran::runtime::Terminator
 const&))
  >>> referenced by 
libFortranRuntime.a(unit.cpp.obj):(Fortran::runtime::io::ExternalFileUnit::LookUp(int))
  >>> referenced by 
libFortranRuntime.a(unit.cpp.obj):(Fortran::runtime::io::ExternalFileUnit::GetUnitMap())
  >>> referenced 23 more times
  flang-new: error: linker command failed with exit code 1 (use -v to see 
invocation)

Not sure what next. 🤷‍♂️


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

https://reviews.llvm.org/D126291

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

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz updated this revision to Diff 434023.
sgraenitz added a comment.
Herald added subscribers: jsji, pengfei.

LLVM CodeGen: check that presence of bundle operands avoids truncation


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124762

Files:
  clang/lib/CodeGen/CGCall.cpp
  llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
  llvm/lib/CodeGen/WinEHPrepare.cpp
  llvm/test/CodeGen/X86/win64-funclet-preisel-intrinsics.ll

Index: llvm/test/CodeGen/X86/win64-funclet-preisel-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/win64-funclet-preisel-intrinsics.ll
@@ -0,0 +1,67 @@
+; RUN: llc -mtriple=x86_64-windows-msvc < %s | FileCheck %s
+
+; Reduced IR generated from ObjC++ source:
+;
+; @class Ety;
+; void opaque(void);
+; void test_catch(void) {
+;   @try {
+; opaque();
+;   } @catch (Ety *ex) {
+; // Destroy ex when leaving catchpad. This emits calls to two intrinsic
+; // functions, llvm.objc.retain and llvm.objc.storeStrong, but only one
+; // is required to trigger the funclet truncation.
+;   }
+; }
+
+define void @test_catch() personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+entry:
+  %exn.slot = alloca i8*, align 8
+  %ex2 = alloca i8*, align 8
+  invoke void @opaque() to label %invoke.cont unwind label %catch.dispatch
+
+catch.dispatch:
+  %0 = catchswitch within none [label %catch] unwind to caller
+
+invoke.cont:
+  unreachable
+
+catch:
+  %1 = catchpad within %0 [i8* null, i32 64, i8** %exn.slot]
+  call void @llvm.objc.storeStrong(i8** %ex2, i8* null) [ "funclet"(token %1) ]
+  catchret from %1 to label %catchret.dest
+
+catchret.dest:
+  ret void
+}
+
+declare void @opaque()
+declare void @llvm.objc.storeStrong(i8**, i8*) #0
+declare i32 @__CxxFrameHandler3(...)
+
+attributes #0 = { nounwind }
+
+; llvm.objc.storeStrong is a Pre-ISel intrinsic, which used to cause truncations
+; when it occurred in SEH funclets like catchpads:
+; CHECK: # %catch
+; CHECK: pushq   %rbp
+; CHECK: .seh_pushreg %rbp
+;...
+; CHECK: .seh_endprologue
+;
+; At this point the code used to be truncated:
+; CHECK-NOT: int3
+;
+; Instead, the call to objc_storeStrong should be emitted:
+; CHECK: leaq	-24(%rbp), %rcx
+; CHECK: xorl	%edx, %edx
+; CHECK: callq	objc_storeStrong
+;...
+; 
+; This is the end of the funclet:
+; CHECK: popq	%rbp
+; CHECK: retq# CATCHRET
+;...
+; CHECK: .seh_handlerdata
+;...
+; CHECK: .seh_endproc
Index: llvm/lib/CodeGen/WinEHPrepare.cpp
===
--- llvm/lib/CodeGen/WinEHPrepare.cpp
+++ llvm/lib/CodeGen/WinEHPrepare.cpp
@@ -963,7 +963,7 @@
 if (auto BU = CB->getOperandBundle(LLVMContext::OB_funclet))
   FuncletBundleOperand = BU->Inputs.front();
 
-if (FuncletBundleOperand == FuncletPad)
+if (!FuncletPad || FuncletBundleOperand == FuncletPad)
   continue;
 
 // Skip call sites which are nounwind intrinsics or inline asm.
Index: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
===
--- llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -107,7 +107,9 @@
 
 IRBuilder<> Builder(CI->getParent(), CI->getIterator());
 SmallVector Args(CI->args());
-CallInst *NewCI = Builder.CreateCall(FCache, Args);
+SmallVector BundleList;
+CI->getOperandBundlesAsDefs(BundleList);
+CallInst *NewCI = Builder.CreateCall(FCache, Args, BundleList);
 NewCI->setName(CI->getName());
 
 // Try to set the most appropriate TailCallKind based on both the current
Index: clang/lib/CodeGen/CGCall.cpp
===
--- clang/lib/CodeGen/CGCall.cpp
+++ clang/lib/CodeGen/CGCall.cpp
@@ -25,11 +25,13 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclObjC.h"
 #include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/TargetBuiltins.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/CodeGen/SwiftCallingConv.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Analysis/ObjCARCInstKind.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Assumptions.h"
 #include "llvm/IR/Attributes.h"
@@ -4465,16 +4467,37 @@
 CodeGenFunction::getBundlesForFunclet(llvm::Value *Callee) {
   SmallVector BundleList;
   // There is no need for a funclet operand bundle if we aren't inside a
-  // funclet.
+  // funclet or the callee is not a function.
   if (!CurrentFuncletPad)
 return BundleList;
-
-  // Skip intrinsics which cannot throw.
   auto *CalleeFn = dyn_cast(Callee->stripPointerCasts

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz updated this revision to Diff 434024.
sgraenitz added a comment.

Clang frontend: check that 'funclet' bundle operands are emitted for Pre-ISel 
intrinsics


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124762

Files:
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGenObjCXX/arc-exceptions-seh.mm
  llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
  llvm/lib/CodeGen/WinEHPrepare.cpp
  llvm/test/CodeGen/X86/win64-funclet-preisel-intrinsics.ll

Index: llvm/test/CodeGen/X86/win64-funclet-preisel-intrinsics.ll
===
--- /dev/null
+++ llvm/test/CodeGen/X86/win64-funclet-preisel-intrinsics.ll
@@ -0,0 +1,67 @@
+; RUN: llc -mtriple=x86_64-windows-msvc < %s | FileCheck %s
+
+; Reduced IR generated from ObjC++ source:
+;
+; @class Ety;
+; void opaque(void);
+; void test_catch(void) {
+;   @try {
+; opaque();
+;   } @catch (Ety *ex) {
+; // Destroy ex when leaving catchpad. This emits calls to two intrinsic
+; // functions, llvm.objc.retain and llvm.objc.storeStrong, but only one
+; // is required to trigger the funclet truncation.
+;   }
+; }
+
+define void @test_catch() personality i8* bitcast (i32 (...)* @__CxxFrameHandler3 to i8*) {
+entry:
+  %exn.slot = alloca i8*, align 8
+  %ex2 = alloca i8*, align 8
+  invoke void @opaque() to label %invoke.cont unwind label %catch.dispatch
+
+catch.dispatch:
+  %0 = catchswitch within none [label %catch] unwind to caller
+
+invoke.cont:
+  unreachable
+
+catch:
+  %1 = catchpad within %0 [i8* null, i32 64, i8** %exn.slot]
+  call void @llvm.objc.storeStrong(i8** %ex2, i8* null) [ "funclet"(token %1) ]
+  catchret from %1 to label %catchret.dest
+
+catchret.dest:
+  ret void
+}
+
+declare void @opaque()
+declare void @llvm.objc.storeStrong(i8**, i8*) #0
+declare i32 @__CxxFrameHandler3(...)
+
+attributes #0 = { nounwind }
+
+; llvm.objc.storeStrong is a Pre-ISel intrinsic, which used to cause truncations
+; when it occurred in SEH funclets like catchpads:
+; CHECK: # %catch
+; CHECK: pushq   %rbp
+; CHECK: .seh_pushreg %rbp
+;...
+; CHECK: .seh_endprologue
+;
+; At this point the code used to be truncated:
+; CHECK-NOT: int3
+;
+; Instead, the call to objc_storeStrong should be emitted:
+; CHECK: leaq	-24(%rbp), %rcx
+; CHECK: xorl	%edx, %edx
+; CHECK: callq	objc_storeStrong
+;...
+; 
+; This is the end of the funclet:
+; CHECK: popq	%rbp
+; CHECK: retq# CATCHRET
+;...
+; CHECK: .seh_handlerdata
+;...
+; CHECK: .seh_endproc
Index: llvm/lib/CodeGen/WinEHPrepare.cpp
===
--- llvm/lib/CodeGen/WinEHPrepare.cpp
+++ llvm/lib/CodeGen/WinEHPrepare.cpp
@@ -963,7 +963,7 @@
 if (auto BU = CB->getOperandBundle(LLVMContext::OB_funclet))
   FuncletBundleOperand = BU->Inputs.front();
 
-if (FuncletBundleOperand == FuncletPad)
+if (!FuncletPad || FuncletBundleOperand == FuncletPad)
   continue;
 
 // Skip call sites which are nounwind intrinsics or inline asm.
Index: llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
===
--- llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
+++ llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
@@ -107,7 +107,9 @@
 
 IRBuilder<> Builder(CI->getParent(), CI->getIterator());
 SmallVector Args(CI->args());
-CallInst *NewCI = Builder.CreateCall(FCache, Args);
+SmallVector BundleList;
+CI->getOperandBundlesAsDefs(BundleList);
+CallInst *NewCI = Builder.CreateCall(FCache, Args, BundleList);
 NewCI->setName(CI->getName());
 
 // Try to set the most appropriate TailCallKind based on both the current
Index: clang/test/CodeGenObjCXX/arc-exceptions-seh.mm
===
--- /dev/null
+++ clang/test/CodeGenObjCXX/arc-exceptions-seh.mm
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fobjc-arc -fexceptions -fobjc-exceptions -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s
+
+@class Ety;
+void opaque(void);
+void test_catch_preisel_intrinsic(void) {
+  @try {
+opaque();
+  } @catch (Ety *ex) {
+// Destroy ex when leaving catchpad. Emits calls to two intrinsic functions,
+// that should both have a "funclet" bundle operand that refers to the
+// catchpad's SSA value.
+  }
+}
+
+// CHECK-LABEL: define{{.*}} void {{.*}}test_catch_preisel_intrinsic
+//...
+// CHECK:   catch.dispatch:
+// CHECK-NEXT:[[CATCHSWITCH:%[0-9]+]] = catchswitch within none
+//...
+// CHECK:   catch:
+// CHECK-NEXT:[[CATCHPAD:%[0-9]+]] = catchpad within [[CATCHSWITCH]]
+// CHECK: {{%[0-9]+}} = call {{.*}} @llvm.objc.retai

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Luca Versari via Phabricator via cfe-commits]

veluca93 marked 5 inline comments as done.
veluca93 added a comment.

In D124918#3555719 , @ymandel wrote:

> Luca,
>
> Can you expand the description to give a better intuition as to what this 
> check is finding that the diagnostic does not? The example is good, but it 
> would be helpful if you could phrase the general rule being enforced.  
> Similarly, that would be helpful in the documentation file -- before diving 
> into the formal specification, explain what the intuition. For that matter, 
> why does this merit a separate clang-tidy vs. integeration into the existing 
> code for the diagnostic (or a similar, parallel one)? I'm not advocating one 
> way or another, I'd just like to understand your decision.

I wrote in the doc:

This check is similar to the -Wunused-variable and -Wunused-but-set-variable
compiler warnings; however, it integrates (configurable) knowledge about
library types and functions to be able to extend these checks to more types of
variables that do not produce user-visible side effects but i.e. perform
allocations; not using such variables is almost always a programming error.

The reason for this to be a tidy rather than an extension of the warning is 
twofold:

- I'm not quite confident that this check meets (or can meet) the standard for 
FPR that warnings are held to. It may well be the case, but it's not something 
I checked extensively.
- I don't think compiler warnings can be configured to the extent that would be 
needed for this check to be useful.

> Separately, can you provide some data as to how much of a problem this is in 
> practice? The code here is quite complex and I think that sets a high bar (in 
> terms of benefit to users) for committing it.

I have seen this check trigger about 100k times among ~100M analyzed lines of 
code.

> Thanks!






Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp:90
+if (!Op->isAssignmentOp()) {
+  markSideEffectFree(Op->getRHS());
+}

asoffer wrote:
> Perhaps I'm not understanding precisely what `markSideEffectFree` means, but 
> this doesn't seem right to me:
> 
> ```
> int *data = ...;
> auto getNextEntry = [&] () -> int& { return *++data; };
> int n = (getNextDataEntry() + 17); // We can't mark RHS as side-effect free.
> *data = n;
> ```
`markSideEffectFree` is a bad name - I renamed it to `markMaybeNotObservable`, 
with the idea being that an expression is observable iff any of its ancestors 
in the AST is marked as Observable.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h:56
+  llvm::SmallVector SmartPointerTypes;
+  // Map of function name to bitmask of arguments that are not read from.
+  llvm::StringMap> FunctionAllowList;

ymandel wrote:
> can you expand or rephrase? is the idea that these are argument positions 
> that should be ignored for the purposes of this check? If so, what does the 
> name of the field mean?
I renamed the field and expanded the comment.



Comment at: clang-tools-extra/docs/clang-tidy/checks/list.rst:116
`bugprone-virtual-near-miss `_, "Yes"
-   `cert-dcl21-cpp `_, "Yes"
+   `cert-dcl21-cpp `_,
`cert-dcl50-cpp `_,

ymandel wrote:
> why the change to this line?
No idea, must have been a merge gone wrong :) Reverted.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Luca Versari via Phabricator via cfe-commits]

veluca93 updated this revision to Diff 434026.
veluca93 marked 3 inline comments as done.
veluca93 added a comment.

Respond to comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
@@ -0,0 +1,144 @@
+// RUN: %check_clang_tidy %s performance-unused-no-side-effect %t -- \
+// RUN:  -config="{CheckOptions: [{key: \"performance-unused-no-side-effect.TypesByBase\", value: '{\"Marker\":[\"swap\"]}'}]}"
+
+namespace std {
+
+template 
+struct char_traits {};
+
+template 
+struct allocator {};
+
+template <
+class CharT,
+class Traits = std::char_traits,
+class Allocator = std::allocator>
+class basic_string {
+public:
+  basic_string &operator+=(const char *);
+  ~basic_string();
+};
+
+using string = basic_string;
+
+class thread {
+public:
+  ~thread();
+};
+
+template >
+class vector {
+public:
+  void push_back(T);
+};
+
+template 
+class unique_ptr {
+public:
+  unique_ptr(T *);
+};
+
+} // namespace std
+
+struct Marker {
+};
+
+struct SideEffectFree : Marker {
+  SideEffectFree();
+  ~SideEffectFree();
+  void swap(SideEffectFree *);
+};
+
+struct POD {
+  int A;
+};
+
+template 
+struct CustomPair {
+  T t;
+  U u;
+};
+
+void f(const std::string &);
+
+void unusedString() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+}
+
+void unusedStringWithMethodCall() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+  SomeString += "hi";
+}
+
+void unusedStdThread() {
+  std::thread T;
+}
+
+void usedString() {
+  std::string SomeString;
+  SomeString += "hi";
+  f(SomeString);
+}
+
+void vectorString() {
+  std::vector VecString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: variable 'VecString' is never read [performance-unused-no-side-effect]
+  VecString.push_back(std::string{});
+}
+
+void vectorThread() {
+  std::vector VecThread;
+  VecThread.push_back(std::thread{});
+}
+
+void withMarker() {
+  SideEffectFree M;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: variable 'M' is never read [performance-unused-no-side-effect]
+}
+
+void withMarkerAndExcludedMethod(SideEffectFree *S) {
+  SideEffectFree M;
+  M.swap(S);
+}
+
+void withPOD() {
+  POD P;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'P' is never read [performance-unused-no-side-effect]
+  P.A = 1;
+}
+
+int withPODReturn() {
+  POD P;
+  P.A = 1;
+  return P.A;
+}
+
+void withLoops() {
+  std::vector VecLoops;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: variable 'VecLoops' is never read [performance-unused-no-side-effect]
+  for (int i = 0; i < 15; i++) {
+VecLoops.push_back(1);
+  }
+  int j = 0;
+  while (j < 1) {
+VecLoops.push_back(2);
+j++;
+  }
+  (VecLoops).push_back(3);
+}
+
+void withSmartPointer(int *Arg) {
+  std::unique_ptr Unused(new int);
+  // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: variable 'Unused' is never read [performance-unused-no-side-effect]
+
+  std::unique_ptr Used(Arg);
+}
+
+void withCustomPair() {
+  CustomPair> Unused{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:45: warning: variable 'Unused' is never read [performance-unused-no-side-effect]
+
+  CustomPair Used;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
@@ -0,0 +1,199 @@
+.. title:: clang-tidy - performance-unused-no-side-effect
+
+performance-unused-no-side-effect
+=
+
+Finds variables that are not used and do not have user-visible side effects.
+
+This check is similar to the -Wunused-variable and -Wunused-but-set-variable
+compiler warnings; however, it integrates (configurable) knowledge about
+library types and functions to be able to extend these checks to more types of
+variables that do not produce user-visible side effects but i.e. perform
+allocations; not using such variables is almost always a programming error.
+

[PATCH] D126725: [pseudo] rename pseudo-gen -> clang-pseudo-gen. NFC

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Renaming this seems like a great change to me fwiw :)




Comment at: llvm/utils/gn/secondary/clang-tools-extra/pseudo/gen/BUILD.gn:1
-executable("pseudo-gen") {
+executable("clang-pseudo-gen") {
   configs += [ "//llvm/utils/gn/build:clang_code" ]

hokein wrote:
> I was skeptical there should be an additional change in the gn build 
> (building the inc file), but it looks like the gn file is incomplete, the 
> `cxx` doesn't compile in gn build, there is a 
> [fixme](https://github.com/llvm/llvm-project/blob/main/llvm/utils/gn/secondary/clang-tools-extra/pseudo/cxx/BUILD.gn#L2),
>  I think it is ok.
(fwiw we usually only add things to the gn build covered by tests. nothing in 
the test suite needs this at the moment, so i haven't ported it. i have a local 
branch ready to go in case it's needed, it looks like D126966.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126725

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Markus Mützel via Phabricator via cfe-commits]

mmuetzel added a comment.

In case you don't receive notifications on edits. (Please forgive the noise if 
you do.)
After applying this additional patch:

  From d74a276679778b943b1e2e50f5dd45f65c530252 Mon Sep 17 00:00:00 2001
  From: =?UTF-8?q?Markus=20M=C3=BCtzel?= 
  Date: Fri, 3 Jun 2022 16:36:19 +0200
  Subject: [PATCH] MinGW
  
  ---
   clang/lib/Driver/ToolChains/MinGW.cpp | 5 +
   1 file changed, 5 insertions(+)
  
  diff --git a/clang/lib/Driver/ToolChains/MinGW.cpp 
b/clang/lib/Driver/ToolChains/MinGW.cpp
  index ceeaa79bc202..9e2200467548 100644
  --- a/clang/lib/Driver/ToolChains/MinGW.cpp
  +++ b/clang/lib/Driver/ToolChains/MinGW.cpp
  @@ -218,6 +218,11 @@ void tools::MinGW::Linker::ConstructJob(Compilation &C, 
const JobAction &JA,
   
 AddLinkerInputs(TC, Inputs, Args, CmdArgs, JA);
   
  +  if (C.getDriver().IsFlangMode()) {
  +tools::addFortranRuntimeLibraryPath(TC, Args, CmdArgs);
  +tools::addFortranRuntimeLibs(TC, CmdArgs);
  +  }
  +
 // TODO: Add profile stuff here
   
 if (TC.ShouldLinkCXXStdlib(Args)) {
  -- 
  2.35.3.windows.1

The Hello World program compiled and linked successfully with an additional 
`-lc++` switch:

  $ PATH=./pkg/bin:$PATH flang-new hello.f90 -L/clang64/lib -lc++
  $ ./a.exe
   Hello, World!


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

https://reviews.llvm.org/D126291

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

[PATCH] D126291: [flang][Driver] Update link job on windows

[Markus Mützel via Phabricator via cfe-commits]

mmuetzel added inline comments.



Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:140
+// defined in flang's runtime libraries.
+if (TC.getTriple().isKnownWindowsMSVCEnvironment())
+  CmdArgs.push_back("/subsystem:console");

Is the MSVC toolchain used anywhere else but on Windows?


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

https://reviews.llvm.org/D126291

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

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

[Stefan Gränitz via Phabricator via cfe-commits]

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

My follow-up got delayed, because I hit another bug, which appears to be a 
regression in release 14. This is why I wrote the tests for release/13.x and I 
still have to port them back to mainline, so this is *not yet ready to land*. 
As I don't expect it to cause big changes, however, I though it might be good 
to update the review anyway.

Rebased on release/13.x I am running the tests like this and both are passing:

  > ninja llvm-config llvm-readobj llc FileCheck count not
  > bin\llvm-lit.py --filter=win64-funclet-preisel-intrinsics test\CodeGen\X86
  > ninja clang
  > bin\llvm-lit.py --filter=arc-exceptions-seh tools\clang\test

@rnk What do you think about implementation, naming, etc. Is that ok?

In D124762#3486288 , @rnk wrote:

> Lastly, I think the inliner needs to be taught that these intrinsics need to 
> carry operand bundles, otherwise this problem will arise after optimization. 
> The inliner has the same logic here:
> https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/InlineFunction.cpp#L2311

Thanks for the note. Yes, I can reproduce that. I am not yet sure what's the 
best way to fix it though. Actually, I'd prefer to split it out into a separate 
review, so the two discussions can remain their individual focus. What do you 
think?




Comment at: clang/test/CodeGenObjCXX/arc-exceptions-seh.mm:1
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fobjc-arc 
-fexceptions -fobjc-exceptions -fobjc-runtime=gnustep-2.0 -o - %s | FileCheck %s
+

Note: There is another test without the `-seh` postfix that checks very similar 
situations for CodeGen with the macOS runtime.



Comment at: llvm/lib/CodeGen/WinEHPrepare.cpp:966
 
-if (FuncletBundleOperand == FuncletPad)
+if (!FuncletPad || FuncletBundleOperand == FuncletPad)
   continue;

sgraenitz wrote:
> rnk wrote:
> > Is this change necessary? It seems like it shouldn't be after the clang and 
> > preisel pass changes.
> Yes, apparently it is necessary. There are cases, where 
> `CodeGenFunction::getBundlesForFunclet()` registers a "funclet" bundle 
> operand, but the `FuncletPad` here is null. My guess was it might be due to 
> optimizations?
Might that be due to the inliner issue then? I'd address it in the follow-up 
review if it's ok?



Comment at: llvm/test/CodeGen/X86/win64-funclet-preisel-intrinsics.ll:53
+; At this point the code used to be truncated:
+; CHECK-NOT: int3
+;

Another side-note: Often times Clang just didn't emit anything here, so 
execution would run into whatever code comes next. The `int3` I sometimes got 
might as well have been "accidental" padding.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124762

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

[PATCH] D124762: [WinEHPrepare] Avoid truncation of EH funclets with GNUstep ObjC runtime

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz added a comment.

For illustration, truncations look like this:

F23303573: Screenshot 2022-06-02 at 15.35.19.png 


F23303572: Screenshot 2022-06-02 at 19.06.41.png 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124762

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

[PATCH] D126853: [clang-tidy] `bugprone-use-after-move`: Don't warn on self-moves.

[Nathan James via Phabricator via cfe-commits]

njames93 accepted this revision.
njames93 added a comment.

LGTM.

I feel that this case should produce a warning akin to the no self assignment 
diagnostics, obviously nothing to do with this check though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126853

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

[PATCH] D126719: [clang-cl] Add support for /kernel

[Nico Weber via Phabricator via cfe-commits]

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

Looking pretty good!




Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7620
+   std::vector Arches = {"IA32"};
+   SupportedArches.insert(Arches.begin(), Arches.end());
+ }

Just `SupportedArches.push_back("IA32")` seems a bit simpler.

(Or use append_range, but there's really no need)



Comment at: clang/test/Driver/cl-zc.cpp:47
+// KERNEL-X64-AVX2: error: invalid argument '/arch:AVX2' not allowed with 
'/kernel'
+// KERNEL-X64-AVX512: error: invalid argument '/arch:AVX512' not allowed with 
'/kernel'
+

Tests for interaction of /kernel with /GR and /EH would be nice too, as 
mentioned.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126719

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

[PATCH] D126902: [Attributes] Remove AttrSyntax and migrate uses to AttributeCommonInfo::Syntax (NFC)

[Leonard Grey via Phabricator via cfe-commits]

lgrey updated this revision to Diff 434036.
lgrey added a comment.

Add assert


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

https://reviews.llvm.org/D126902

Files:
  clang/include/clang/Basic/Attributes.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3331,24 +3331,24 @@
 
   OS << "const llvm::Triple &T = Target.getTriple();\n";
   OS << "switch (Syntax) {\n";
-  OS << "case AttrSyntax::GNU:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_GNU:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(GNU, OS, "GNU");
-  OS << "case AttrSyntax::Declspec:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_Declspec:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(Declspec, OS, "Declspec");
-  OS << "case AttrSyntax::Microsoft:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_Microsoft:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(Microsoft, OS, "Microsoft");
-  OS << "case AttrSyntax::Pragma:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_Pragma:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(Pragma, OS, "Pragma");
-  OS << "case AttrSyntax::HLSLSemantic:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_HLSLSemantic:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(HLSLSemantic, OS, "HLSLSemantic");
-  auto fn = [&OS](const char *Spelling, const char *Variety,
+  auto fn = [&OS](const char *Spelling,
   const std::map> &List) {
-OS << "case AttrSyntax::" << Variety << ": {\n";
+OS << "case AttributeCommonInfo::Syntax::AS_" << Spelling << ": {\n";
 // C++11-style attributes are further split out based on the Scope.
 for (auto I = List.cbegin(), E = List.cend(); I != E; ++I) {
   if (I != List.cbegin())
@@ -3363,8 +3363,13 @@
 }
 OS << "\n} break;\n";
   };
-  fn("CXX11", "CXX", CXX);
-  fn("C2x", "C", C2x);
+  fn("CXX11", CXX);
+  fn("C2x", C2x);
+  OS << "case AttributeCommonInfo::Syntax::AS_Keyword:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_ContextSensitiveKeyword:\n";
+  OS << "  llvm_unreachable(\"hasAttribute not supported for keyword\");\n";
+  OS << "  return 0;\n";
+
   OS << "}\n";
 }
 
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -10,15 +10,16 @@
 //
 //===--===//
 
-#include "clang/Parse/Parser.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
+#include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Parse/ParseDiagnostic.h"
+#include "clang/Parse/Parser.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/ParsedTemplate.h"
@@ -4308,16 +4309,17 @@
 
   // Try parsing microsoft attributes
   if (getLangOpts().MicrosoftExt || getLangOpts().HLSL) {
-if (hasAttribute(AttrSyntax::Microsoft, ScopeName, AttrName,
- getTargetInfo(), getLangOpts()))
+if (hasAttribute(AttributeCommonInfo::Syntax::AS_Microsoft, ScopeName,
+ AttrName, getTargetInfo(), getLangOpts()))
   Syntax = ParsedAttr::AS_Microsoft;
   }
 
   // If the attribute isn't known, we will not attempt to parse any
   // arguments.
   if (Syntax != ParsedAttr::AS_Microsoft &&
-  !hasAttribute(LO.CPlusPlus ? AttrSyntax::CXX : AttrSyntax::C, ScopeName,
-AttrName, getTargetInfo(), getLangOpts())) {
+  !hasAttribute(LO.CPlusPlus ? AttributeCommonInfo::Syntax::AS_CXX11
+ : AttributeCommonInfo::Syntax::AS_C2x,
+ScopeName, AttrName, getTargetInfo(), getLangOpts())) {
 if (getLangOpts().MicrosoftExt || getLangOpts().HLSL) {}
 // Eat the left paren, then skip to the ending right paren.
 ConsumeParen();
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -10,16 +10,17 @@
 //
 //===--===//
 
-#include "clang/Parse/Parser.h"
-#include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/AST/ASTContext.h"
 #include "clan

[PATCH] D126291: [flang][Driver] Update link job on windows

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D126291#3555966 , @mmuetzel wrote:

> In case you don't receive notifications on edits. (Please forgive the noise 
> if you do.)

That's not a problem at all!

> After applying this additional patch (not the one in the previous comment):

Ah, so we were looking in the wrong toolchain file to begin with - good catch!

> The Hello World program compiled and linked successfully

🎉

> with an additional `-lc++` switch

What error do you get if you skip `-lc++`? We need to make sure that Flang does 
no depend on the C++ runtime (it's one of the design requirements). I suspect 
that there's other C library that's required and that's pulled together with 
libc++.

Again - many thanks for checking all this!




Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:140
+// defined in flang's runtime libraries.
+if (TC.getTriple().isKnownWindowsMSVCEnvironment())
+  CmdArgs.push_back("/subsystem:console");

mmuetzel wrote:
> Is the MSVC toolchain used anywhere else but on Windows?
No: 
https://github.com/llvm/llvm-project/blob/5fee1799f4d8da59c251e2d04172fc2f387cbe54/llvm/include/llvm/ADT/Triple.h#L576-L578
 ;-) 


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

https://reviews.llvm.org/D126291

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

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

[Tom Honermann via Phabricator via cfe-commits]

tahonermann added a comment.

> Implementation modules are never serialized (-emit-module-interface for an 
> implementation unit is diagnosed and rejected).

Never? Or just not via -emit-module-interface? I would expect to be able to 
serialize via -emit-ast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959

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

[PATCH] D126969: Allow use of an elaborated type specifier in a _Generic association in C++

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: erichkeane, jyknight, clang-language-wg.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

Currently, Clang accepts this code in C mode (where the tag is required to be 
used) but rejects it in C++ mode thinking that the association is defining a 
new type.

  void foo(void) {
struct S { int a; };
_Generic(something, struct S : 1);
  }

Clang thinks this in C++ because it sees `struct S :` when parsing the class 
specifier and decides that must be a type definition (because the colon 
signifies the presence of a base class type). This patch adds a new declarator 
context to represent a `_Generic` association so that we can distinguish these 
situations properly.

Fixes #55562


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126969

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExpr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/generic-selection.c
  clang/test/SemaCXX/generic-selection.cpp

Index: clang/test/SemaCXX/generic-selection.cpp
===
--- clang/test/SemaCXX/generic-selection.cpp
+++ clang/test/SemaCXX/generic-selection.cpp
@@ -69,3 +69,24 @@
   default : 3
 ) == 2, "we had better pick const Test, not Test!"); // C++-specific result
 }
+
+namespace GH55562 {
+struct S { // expected-note {{declared here}}
+  int i;
+};
+
+void func(struct S s) {
+  // We would previously reject this because the parser thought 'struct S :'
+  // was the start of a definition (with a base class specifier); it's not, it
+  // is an elaborated type specifier followed by the association's value and
+  // it should work the same as in C.
+  (void)_Generic(s, struct S : 1);
+
+  // The rest of these cases test that we still produce a reasonable diagnostic
+  // when referencing an unknown type or trying to define a type in other ways.
+  (void)_Generic(s, struct T : 1);// expected-error {{type 'struct T' in generic association incomplete}}
+  (void)_Generic(s, struct U { int a; } : 1); // expected-error {{'U' cannot be defined in a type specifier}}
+  (void)_Generic(s, struct V : S);// expected-error {{'S' does not refer to a value}}
+  (void)_Generic(s, struct W : S { int b; } : 1); // expected-error {{expected '(' for function-style cast or type construction}}
+}
+} // namespace GH55562
Index: clang/test/Sema/generic-selection.c
===
--- clang/test/Sema/generic-selection.c
+++ clang/test/Sema/generic-selection.c
@@ -78,3 +78,11 @@
   default : 3
 ) == 1, "we had better pick struct Test, not const struct Test!"); // C-specific result
 }
+
+void GH55562(void) {
+  // Ensure that you can still define a type within a generic selection
+  // association (despite it not being particularly useful).
+  (void)_Generic(1, struct S { int a; } : 0, default : 0); // ext-warning {{'_Generic' is a C11 extension}}
+  struct S s = { 0 };
+  int i = s.a;
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -3538,6 +3538,7 @@
 break; // auto(x)
   LLVM_FALLTHROUGH;
 case DeclaratorContext::TypeName:
+case DeclaratorContext::Association:
   Error = 15; // Generic
   break;
 case DeclaratorContext::File:
@@ -3648,6 +3649,7 @@
 case DeclaratorContext::ObjCCatch:
 case DeclaratorContext::TemplateArg:
 case DeclaratorContext::TemplateTypeArg:
+case DeclaratorContext::Association:
   DiagID = diag::err_type_defined_in_type_specifier;
   break;
 case DeclaratorContext::Prototype:
@@ -4735,6 +4737,7 @@
 case DeclaratorContext::TypeName:
 case DeclaratorContext::FunctionalCast:
 case DeclaratorContext::RequiresExpr:
+case DeclaratorContext::Association:
   // Don't infer in these contexts.
   break;
 }
@@ -5777,6 +5780,7 @@
 case DeclaratorContext::TrailingReturnVar:
 case DeclaratorContext::TemplateArg:
 case DeclaratorContext::TemplateTypeArg:
+case DeclaratorContext::Association:
   // FIXME: We may want to allow parameter packs in block-literal contexts
   // in the future.
   S.Diag(D.getEllipsisLoc(),
Index: clang/lib/Parse/ParseExpr.cpp
===
--- clang/lib/Parse/ParseExpr.cpp
+++ clang/lib/Parse/ParseExpr.cpp
@@ -3276,7 +3276,7 @@
   Ty = nullptr;
 } else {
   ColonProtectionRAIIObject X(*this);
-  TypeResult TR = ParseTypeName();
+  TypeResult TR = ParseTypeName(nullptr, DeclaratorContext::Association);
   if (TR.isInvalid()) {
 SkipUntil(tok::r_

[PATCH] D126291: [flang][Driver] Update link job on windows

[Markus Mützel via Phabricator via cfe-commits]

mmuetzel added a comment.

The error message I get without `-lc++`:

  ld.lld: error: undefined symbol: std::__1::mutex::lock()
  >>> referenced by 
libFortranRuntime.a(io-api.cpp.obj):(Fortran::runtime::io::IoStatementState* 
Fortran::runtime::io::BeginExternalListIO<(Fortran::runtime::io::Direction)0, 
Fortran::runtime::io::ExternalListIoStatementState>(int, char const*, int))
  >>> referenced by 
libFortranRuntime.a(io-api.cpp.obj):(Fortran::runtime::io::IoStatementState* 
Fortran::runtime::io::BeginExternalListIO<(Fortran::runtime::io::Direction)0, 
Fortran::runtime::io::ExternalListIoStatementState>(int, char const*, int))
  >>> referenced by 
libFortranRuntime.a(io-api.cpp.obj):(Fortran::runtime::io::IoStatementState* 
Fortran::runtime::io::BeginExternalListIO<(Fortran::runtime::io::Direction)1, 
Fortran::runtime::io::ExternalListIoStatementState>(int, char const*, int))
  >>> referenced 36 more times
  
  ld.lld: error: undefined symbol: std::__1::mutex::unlock()
  >>> referenced by 
libFortranRuntime.a(unit.cpp.obj):(Fortran::runtime::io::FlushOutputOnCrash(Fortran::runtime::Terminator
 const&))
  >>> referenced by 
libFortranRuntime.a(unit.cpp.obj):(Fortran::runtime::io::ExternalFileUnit::LookUp(int))
  >>> referenced by 
libFortranRuntime.a(unit.cpp.obj):(Fortran::runtime::io::ExternalFileUnit::GetUnitMap())
  >>> referenced 23 more times
  flang-new: error: linker command failed with exit code 1 (use -v to see 
invocation)




Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:140
+// defined in flang's runtime libraries.
+if (TC.getTriple().isKnownWindowsMSVCEnvironment())
+  CmdArgs.push_back("/subsystem:console");

awarzynski wrote:
> mmuetzel wrote:
> > Is the MSVC toolchain used anywhere else but on Windows?
> No: 
> https://github.com/llvm/llvm-project/blob/5fee1799f4d8da59c251e2d04172fc2f387cbe54/llvm/include/llvm/ADT/Triple.h#L576-L578
>  ;-) 
In that case, this argument could probably be added unconditionally.


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

https://reviews.llvm.org/D126291

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

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 434040.
serge-sans-paille added a comment.

Fix typo


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

https://reviews.llvm.org/D126864

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Driver/Options.td
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/CodeGen/object-size-flex-array.c

Index: clang/test/CodeGen/object-size-flex-array.c
===
--- /dev/null
+++ clang/test/CodeGen/object-size-flex-array.c
@@ -0,0 +1,60 @@
+// RUN: %clang -fstrict-flex-arrays-target x86_64-apple-darwin -S -emit-llvm %s -o - 2>&1 | FileCheck --check-prefix CHECK-STRICT %s
+// RUN: %clang -fno-strict-flex-arrays -target x86_64-apple-darwin -S -emit-llvm %s -o - 2>&1 | FileCheck %s
+
+#ifndef DYNAMIC
+#define OBJECT_SIZE_BUILTIN __builtin_object_size
+#else
+#define OBJECT_SIZE_BUILTIN __builtin_dynamic_object_size
+#endif
+
+typedef struct {
+  float f;
+  double c[];
+} foo_t;
+
+typedef struct {
+  float f;
+  double c[0];
+} foo0_t;
+
+typedef struct {
+  float f;
+  double c[1];
+} foo1_t;
+
+typedef struct {
+  float f;
+  double c[2];
+} foo2_t;
+
+// CHECK-LABEL: @bar
+// CHECK-STRICT-LABEL: @bar
+unsigned bar(foo_t *f) {
+  // CHECK: ret i32 %
+  // CHECK-STRICT: ret i32 %
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
+
+// CHECK-LABEL: @bar0
+// CHECK-STRICT-LABEL: @bar
+unsigned bar0(foo0_t *f) {
+  // CHECK: ret i32 %
+  // CHECK-STRICT: ret i32 0
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
+
+// CHECK-LABEL: @bar1
+// CHECK-STRICT-LABEL: @bar
+unsigned bar1(foo1_t *f) {
+  // CHECK: ret i32 %
+  // CHECK-STRICT: ret i32 8
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
+
+// CHECK-LABEL: @bar2
+// CHECK-STRICT-LABEL: @bar
+unsigned bar2(foo2_t *f) {
+  // CHECK: ret i32 %
+  // CHECK-STRICT: ret i32 16
+  return OBJECT_SIZE_BUILTIN(f->c, 1);
+}
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -6214,6 +6214,9 @@
   Args.AddLastArg(CmdArgs, options::OPT_funroll_loops,
   options::OPT_fno_unroll_loops);
 
+  Args.addOptInFlag(CmdArgs, options::OPT_fstrict_flex_arrays,
+options::OPT_fno_strict_flex_arrays);
+
   Args.AddLastArg(CmdArgs, options::OPT_pthread);
 
   if (Args.hasFlag(options::OPT_mspeculative_load_hardening,
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11600,6 +11600,8 @@
   return LVal.InvalidBase &&
  Designator.Entries.size() == Designator.MostDerivedPathLength &&
  Designator.MostDerivedIsArrayElement &&
+ (Designator.isMostDerivedAnUnsizedArray() ||
+  !Ctx.getLangOpts().StrictFlexArrays) &&
  isDesignatorAtObjectEnd(Ctx, LVal);
 }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1132,6 +1132,10 @@
 def fapple_kext : Flag<["-"], "fapple-kext">, Group, Flags<[CC1Option]>,
   HelpText<"Use Apple's kernel extensions ABI">,
   MarshallingInfoFlag>;
+defm strict_flex_arrays : BoolFOption<"strict-flex-arrays",
+  LangOpts<"StrictFlexArrays">, DefaultFalse,
+  PosFlag,
+  NegFlag>;
 defm apple_pragma_pack : BoolFOption<"apple-pragma-pack",
   LangOpts<"ApplePragmaPack">, DefaultFalse,
   PosFlag,
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -422,6 +422,7 @@
 LANGOPT(RegisterStaticDestructors, 1, 1, "Register C++ static destructors")
 
 LANGOPT(MatrixTypes, 1, 0, "Enable or disable the builtin matrix type")
+LANGOPT(StrictFlexArrays, 1, 0, "Rely on strict definition of flexible arrays")
 
 COMPATIBLE_VALUE_LANGOPT(MaxTokens, 32, 0, "Max number of tokens per TU or 0")
 
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -68,6 +68,11 @@
 
   Randomizing structure layout is a C-only feature.
 
+- Clang now supports the ``-fstrict-flex-arrays`` to only consider trailing
+  arrays with unknown size arrays as flexible arrays. This breaks compatibility
+  with some legacy code but allows for better folding of
+  ``__builtin_object_size``.
+
 Bug Fixes
 -
 - ``CXXNewExpr::getArraySize()`` previously returned a ``llvm::Optional``
Index: clang/docs/ClangCommandLineReference.rst
===
--- clang/docs/ClangComm

[PATCH] D126959: [C++20][Modules] Introduce an implementation module.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126959#3556074 , @tahonermann 
wrote:

>> Implementation modules are never serialized (-emit-module-interface for an 
>> implementation unit is diagnosed and rejected).
>
> Never? Or just not via -emit-module-interface? I would expect to be able to 
> serialize via -emit-ast.

My current intent is that this is a implementation detail (a mechanism for 
maintaining the module purview and type for Sema and CG).

As things stand, I do not think we are set up to handle (de-)serialization of 
two kinds of module with the same name, so I'd prefer to defer
possible extension to allow this for now (not against the principle, just 
trying avoid a rabbit hole that's tangential to the current purpose).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126959

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

[PATCH] D112916: Confusable identifiers detection

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

Thanks @thakis for the post-commit review. I'll give it another try next week.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112916

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

[clang] dd6bcdb - [Attributes] Remove AttrSyntax and migrate uses to AttributeCommonInfo::Syntax (NFC)

[Leonard Grey via cfe-commits]

Author: Leonard Grey
Date: 2022-06-03T12:11:48-04:00
New Revision: dd6bcdbf21716c56d3defd7f4cacddc7befd5de1

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

LOG: [Attributes] Remove AttrSyntax and migrate uses to 
AttributeCommonInfo::Syntax (NFC)

This is setup for allowing hasAttribute to work for plugin-provided attributes

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

Added: 


Modified: 
clang/include/clang/Basic/Attributes.h
clang/lib/Basic/Attributes.cpp
clang/lib/Lex/PPMacroExpansion.cpp
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/utils/TableGen/ClangAttrEmitter.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/Attributes.h 
b/clang/include/clang/Basic/Attributes.h
index 4afb6a1b9ca25..3fc5fbacdb2cb 100644
--- a/clang/include/clang/Basic/Attributes.h
+++ b/clang/include/clang/Basic/Attributes.h
@@ -9,6 +9,7 @@
 #ifndef LLVM_CLANG_BASIC_ATTRIBUTES_H
 #define LLVM_CLANG_BASIC_ATTRIBUTES_H
 
+#include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/TargetInfo.h"
 
@@ -16,28 +17,11 @@ namespace clang {
 
 class IdentifierInfo;
 
-enum class AttrSyntax {
-  /// Is the identifier known as a GNU-style attribute?
-  GNU,
-  /// Is the identifier known as a __declspec-style attribute?
-  Declspec,
-  /// Is the identifier known as a [] Microsoft-style attribute?
-  Microsoft,
-  // Is the identifier known as a C++-style attribute?
-  CXX,
-  // Is the identifier known as a C-style attribute?
-  C,
-  // Is the identifier known as a pragma attribute?
-  Pragma,
-  // Is the identifier known as a HLSL semantic?
-  HLSLSemantic,
-};
-
 /// Return the version number associated with the attribute if we
 /// recognize and implement the attribute specified by the given information.
-int hasAttribute(AttrSyntax Syntax, const IdentifierInfo *Scope,
- const IdentifierInfo *Attr, const TargetInfo &Target,
- const LangOptions &LangOpts);
+int hasAttribute(AttributeCommonInfo::Syntax Syntax,
+ const IdentifierInfo *Scope, const IdentifierInfo *Attr,
+ const TargetInfo &Target, const LangOptions &LangOpts);
 
 } // end namespace clang
 

diff  --git a/clang/lib/Basic/Attributes.cpp b/clang/lib/Basic/Attributes.cpp
index bd27dc13a92f0..960c9773d192a 100644
--- a/clang/lib/Basic/Attributes.cpp
+++ b/clang/lib/Basic/Attributes.cpp
@@ -5,9 +5,9 @@
 #include "llvm/ADT/StringSwitch.h"
 using namespace clang;
 
-int clang::hasAttribute(AttrSyntax Syntax, const IdentifierInfo *Scope,
-const IdentifierInfo *Attr, const TargetInfo &Target,
-const LangOptions &LangOpts) {
+int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
+const IdentifierInfo *Scope, const IdentifierInfo 
*Attr,
+const TargetInfo &Target, const LangOptions &LangOpts) 
{
   StringRef Name = Attr->getName();
   // Normalize the attribute name, __foo__ becomes foo.
   if (Name.size() >= 4 && Name.startswith("__") && Name.endswith("__"))

diff  --git a/clang/lib/Lex/PPMacroExpansion.cpp 
b/clang/lib/Lex/PPMacroExpansion.cpp
index 07ee07e20bb2d..063199b0e6ce5 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -11,6 +11,7 @@
 //
 
//===--===//
 
+#include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/FileManager.h"
@@ -1689,8 +1690,9 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
   [this](Token &Tok, bool &HasLexedNextToken) -> int {
 IdentifierInfo *II = ExpectFeatureIdentifierInfo(Tok, *this,
diag::err_feature_check_malformed);
-return II ? hasAttribute(AttrSyntax::GNU, nullptr, II,
- getTargetInfo(), getLangOpts()) : 0;
+return II ? hasAttribute(AttributeCommonInfo::Syntax::AS_GNU, nullptr,
+ II, getTargetInfo(), getLangOpts())
+  : 0;
   });
   } else if (II == Ident__has_declspec) {
 EvaluateFeatureLikeBuiltinMacro(OS, Tok, II, *this, true,
@@ -1700,8 +1702,8 @@ void Preprocessor::ExpandBuiltinMacro(Token &Tok) {
 if (II) {
   const LangOptions &LangOpts = getLangOpts();
   return LangOpts.DeclSpecKeyword &&
- hasAttribute(AttrSyntax::Declspec, nullptr, II,
-  getTargetInfo(), LangOpts);
+ hasAttribute(AttributeCommonInfo::Syntax::AS_Declspec, 
nullptr,
+  II, getTargetInfo(), LangOpts);
   

[PATCH] D126902: [Attributes] Remove AttrSyntax and migrate uses to AttributeCommonInfo::Syntax (NFC)

[Leonard Grey via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGdd6bcdbf2171: [Attributes] Remove AttrSyntax and migrate 
uses to AttributeCommonInfo::Syntax… (authored by lgrey).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126902

Files:
  clang/include/clang/Basic/Attributes.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Lex/PPMacroExpansion.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/utils/TableGen/ClangAttrEmitter.cpp

Index: clang/utils/TableGen/ClangAttrEmitter.cpp
===
--- clang/utils/TableGen/ClangAttrEmitter.cpp
+++ clang/utils/TableGen/ClangAttrEmitter.cpp
@@ -3331,24 +3331,24 @@
 
   OS << "const llvm::Triple &T = Target.getTriple();\n";
   OS << "switch (Syntax) {\n";
-  OS << "case AttrSyntax::GNU:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_GNU:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(GNU, OS, "GNU");
-  OS << "case AttrSyntax::Declspec:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_Declspec:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(Declspec, OS, "Declspec");
-  OS << "case AttrSyntax::Microsoft:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_Microsoft:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(Microsoft, OS, "Microsoft");
-  OS << "case AttrSyntax::Pragma:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_Pragma:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(Pragma, OS, "Pragma");
-  OS << "case AttrSyntax::HLSLSemantic:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_HLSLSemantic:\n";
   OS << "  return llvm::StringSwitch(Name)\n";
   GenerateHasAttrSpellingStringSwitch(HLSLSemantic, OS, "HLSLSemantic");
-  auto fn = [&OS](const char *Spelling, const char *Variety,
+  auto fn = [&OS](const char *Spelling,
   const std::map> &List) {
-OS << "case AttrSyntax::" << Variety << ": {\n";
+OS << "case AttributeCommonInfo::Syntax::AS_" << Spelling << ": {\n";
 // C++11-style attributes are further split out based on the Scope.
 for (auto I = List.cbegin(), E = List.cend(); I != E; ++I) {
   if (I != List.cbegin())
@@ -3363,8 +3363,13 @@
 }
 OS << "\n} break;\n";
   };
-  fn("CXX11", "CXX", CXX);
-  fn("C2x", "C", C2x);
+  fn("CXX11", CXX);
+  fn("C2x", C2x);
+  OS << "case AttributeCommonInfo::Syntax::AS_Keyword:\n";
+  OS << "case AttributeCommonInfo::Syntax::AS_ContextSensitiveKeyword:\n";
+  OS << "  llvm_unreachable(\"hasAttribute not supported for keyword\");\n";
+  OS << "  return 0;\n";
+
   OS << "}\n";
 }
 
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -10,15 +10,16 @@
 //
 //===--===//
 
-#include "clang/Parse/Parser.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/PrettyDeclStackTrace.h"
+#include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/Attributes.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/Parse/ParseDiagnostic.h"
+#include "clang/Parse/Parser.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/ParsedTemplate.h"
@@ -4308,16 +4309,17 @@
 
   // Try parsing microsoft attributes
   if (getLangOpts().MicrosoftExt || getLangOpts().HLSL) {
-if (hasAttribute(AttrSyntax::Microsoft, ScopeName, AttrName,
- getTargetInfo(), getLangOpts()))
+if (hasAttribute(AttributeCommonInfo::Syntax::AS_Microsoft, ScopeName,
+ AttrName, getTargetInfo(), getLangOpts()))
   Syntax = ParsedAttr::AS_Microsoft;
   }
 
   // If the attribute isn't known, we will not attempt to parse any
   // arguments.
   if (Syntax != ParsedAttr::AS_Microsoft &&
-  !hasAttribute(LO.CPlusPlus ? AttrSyntax::CXX : AttrSyntax::C, ScopeName,
-AttrName, getTargetInfo(), getLangOpts())) {
+  !hasAttribute(LO.CPlusPlus ? AttributeCommonInfo::Syntax::AS_CXX11
+ : AttributeCommonInfo::Syntax::AS_C2x,
+ScopeName, AttrName, getTargetInfo(), getLangOpts())) {
 if (getLangOpts().MicrosoftExt || getLangOpts().HLSL) {}
 // Eat the left paren, then skip to the ending right paren.
 ConsumeParen();
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -10,16 +10,17 @@
 //
 //===

[PATCH] D126969: Allow use of an elaborated type specifier in a _Generic association in C++

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

This mostly/all seems reasonable to me, and the diagnostics, while not perfect, 
are IMO a vast improvement.




Comment at: clang/include/clang/Sema/DeclSpec.h:2068
 case DeclaratorContext::TrailingReturnVar:
+case DeclaratorContext::Association:
   return false;

Is this right?  According to the comment, this is 'true'if the identifier is 
optional or required, but the mayOmitIdentifier, where you are returning 'true' 
says optional or not allowed?  Does this make identifier 'not allowed' here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126969

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

[PATCH] D126972: [clang][dataflow] Modify optional-checker to handle type aliases.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Previously, type aliases were not handled (and resulted in an assertion
firing). This patch generalizes the code to consider aliases everywhere (a
previous patch already considered aliases for optional-returning functions).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126972

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


Index: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2214,6 +2214,23 @@
   UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
 }
 
+// Verifies that the check sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+template 
+using MyOptional = $ns::$optional;
+
+void target(MyOptional opt) {
+  opt.value();
+  /*[[check]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+}
+
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -43,13 +43,14 @@
   hasTemplateArgument(0, refersToType(type().bind("T";
 }
 
-auto hasOptionalType() { return hasType(optionalClass()); }
-
-auto hasOptionalOrAliasType() {
+auto optionalOrAliasType() {
   return hasUnqualifiedDesugaredType(
   recordType(hasDeclaration(optionalClass(;
 }
 
+/// Matches any of the spellings of the optional types and sugar, aliases, etc.
+auto hasOptionalType() { return hasType(optionalOrAliasType()); }
+
 auto isOptionalMemberCallWithName(
 llvm::StringRef MemberName,
 llvm::Optional Ignorable = llvm::None) {
@@ -164,9 +165,8 @@
 }
 
 auto isCallReturningOptional() {
-  return callExpr(callee(functionDecl(
-  returns(anyOf(hasOptionalOrAliasType(),
-referenceType(pointee(hasOptionalOrAliasType(;
+  return callExpr(callee(functionDecl(returns(anyOf(
+  optionalOrAliasType(), 
referenceType(pointee(optionalOrAliasType(;
 }
 
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
@@ -485,8 +485,9 @@
   return MatchSwitchBuilder()
   // Attach a symbolic "has_value" state to optional values that we see for
   // the first time.
-  .CaseOf(expr(anyOf(declRefExpr(), memberExpr()), 
hasOptionalType()),
-initializeOptionalReference)
+  .CaseOf(
+  expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()),
+  initializeOptionalReference)
 
   // make_optional
   .CaseOf(isMakeOptionalCall(), transferMakeOptionalCall)


Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2214,6 +2214,23 @@
   UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
 }
 
+// Verifies that the check sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+template 
+using MyOptional = $ns::$optional;
+
+void target(MyOptional opt) {
+  opt.value();
+  /*[[check]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+}
+
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -43,13 +43,14 @@
   hasTemplateArgument(0, refersToType(type().bind("T";
 }
 
-auto hasOptionalType() { return hasType(optionalClass()); }
-
-auto hasOptionalOrAliasType() {
+auto optionalOrAliasType() {
   return hasUnqualifiedDesugaredType(
   recordType(hasDeclaration(option

[PATCH] D126972: [clang][dataflow] Modify optional-checker to handle type aliases.

[Stanislav Gatev via Phabricator via cfe-commits]

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



Comment at: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp:2217
 
+// Verifies that the check sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {

Let's replace "check"/"checker" with "model" here and in the commit message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126972

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

[PATCH] D126972: [clang][dataflow] Modify optional-checker to handle type aliases.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 434048.
ymandel marked an inline comment as done.
ymandel added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126972

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


Index: 
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2214,6 +2214,23 @@
   UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
 }
 
+// Verifies that the model sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+template 
+using MyOptional = $ns::$optional;
+
+void target(MyOptional opt) {
+  opt.value();
+  /*[[check]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+}
+
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -43,13 +43,14 @@
   hasTemplateArgument(0, refersToType(type().bind("T";
 }
 
-auto hasOptionalType() { return hasType(optionalClass()); }
-
-auto hasOptionalOrAliasType() {
+auto optionalOrAliasType() {
   return hasUnqualifiedDesugaredType(
   recordType(hasDeclaration(optionalClass(;
 }
 
+/// Matches any of the spellings of the optional types and sugar, aliases, etc.
+auto hasOptionalType() { return hasType(optionalOrAliasType()); }
+
 auto isOptionalMemberCallWithName(
 llvm::StringRef MemberName,
 llvm::Optional Ignorable = llvm::None) {
@@ -164,9 +165,8 @@
 }
 
 auto isCallReturningOptional() {
-  return callExpr(callee(functionDecl(
-  returns(anyOf(hasOptionalOrAliasType(),
-referenceType(pointee(hasOptionalOrAliasType(;
+  return callExpr(callee(functionDecl(returns(anyOf(
+  optionalOrAliasType(), 
referenceType(pointee(optionalOrAliasType(;
 }
 
 /// Creates a symbolic value for an `optional` value using `HasValueVal` as the
@@ -485,8 +485,9 @@
   return MatchSwitchBuilder()
   // Attach a symbolic "has_value" state to optional values that we see for
   // the first time.
-  .CaseOf(expr(anyOf(declRefExpr(), memberExpr()), 
hasOptionalType()),
-initializeOptionalReference)
+  .CaseOf(
+  expr(anyOf(declRefExpr(), memberExpr()), hasOptionalType()),
+  initializeOptionalReference)
 
   // make_optional
   .CaseOf(isMakeOptionalCall(), transferMakeOptionalCall)


Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2214,6 +2214,23 @@
   UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
 }
 
+// Verifies that the model sees through aliases.
+TEST_P(UncheckedOptionalAccessTest, WithAlias) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+template 
+using MyOptional = $ns::$optional;
+
+void target(MyOptional opt) {
+  opt.value();
+  /*[[check]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+}
+
 // FIXME: Add support for:
 // - constructors (copy, move)
 // - assignment operators (default, copy, move)
Index: clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
===
--- clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -43,13 +43,14 @@
   hasTemplateArgument(0, refersToType(type().bind("T";
 }
 
-auto hasOptionalType() { return hasType(optionalClass()); }
-
-auto hasOptionalOrAliasType() {
+auto optionalOrAliasType() {
   return hasUnqualifiedDesugaredType(
   recordType(hasDeclaration(optionalClass(;
 }
 
+/// Matches any of the spellings of the optional types and sugar, aliases, etc.
+auto hasOptionalType() { return hasType(optionalOrAliasType()); }
+
 auto isOptionalMemberCallWithName(
 llvm::StringRef MemberName,
 llvm::Optional Ignorable = llvm::None

[PATCH] D126972: [clang][dataflow] Modify `optional` model to handle type aliases.

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

xazax.hun accepted this revision.
xazax.hun added a comment.

This patch looks good to me. On the other hand, I was wondering whether 
functions like this would be handled:

  pair f();
  array g();
  MyStructWithOptionals h();

And so on. In general, I wonder if it is a better approach to be able to plug 
the optional creation function into the general value creation function of the 
engine. So whenever the engine encounters an optional type, it could create the 
corresponding value. It could do it lazily or eagerly, however it choses. And 
the user would no longer need to match all the possible ways an optional type 
can be mentioned somewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126972

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

[PATCH] D126973: [clang][dataflow] Relax assumption that `AggregateStorageLocations` correspond to struct type.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

Currently, when an `AggregateStorageLocation` is mapped to a `StructValue`, we
enforce that it's type is a struct or class. However, we don't need this to hold
and there are reasonable violations of this assumption -- for example, using the
`StructValue` for its properties, while modeling a non-struct type.

This patch removes the assertion.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D126973

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp


Index: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -10,6 +10,7 @@
 #include "NoopAnalysis.h"
 #include "TestingSupport.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "gmock/gmock.h"
@@ -20,8 +21,7 @@
 
 using namespace clang;
 using namespace dataflow;
-using ::testing::ElementsAre;
-using ::testing::Pair;
+using ::testing::NotNull;
 
 class EnvironmentTest : public ::testing::Test {
   DataflowAnalysisContext Context;
@@ -98,4 +98,25 @@
   EXPECT_NE(PV, nullptr);
 }
 
+TEST_F(EnvironmentTest, SetValueAdmitsNonStructAggregateStorageLocations) {
+  using namespace ast_matchers;
+
+  std::string Code = "bool X;";
+  auto Unit =
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  const auto *Ty = selectFirst(
+  "target", match(qualType(builtinType()).bind("target"), Context));
+  ASSERT_TRUE(Ty != nullptr && !Ty->isNull());
+
+  // Create an `AggregateStorageLocation` that is *not* backed by a struct.
+  auto &Loc =
+  Env.takeOwnership(std::make_unique(*Ty));
+  auto &V = Env.takeOwnership(std::make_unique());
+  Env.setValue(Loc, V);
+  EXPECT_EQ(Env.getValue(Loc), &V);
+}
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -374,11 +374,7 @@
 
   if (auto *StructVal = dyn_cast(&Val)) {
 auto &AggregateLoc = *cast(&Loc);
-
-const QualType Type = AggregateLoc.getType();
-assert(Type->isStructureOrClassType());
-
-for (const FieldDecl *Field : getObjectFields(Type)) {
+for (const FieldDecl *Field : getObjectFields(AggregateLoc.getType())) {
   assert(Field != nullptr);
   StorageLocation &FieldLoc = AggregateLoc.getChild(*Field);
   MemberLocToStruct[&FieldLoc] = std::make_pair(StructVal, Field);


Index: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -10,6 +10,7 @@
 #include "NoopAnalysis.h"
 #include "TestingSupport.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
 #include "gmock/gmock.h"
@@ -20,8 +21,7 @@
 
 using namespace clang;
 using namespace dataflow;
-using ::testing::ElementsAre;
-using ::testing::Pair;
+using ::testing::NotNull;
 
 class EnvironmentTest : public ::testing::Test {
   DataflowAnalysisContext Context;
@@ -98,4 +98,25 @@
   EXPECT_NE(PV, nullptr);
 }
 
+TEST_F(EnvironmentTest, SetValueAdmitsNonStructAggregateStorageLocations) {
+  using namespace ast_matchers;
+
+  std::string Code = "bool X;";
+  auto Unit =
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  const auto *Ty = selectFirst(
+  "target", match(qualType(builtinType()).bind("target"), Context));
+  ASSERT_TRUE(Ty != nullptr && !Ty->isNull());
+
+  // Create an `AggregateStorageLocation` that is *not* backed by a struct.
+  auto &Loc =
+  Env.takeOwnership(std::make_unique(*Ty));
+  auto &V = Env.takeOwnership(std::make_unique());
+  Env.setValue(Loc, V);
+  EXPECT_EQ(Env.getValue(Loc), &V);
+}
 } // namespace
Index: clang/lib/Analysis/

[PATCH] D126864: [clang] Introduce -fstrict-flex-arrays for stricter handling of flexible arrays

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Probably IsTailPaddedMemberArray() in SemaChecking.cpp and 
isFlexibleArrayMemberExpr() in CGExpr.cpp should also check for this flag.  Not 
sure if there's anything else that does similar checks; the static analyzer has 
its own flag consider-single-element-arrays-as-flexible-array-members, but I 
think that's disabled by default anyway.

I'm a little concerned about the premise of this, though.  See 
https://github.com/llvm/llvm-project/issues/29694 for why we relaxed this check 
in the first place.  I mean, the Linux kernel itself can maybe ensure it isn't 
doing anything silly, but most code has to deal with system headers, which are 
apparently broken.  So this option is a trap for most code.


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

https://reviews.llvm.org/D126864

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