[PATCH] D151042: [clang] Add tests for CWG issues 977, 1482, 2516

[Shafik Yaghmour via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151042

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

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added a comment.

In D151047#4361205 , @Sedeniono wrote:

> Regarding name and mail: If you use "Sedenion" as name and 
> "39583823+sedeni...@users.noreply.github.com" as mail, I guess github will 
> attribute the commit to my github account ?

See D150403#4358845 .

> Also, should it be merged into the LLVM 16.x branch?

+1. @MyDeveloperDay and @HazardyKnusperkeks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047

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

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6dcde658b238: This is a retry of 
https://reviews.llvm.org/D114583, which was backed (authored by galenelias, 
committed by owenpan).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150403

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -40,6 +40,8 @@
   EXPECT_EQ((FormatTok)->getType(), Type) << *(FormatTok)
 #define EXPECT_TOKEN_PRECEDENCE(FormatTok, Prec)   \
   EXPECT_EQ((FormatTok)->getPrecedence(), Prec) << *(FormatTok)
+#define EXPECT_BRACE_KIND(FormatTok, Kind) \
+  EXPECT_EQ(FormatTok->getBlockKind(), Kind) << *(FormatTok)
 #define EXPECT_TOKEN(FormatTok, Kind, Type)\
   do { \
 EXPECT_TOKEN_KIND(FormatTok, Kind);\
@@ -1800,6 +1802,22 @@
   EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsNestedBlocks) {
+  // The closing braces are not annotated. It doesn't seem to cause a problem.
+  // So we only test for the opening braces.
+  auto Tokens = annotate("{\n"
+ "  {\n"
+ "{ int a = 0; }\n"
+ "  }\n"
+ "  {}\n"
+ "}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_BRACE_KIND(Tokens[0], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[1], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[2], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[10], BK_Block);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13732,6 +13732,26 @@
"  struct Dummy {};\n"
"  f(v);\n"
"}");
+  verifyFormat("void foo() {\n"
+   "  { // asdf\n"
+   "{ int a; }\n"
+   "  }\n"
+   "  {\n"
+   "{ int b; }\n"
+   "  }\n"
+   "}");
+  verifyFormat("namespace n {\n"
+   "void foo() {\n"
+   "  {\n"
+   "{\n"
+   "  statement();\n"
+   "  if (false) {\n"
+   "  }\n"
+   "}\n"
+   "  }\n"
+   "  {}\n"
+   "}\n"
+   "} // namespace n");
 
   // Long lists should be formatted in columns even if they are nested.
   verifyFormat(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -491,7 +491,11 @@
   // Keep a stack of positions of lbrace tokens. We will
   // update information about whether an lbrace starts a
   // braced init list or a different block during the loop.
-  SmallVector LBraceStack;
+  struct StackEntry {
+FormatToken *Tok;
+const FormatToken *PrevTok;
+  };
+  SmallVector LBraceStack;
   assert(Tok->is(tok::l_brace));
   do {
 // Get next non-comment token.
@@ -521,12 +525,12 @@
   } else {
 Tok->setBlockKind(BK_Unknown);
   }
-  LBraceStack.push_back(Tok);
+  LBraceStack.push_back({Tok, PrevTok});
   break;
 case tok::r_brace:
   if (LBraceStack.empty())
 break;
-  if (LBraceStack.back()->is(BK_Unknown)) {
+  if (LBraceStack.back().Tok->is(BK_Unknown)) {
 bool ProbablyBracedList = false;
 if (Style.Language == FormatStyle::LK_Proto) {
   ProbablyBracedList = NextTok->isOneOf(tok::comma, tok::r_square);
@@ -554,7 +558,7 @@
 
   // If we already marked the opening brace as braced list, the closing
   // must also be part of it.
-  ProbablyBracedList = LBraceStack.back()->is(TT_BracedListLBrace);
+  ProbablyBracedList = LBraceStack.back().Tok->is(TT_BracedListLBrace);
 
   ProbablyBracedList = ProbablyBracedList ||
(Style.isJavaScript() &&
@@ -570,8 +574,14 @@
   ProbablyBracedList =
   ProbablyBracedList ||
   NextTok->isOneOf(tok::comma, tok::period, tok::colon,
-   tok::r_paren, tok::r_square, tok::l_brace,
-   tok::ellipsis);
+   tok::r_paren, 

[clang] 6dcde65 - This is a retry of https://reviews.llvm.org/D114583, which was backed

[Owen Pan via cfe-commits]

Author: Galen Elias
Date: 2023-05-22T20:25:55-07:00
New Revision: 6dcde658b2380d7ca1451ea5d1099af3e294ea16

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

LOG: This is a retry of https://reviews.llvm.org/D114583, which was backed
out for regressions.

Clang Format is detecting a nested scope followed by another open brace
as a braced initializer list due to incorrectly thinking it's matching a
braced initializer at the end of a constructor initializer list which is
followed by the body open brace.

Unfortunately, UnwrappedLineParser isn't doing a very detailed parse, so
it's not super straightforward to distinguish these cases given the
current structure of calculateBraceTypes. My current hypothesis is that
these can be disambiguated by looking at the token preceding the
l_brace, as initializer list parameters will be preceded by an
identifier, but a scope block generally will not (barring the MACRO
wildcard).

To this end, I am adding tracking of the previous token to the LBraceStack
to help scope this particular case.

TokenAnnotatorTests cherry picked from https://reviews.llvm.org/D150452.

Fixes #33891.
Fixes #52911.

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

Added: 


Modified: 
clang/lib/Format/UnwrappedLineParser.cpp
clang/unittests/Format/FormatTest.cpp
clang/unittests/Format/TokenAnnotatorTest.cpp

Removed: 




diff  --git a/clang/lib/Format/UnwrappedLineParser.cpp 
b/clang/lib/Format/UnwrappedLineParser.cpp
index 31b45fa7cc898..66c1205757257 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -491,7 +491,11 @@ void UnwrappedLineParser::calculateBraceTypes(bool 
ExpectClassBody) {
   // Keep a stack of positions of lbrace tokens. We will
   // update information about whether an lbrace starts a
   // braced init list or a 
diff erent block during the loop.
-  SmallVector LBraceStack;
+  struct StackEntry {
+FormatToken *Tok;
+const FormatToken *PrevTok;
+  };
+  SmallVector LBraceStack;
   assert(Tok->is(tok::l_brace));
   do {
 // Get next non-comment token.
@@ -521,12 +525,12 @@ void UnwrappedLineParser::calculateBraceTypes(bool 
ExpectClassBody) {
   } else {
 Tok->setBlockKind(BK_Unknown);
   }
-  LBraceStack.push_back(Tok);
+  LBraceStack.push_back({Tok, PrevTok});
   break;
 case tok::r_brace:
   if (LBraceStack.empty())
 break;
-  if (LBraceStack.back()->is(BK_Unknown)) {
+  if (LBraceStack.back().Tok->is(BK_Unknown)) {
 bool ProbablyBracedList = false;
 if (Style.Language == FormatStyle::LK_Proto) {
   ProbablyBracedList = NextTok->isOneOf(tok::comma, tok::r_square);
@@ -554,7 +558,7 @@ void UnwrappedLineParser::calculateBraceTypes(bool 
ExpectClassBody) {
 
   // If we already marked the opening brace as braced list, the closing
   // must also be part of it.
-  ProbablyBracedList = LBraceStack.back()->is(TT_BracedListLBrace);
+  ProbablyBracedList = LBraceStack.back().Tok->is(TT_BracedListLBrace);
 
   ProbablyBracedList = ProbablyBracedList ||
(Style.isJavaScript() &&
@@ -570,8 +574,14 @@ void UnwrappedLineParser::calculateBraceTypes(bool 
ExpectClassBody) {
   ProbablyBracedList =
   ProbablyBracedList ||
   NextTok->isOneOf(tok::comma, tok::period, tok::colon,
-   tok::r_paren, tok::r_square, tok::l_brace,
-   tok::ellipsis);
+   tok::r_paren, tok::r_square, tok::ellipsis);
+
+  // Distinguish between braced list in a constructor initializer list
+  // followed by constructor body, or just adjacent blocks.
+  ProbablyBracedList =
+  ProbablyBracedList ||
+  (NextTok->is(tok::l_brace) && LBraceStack.back().PrevTok &&
+   LBraceStack.back().PrevTok->is(tok::identifier));
 
   ProbablyBracedList =
   ProbablyBracedList ||
@@ -595,10 +605,10 @@ void UnwrappedLineParser::calculateBraceTypes(bool 
ExpectClassBody) {
 }
 if (ProbablyBracedList) {
   Tok->setBlockKind(BK_BracedInit);
-  LBraceStack.back()->setBlockKind(BK_BracedInit);
+  LBraceStack.back().Tok->setBlockKind(BK_BracedInit);
 } else {
   Tok->setBlockKind(BK_Block);
-  LBraceStack.back()->setBlockKind(BK_Block);
+  LBraceStack.back().Tok->setBlockKind(BK_Block);
 }
   }
   LBraceStack.pop_back();
@@ -615,8 +625,8 @@ void UnwrappedLineParser::calculateBraceTypes(bool 
ExpectClassBody) {
 case tok::kw_switch:
 case tok::kw_try:
 case tok::kw___try:
-  if 

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

[Owen Pan via Phabricator via cfe-commits]

owenpan accepted this revision.
owenpan added a comment.

In D150403#4362403 , @galenelias 
wrote:

> Galen Elias 
>
> I'm not sure if there is somewhere I should be putting that in the summary or 
> diff itself, or just here in the comments?

Just here in the comments would be enough.


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

https://reviews.llvm.org/D150403

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

[clang] 52bc4b1 - [NFC] [C++20] [Modules] Refactor Sema::isModuleUnitOfCurrentTU into

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2023-05-23T10:52:22+08:00
New Revision: 52bc4b16cb68d6d64c0d9499b2e6c1d719e78085

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

LOG: [NFC] [C++20] [Modules] Refactor Sema::isModuleUnitOfCurrentTU into
Decl::isInAnotherModuleUnit

Refactor `Sema::isModuleUnitOfCurrentTU` to `Decl::isInAnotherModuleUnit`
to make code simpler a little bit. Note that although this patch
introduces a FIXME, this is an existing issue and this patch just tries
to describe it explicitly.

Added: 
clang/test/Modules/redundant-template-default-arg4.cpp
clang/test/Modules/redundant-template-default-arg5.cpp

Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/DeclBase.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaLookup.cpp
clang/lib/Sema/SemaModule.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplate.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index bc4a0df296d71..40cadd93158c6 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -448,7 +448,7 @@ class ASTContext : public RefCountedBase {
   llvm::DenseMap ModuleInitializers;
 
   /// This is the top-level (C++20) Named module we are building.
-   Module *CurrentCXXNamedModule = nullptr;
+  Module *CurrentCXXNamedModule = nullptr;
 
   static constexpr unsigned ConstantArrayTypesLog2InitSize = 8;
   static constexpr unsigned GeneralTypesLog2InitSize = 9;

diff  --git a/clang/include/clang/AST/DeclBase.h 
b/clang/include/clang/AST/DeclBase.h
index e0dc6dab7b334..f7d5b3a83141a 100644
--- a/clang/include/clang/AST/DeclBase.h
+++ b/clang/include/clang/AST/DeclBase.h
@@ -644,6 +644,9 @@ class alignas(8) Decl {
 return getModuleOwnershipKind() > ModuleOwnershipKind::VisibleWhenImported;
   }
 
+  /// Whether this declaration comes from another module unit.
+  bool isInAnotherModuleUnit() const;
+
   /// FIXME: Implement discarding declarations actually in global module
   /// fragment. See [module.global.frag]p3,4 for details.
   bool isDiscardedInGlobalModuleFragment() const { return false; }

diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index bc6fb74df4d62..e869117a8ea66 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -2364,9 +2364,6 @@ class Sema final {
 return Entity->getOwningModule();
   }
 
-  // Determine whether the module M belongs to the  current TU.
-  bool isModuleUnitOfCurrentTU(const Module *M) const;
-
   /// Make a merged definition of an existing hidden definition \p ND
   /// visible at the specified location.
   void makeMergedDefinitionVisible(NamedDecl *ND);

diff  --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index ad1e940c4bda6..2307c33c5900c 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1155,7 +1155,7 @@ ArrayRef ASTContext::getModuleInitializers(Module 
*M) {
 void ASTContext::setCurrentNamedModule(Module *M) {
   assert(M->isModulePurview());
   assert(!CurrentCXXNamedModule &&
-"We should set named module for ASTContext for only once");
+ "We should set named module for ASTContext for only once");
   CurrentCXXNamedModule = M;
 }
 
@@ -11935,9 +11935,7 @@ bool ASTContext::DeclMustBeEmitted(const Decl *D) {
 return false;
 
   // Variables in other module units shouldn't be forced to be emitted.
-  auto *VM = VD->getOwningModule();
-  if (VM && VM->getTopLevelModule()->isModulePurview() &&
-  VM->getTopLevelModule() != getCurrentNamedModule())
+  if (VD->isInAnotherModuleUnit())
 return false;
 
   // Variables that can be needed in other TUs are required.

diff  --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp
index f49945f434193..834beef49a444 100644
--- a/clang/lib/AST/DeclBase.cpp
+++ b/clang/lib/AST/DeclBase.cpp
@@ -30,6 +30,7 @@
 #include "clang/Basic/IdentifierTable.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/LangOptions.h"
+#include "clang/Basic/Module.h"
 #include "clang/Basic/ObjCRuntime.h"
 #include "clang/Basic/PartialDiagnostic.h"
 #include "clang/Basic/SourceLocation.h"
@@ -1022,6 +1023,28 @@ bool Decl::isInExportDeclContext() const {
   return DC && isa(DC);
 }
 
+bool Decl::isInAnotherModuleUnit() const {
+  auto *M = getOwningModule();
+
+  if (!M)
+return false;
+
+  M = M->getTopLevelModule();
+  // FIXME: It is problematic if the header module lives in another module
+  // unit. Consider to fix this by techniques like
+  // ExternalASTSource::hasExternalDefinitions.
+  if (M->isHeaderLikeModule())
+return false;
+
+  // A global module without parent 

[PATCH] D150892: [clang][ExprConstant] fix __builtin_object_size for flexible array members

[Sterling Augustine via Phabricator via cfe-commits]

saugustine added a comment.

This change causes an assertion failure on the following test case. The memset 
is important for some reason.

  saugustine@saugustine-desktop:~/llvm/build $ cat t.c
  struct foo {
unsigned char x;
unsigned char   data[];
  };
  
  extern void *memset (void *__s, int __c, unsigned long __n);
  
  void bar(void)
  {
struct foo bat;
memset(, 0, sizeof(bat));
  }
  saugustine@saugustine-desktop:~/llvm/build $ ./bin/clang -c t.c
  clang: 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/AST/Decl.cpp:2786:
 clang::CharUnits clang::VarDecl::getFlexibleArrayInitChars(const 
clang::ASTContext &) const: Assertion `hasInit() && "Expect initializer to 
check for flexible array init"' failed.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: ./bin/clang -c t.c
  1.t.c:11:30: current parser token ')'
  2.t.c:9:1: parsing function body 'bar'
  3.t.c:9:1: in compound statement ('{}')
   #0 0x55bf947c1d7d llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/usr/local/google/home/saugustine/llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:602:11
   #1 0x55bf947c21fb PrintStackTraceSignalHandler(void*) 
/usr/local/google/home/saugustine/llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:675:1
   #2 0x55bf947c0496 llvm::sys::RunSignalHandlers() 
/usr/local/google/home/saugustine/llvm/llvm-project/llvm/lib/Support/Signals.cpp:104:5
   #3 0x55bf947c15ee llvm::sys::CleanupOnSignal(unsigned long) 
/usr/local/google/home/saugustine/llvm/llvm-project/llvm/lib/Support/Unix/Signals.inc:368:1
   #4 0x55bf946ff954 (anonymous 
namespace)::CrashRecoveryContextImpl::HandleCrash(int, unsigned long) 
/usr/local/google/home/saugustine/llvm/llvm-project/llvm/lib/Support/CrashRecoveryContext.cpp:0:7
   #5 0x55bf946ffd12 CrashRecoverySignalHandler(int) 
/usr/local/google/home/saugustine/llvm/llvm-project/llvm/lib/Support/CrashRecoveryContext.cpp:391:1
   #6 0x7f95ab65af90 (/lib/x86_64-linux-gnu/libc.so.6+0x3bf90)
   #7 0x7f95ab6a9ccc __pthread_kill_implementation 
./nptl/./nptl/pthread_kill.c:44:76
   #8 0x7f95ab65aef2 raise ./signal/../sysdeps/posix/raise.c:27:6
   #9 0x7f95ab645472 abort ./stdlib/./stdlib/abort.c:81:7
  #10 0x7f95ab645395 _nl_load_domain ./intl/./intl/loadmsgcat.c:1177:9
  #11 0x7f95ab653df2 (/lib/x86_64-linux-gnu/libc.so.6+0x34df2)
  #12 0x55bf9a6a62cf 
clang::VarDecl::getFlexibleArrayInitChars(clang::ASTContext const&) const 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/AST/Decl.cpp:0:3
  #13 0x55bf9a89094b determineEndOffset((anonymous namespace)::EvalInfo&, 
clang::SourceLocation, unsigned int, (anonymous namespace)::LValue const&, 
clang::CharUnits&)::$_15::operator()(clang::QualType, clang::CharUnits&) const 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/AST/ExprConstant.cpp:11741:21
  #14 0x55bf9a89024e determineEndOffset((anonymous namespace)::EvalInfo&, 
clang::SourceLocation, unsigned int, (anonymous namespace)::LValue const&, 
clang::CharUnits&) 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/AST/ExprConstant.cpp:11763:5
  #15 0x55bf9a7dbb62 tryEvaluateBuiltinObjectSize(clang::Expr const*, 
unsigned int, (anonymous namespace)::EvalInfo&, unsigned long&) 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/AST/ExprConstant.cpp:11848:7
  #16 0x55bf9a7db953 clang::Expr::tryEvaluateObjectSize(unsigned long&, 
clang::ASTContext&, unsigned int) const 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/AST/ExprConstant.cpp:16274:3
  #17 0x55bf98a86e07 
clang::Sema::checkFortifiedBuiltinMemoryFunction(clang::FunctionDecl*, 
clang::CallExpr*)::$_1::operator()(unsigned int) const 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp::9
  #18 0x55bf98a86913 
clang::Sema::checkFortifiedBuiltinMemoryFunction(clang::FunctionDecl*, 
clang::CallExpr*) 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/Sema/SemaChecking.cpp:1318:21
  #19 0x55bf98fdcb51 clang::Sema::BuildResolvedCallExpr(clang::Expr*, 
clang::NamedDecl*, clang::SourceLocation, llvm::ArrayRef, 
clang::SourceLocation, clang::Expr*, bool, clang::CallExpr::ADLCallKind) 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/Sema/SemaExpr.cpp:7463:9
  #20 0x55bf98fbbf78 clang::Sema::BuildCallExpr(clang::Scope*, 
clang::Expr*, clang::SourceLocation, llvm::MutableArrayRef, 
clang::SourceLocation, clang::Expr*, bool, bool) 
/usr/local/google/home/saugustine/llvm/llvm-project/clang/lib/Sema/SemaExpr.cpp:7112:10
  #21 0x55bf98fda08f clang::Sema::ActOnCallExpr(clang::Scope*, 
clang::Expr*, clang::SourceLocation, llvm::MutableArrayRef, 
clang::SourceLocation, clang::Expr*) 

[PATCH] D150411: [NFC][Clang][Coverity] Fix Static Code Analysis Concerns with copy without assign

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

(& I realize I'm a bit late to the party, but:

> In several cases it's not completely obvious to me whether a copy assignment 
> operator should or can be defined. But perhaps this doesn't need to be 
> addressed right now: we seem to compile with -Wextra, which contains 
> -Wdeprecated-copy. That should warn if a compiler-generated copy operation is 
> used while another is user-declared.



> The rules for when the copy vs assignment operators are implicitly defined as 
> deleted can be hard to remember. I think being explicit is useful if only to 
> indicate that the author thought about whether such operations should be 
> provided. Since we don't have a principled reason for defining these 
> operations as deleted, it might not be a bad idea to add a comment that 
> states something like "The copy/move assignment operator is defined as 
> deleted pending further motivation".

I'd also vote in favor of the "let it be implicitly deleted - we have the 
warnings enabled to catch it". But don't feel strongly enough to argue that 
chunks of this patch should be reverted)




Comment at: clang/lib/Sema/SemaAccess.cpp:207
+
+// The copy constrcutor and copy assignment operator is defined as deleted
+// pending further motivation.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150411

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

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

[Qingyuan Zheng via Phabricator via cfe-commits]

daiyousei-qz added a comment.

In D148489#4362990 , @nridge wrote:

> (Do you plan to address the last comment before I merge?)

Oh, okay. Let me address that. Didn't see that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148489

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

[PATCH] D148489: [clangd] Implement configs to stop clangd produce a certain semantic tokens

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

(Do you plan to address the last comment before I merge?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148489

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

[PATCH] D143984: [DebugMetadata] Simplify handling subprogram's retainedNodes field. NFCI (1/7)

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D143984#4358115 , @dzhidzhoev 
wrote:

> In D143984#4357517 , @dblaikie 
> wrote:
>
>> Looks like this series got approved - is it waiting on any particular 
>> feedback, or do you need someone to commit it for you?
>
> I had been waiting for approval for the change of  
> https://reviews.llvm.org/D144008. How would you suggest landing these commits 
> - individually or all at once?

Ah, fair enough.

It's a tough balance - committing them all at once can make it difficult to see 
which specific change is the cause of a regression/buildbot failure/etc. But 
committing them too far apart can cause large chunks of breakage/harder to 
revert, etc. So... probably close-ish together, let the bots give you some 
results before pushing the next patch, maybe?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143984

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

[PATCH] D151172: [CodeGen] Fix the type of the constant that is used to zero-initialize a flexible array member

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/CodeGen/CGExprConstant.cpp:964
+  if (NonzeroLength == 0 &&
+  (DesiredType->getNumElements() != 0 || Elements.empty()))
 return llvm::ConstantAggregateZero::get(DesiredType);

Not sure I like this fix.  If we're going to pass in the DesiredType to 
indicate the correct result type, the caller should make sure it uses the 
correct type; we shouldn't need to correct it after the fact.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151172

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

[PATCH] D148381: [WIP][Clang] Add element_count attribute

[Bill Wendling via Phabricator via cfe-commits]

void updated this revision to Diff 524541.
void added a comment.
Herald added a subscriber: ormris.

Add bounds checking for the "ArrayBounds" saniziter kind.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148381

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/AST/ASTImporter.cpp
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Misc/pragma-attribute-supported-attributes-list.test

Index: clang/test/Misc/pragma-attribute-supported-attributes-list.test
===
--- clang/test/Misc/pragma-attribute-supported-attributes-list.test
+++ clang/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -61,6 +61,7 @@
 // CHECK-NEXT: DiagnoseAsBuiltin (SubjectMatchRule_function)
 // CHECK-NEXT: DisableSanitizerInstrumentation (SubjectMatchRule_function, SubjectMatchRule_objc_method, SubjectMatchRule_variable_is_global)
 // CHECK-NEXT: DisableTailCalls (SubjectMatchRule_function, SubjectMatchRule_objc_method)
+// CHECK-NEXT: ElementCount (SubjectMatchRule_field)
 // CHECK-NEXT: EnableIf (SubjectMatchRule_function)
 // CHECK-NEXT: EnforceTCB (SubjectMatchRule_function, SubjectMatchRule_objc_method)
 // CHECK-NEXT: EnforceTCBLeaf (SubjectMatchRule_function, SubjectMatchRule_objc_method)
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8238,6 +8238,29 @@
   D->addAttr(ZeroCallUsedRegsAttr::Create(S.Context, Kind, AL));
 }
 
+static void handleElementCountAttr(Sema , Decl *D, const ParsedAttr ) {
+  // TODO: Probably needs more processing here. See Sema::AddAlignValueAttr.
+  SmallVector Names;
+  SmallVector Ranges;
+
+  for (unsigned I = 0, E = AL.getNumArgs(); I != E; ++I) {
+StringRef Field;
+SourceLocation FieldLoc;
+
+if (!S.checkStringLiteralArgumentAttr(AL, I, Field, ))
+  return;
+
+Names.push_back(Field);
+Ranges.push_back(FieldLoc);
+  }
+
+  ElementCountAttr *ECA = ::new (S.Context) ElementCountAttr(S.Context, AL,
+ Names.data(),
+ Names.size());
+  ECA->addCountFieldSourceRange(Ranges);
+  D->addAttr(ECA);
+}
+
 static void handleFunctionReturnThunksAttr(Sema , Decl *D,
const ParsedAttr ) {
   StringRef KindStr;
@@ -9136,6 +9159,9 @@
   case ParsedAttr::AT_FunctionReturnThunks:
 handleFunctionReturnThunksAttr(S, D, AL);
 break;
+  case ParsedAttr::AT_ElementCount:
+handleElementCountAttr(S, D, AL);
+break;
 
   // Microsoft attributes:
   case ParsedAttr::AT_LayoutVersion:
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -17692,6 +17692,44 @@
  "Broken injected-class-name");
 }
 
+static const FieldDecl *FindFieldWithElementCountAttr(const RecordDecl *RD) {
+  for (const Decl *D : RD->decls()) {
+if (const auto *FD = dyn_cast(D))
+  if (FD->hasAttr())
+return FD;
+
+if (const auto *SubRD = dyn_cast(D))
+  if (const FieldDecl *FD = FindFieldWithElementCountAttr(SubRD))
+return FD;
+  }
+
+  return nullptr;
+}
+
+static StringRef
+CheckElementCountAttr(const RecordDecl *RD, const FieldDecl *FD,
+  SourceRange ) {
+  const ElementCountAttr *ECA = FD->getAttr();
+  unsigned Idx = 0;
+
+  for (StringRef Field : ECA->elementCountFields()) {
+Loc = ECA->getCountFieldSourceRange(Idx++);
+
+auto DeclIter = llvm::find_if(
+RD->fields(), [&](const FieldDecl *FD){
+  return Field == FD->getName();
+});
+
+if (DeclIter == RD->field_end())
+  return Field;
+
+if (auto *SubRD = DeclIter->getType()->getAsRecordDecl())
+  RD = SubRD;
+  }
+
+  return StringRef();
+}
+
 void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
 SourceRange BraceRange) {
   AdjustDeclIfTemplate(TagD);
@@ -17749,6 +17787,19 @@
  [](const FieldDecl *FD) { return FD->isBitField(); }))
   Diag(BraceRange.getBegin(), diag::warn_pragma_align_not_xl_compatible);
   }
+
+  // Check the "element_count" attribute to ensure that the count field exists
+  // in the struct.
+  if (const RecordDecl *RD = dyn_cast(Tag)) {
+if (const FieldDecl *FD = FindFieldWithElementCountAttr(RD)) {
+  SourceRange SR;
+  StringRef Unknown = CheckElementCountAttr(RD, FD, SR);
+
+  if (!Unknown.empty())
+Diag(SR.getBegin(), diag::warn_element_count_placeholder)
+<< Unknown << SR;
+}
+ 

[PATCH] D151162: Add -Wpacked-non-pod to -Wall

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Yeah, looks consistent with GCC ( https://godbolt.org/z/EGc3Ec9YT ), as you say 
- so I'm OK with it. But wouldn't mind a second opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151162

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

[PATCH] D151172: [CodeGen] Fix the type of the constant that is used to zero-initialize a flexible array member

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak created this revision.
ahatanak added reviewers: efriedma, aaron.ballman, rjmccall.
ahatanak added a project: clang.
Herald added a project: All.
ahatanak requested review of this revision.

`EmitArrayConstant` was incorrectly returning a zeroinitializer of zero-element 
array type when the initializer expression wasn't empty.

This fixes an assertion added in https://reviews.llvm.org/D123649. See the 
discussion here: https://reviews.llvm.org/D123649#4362210


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151172

Files:
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/test/CodeGenCXX/flexible-array-init.cpp


Index: clang/test/CodeGenCXX/flexible-array-init.cpp
===
--- clang/test/CodeGenCXX/flexible-array-init.cpp
+++ clang/test/CodeGenCXX/flexible-array-init.cpp
@@ -23,3 +23,14 @@
 struct B { int x; char y; char z[]; };
 B e = {f(), f(), f(), f()}; // expected-error {{cannot compile this flexible 
array initializer yet}}
 #endif
+
+namespace zero_initializer {
+A a0{0, 0}, a1{0, {0, 0}};
+// CHECK: @_ZN16zero_initializer2a0E = global { i32, [1 x i32] } 
zeroinitializer,
+// CHECK: @_ZN16zero_initializer2a1E = global { i32, [2 x i32] } 
zeroinitializer,
+
+struct S { int x[]; };
+
+S s{0};
+// CHECK: @_ZN16zero_initializer1sE = global { [1 x i32] } zeroinitializer,
+}
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -958,7 +958,10 @@
   --NonzeroLength;
   }
 
-  if (NonzeroLength == 0)
+  // Return an all-zero aggregate unless the array is a flexible array member
+  // and Elements isn't empty.
+  if (NonzeroLength == 0 &&
+  (DesiredType->getNumElements() != 0 || Elements.empty()))
 return llvm::ConstantAggregateZero::get(DesiredType);
 
   // Add a zeroinitializer array filler if we have lots of trailing zeroes.


Index: clang/test/CodeGenCXX/flexible-array-init.cpp
===
--- clang/test/CodeGenCXX/flexible-array-init.cpp
+++ clang/test/CodeGenCXX/flexible-array-init.cpp
@@ -23,3 +23,14 @@
 struct B { int x; char y; char z[]; };
 B e = {f(), f(), f(), f()}; // expected-error {{cannot compile this flexible array initializer yet}}
 #endif
+
+namespace zero_initializer {
+A a0{0, 0}, a1{0, {0, 0}};
+// CHECK: @_ZN16zero_initializer2a0E = global { i32, [1 x i32] } zeroinitializer,
+// CHECK: @_ZN16zero_initializer2a1E = global { i32, [2 x i32] } zeroinitializer,
+
+struct S { int x[]; };
+
+S s{0};
+// CHECK: @_ZN16zero_initializer1sE = global { [1 x i32] } zeroinitializer,
+}
Index: clang/lib/CodeGen/CGExprConstant.cpp
===
--- clang/lib/CodeGen/CGExprConstant.cpp
+++ clang/lib/CodeGen/CGExprConstant.cpp
@@ -958,7 +958,10 @@
   --NonzeroLength;
   }
 
-  if (NonzeroLength == 0)
+  // Return an all-zero aggregate unless the array is a flexible array member
+  // and Elements isn't empty.
+  if (NonzeroLength == 0 &&
+  (DesiredType->getNumElements() != 0 || Elements.empty()))
 return llvm::ConstantAggregateZero::get(DesiredType);
 
   // Add a zeroinitializer array filler if we have lots of trailing zeroes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151148: [clang][ExprConstant] fix __builtin_object_size for compound literal

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:11737
 bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
 if (Ty->isStructureType() &&
 Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {

nickdesaulniers wrote:
> efriedma wrote:
> > For the second call to CheckHandleSizeof (line 11791), is the type of 
> > consistent with the type of the variable?  If Ty refers to a field, I'm not 
> > sure it makes sense to call getFlexibleArrayInitChars().  Something along 
> > the lines of "struct Z { struct A { int x, y[]; } z; int a; };"
> Can such an object even be initialized?
> ```
> cat /tmp/x.c
> struct Z { struct A { int x, y[]; } z; int a; };
> 
> static struct Z my_z = {
>   .z = {
> .x = 42,
> .y = {
>   0, 1, 2,
> },
>   },
>   .a = 42,
> };
> 
> unsigned long foo (void) {
>   return __builtin_object_size(_z, 1);
> }
> $ clang -O2 /tmp/x.c -S -o -
> /tmp/x.c:1:37: warning: field 'z' with variable sized type 'struct A' not at 
> the end of a struct or class is a GNU extension 
> [-Wgnu-variable-sized-type-not-at-end]
> struct Z { struct A { int x, y[]; } z; int a; };
> ^
> /tmp/x.c:6:10: error: initialization of flexible array member is not allowed
> .y = {
>  ^
> /tmp/x.c:1:30: note: initialized flexible array member 'y' is here
> struct Z { struct A { int x, y[]; } z; int a; };
>  ^
> 1 warning and 1 error generated.
> ```
> or did you have something else in mind?
I was thinking something more along the lines of

```
struct Z { struct A { int x, y[]; } z; int a; int b[]; };

static struct Z my_z = { .b = {1,2,3} };

unsigned long foo (void) {
  return __builtin_object_size(_z.z, 1);
}
```

The idea being that you're adding the length of the flexible member even though 
the caller isn't trying to ask about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151148

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

[PATCH] D151148: [clang][ExprConstant] fix __builtin_object_size for compound literal

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:11737
 bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
 if (Ty->isStructureType() &&
 Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {

efriedma wrote:
> For the second call to CheckHandleSizeof (line 11791), is the type of 
> consistent with the type of the variable?  If Ty refers to a field, I'm not 
> sure it makes sense to call getFlexibleArrayInitChars().  Something along the 
> lines of "struct Z { struct A { int x, y[]; } z; int a; };"
Can such an object even be initialized?
```
cat /tmp/x.c
struct Z { struct A { int x, y[]; } z; int a; };

static struct Z my_z = {
  .z = {
.x = 42,
.y = {
  0, 1, 2,
},
  },
  .a = 42,
};

unsigned long foo (void) {
  return __builtin_object_size(_z, 1);
}
$ clang -O2 /tmp/x.c -S -o -
/tmp/x.c:1:37: warning: field 'z' with variable sized type 'struct A' not at 
the end of a struct or class is a GNU extension 
[-Wgnu-variable-sized-type-not-at-end]
struct Z { struct A { int x, y[]; } z; int a; };
^
/tmp/x.c:6:10: error: initialization of flexible array member is not allowed
.y = {
 ^
/tmp/x.c:1:30: note: initialized flexible array member 'y' is here
struct Z { struct A { int x, y[]; } z; int a; };
 ^
1 warning and 1 error generated.
```
or did you have something else in mind?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151148

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

[PATCH] D151148: [clang][ExprConstant] fix __builtin_object_size for compound literal

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 524532.
nickdesaulniers marked an inline comment as done.
nickdesaulniers added a comment.

- more dyn_cast


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151148

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGen/object-size.c


Index: clang/test/CodeGen/object-size.c
===
--- clang/test/CodeGen/object-size.c
+++ clang/test/CodeGen/object-size.c
@@ -551,6 +551,10 @@
   // CHECK: ret i64 16
   return OBJECT_SIZE_BUILTIN(, 1);
 }
+unsigned long test35(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(&(struct DynStructVar){}, 1);
+}
 
 // CHECK-LABEL: @PR30346
 void PR30346(void) {
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11736,9 +11736,9 @@
 bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
 if (Ty->isStructureType() &&
 Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {
-  const auto *VD =
-  cast(LVal.getLValueBase().get());
-  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+  if (const auto *V = LVal.getLValueBase().dyn_cast())
+if (const auto *VD = dyn_cast(V))
+  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
 }
 return Ret;
   };


Index: clang/test/CodeGen/object-size.c
===
--- clang/test/CodeGen/object-size.c
+++ clang/test/CodeGen/object-size.c
@@ -551,6 +551,10 @@
   // CHECK: ret i64 16
   return OBJECT_SIZE_BUILTIN(, 1);
 }
+unsigned long test35(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(&(struct DynStructVar){}, 1);
+}
 
 // CHECK-LABEL: @PR30346
 void PR30346(void) {
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11736,9 +11736,9 @@
 bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
 if (Ty->isStructureType() &&
 Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {
-  const auto *VD =
-  cast(LVal.getLValueBase().get());
-  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+  if (const auto *V = LVal.getLValueBase().dyn_cast())
+if (const auto *VD = dyn_cast(V))
+  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
 }
 return Ret;
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D148461: [clang-tidy] Support C++17/20 in bugprone-exception-escape

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs added a comment.

Please cover the changes in as much test cases as possible.




Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:151
 
+bool isFunctionTypeEqual(const FunctionType *From, const FunctionType *To) {
+  if (From == To)

The naming suggests that we check for equality, however in reality we check for 
convertability here. For example if one of the prototypes has exception spec 
and the other doesn't have one, this can still return true. 

Also instead of manually matching everything, you could take a look at 
`ODRHash` or `StructuralEquivalenceContext`. `ODRHash` for example doesn't take 
the `FunctionProtoType` into account when it generates the hashes AFAIK, so 
maybe you could use it to avoid these manual checks. I don't know if 
`StructuralEquivalenceContext` uses it or not, but you might consider taking a 
look at that too.

Maybe you can also consider pasting the appropriate section of the standard 
here, so that whenever someone reads the code they'll understand why are we 
doing what we're doing.

And please cover this function in positive and negative test cases too.



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:185
 if (From->isFunctionType())
-  return To->getPointeeType() == From;
-
-return false;
-  }
-
-  if (To->isMemberFunctionPointerType()) {
-if (!From->isMemberFunctionPointerType())
-  return false;
-
+  return isFunctionTypeEqual(ToFunctionTypePtr,
+ From->castAs());

I think we want to call this function for member function pointers too.

The[[ https://github.com/cplusplus/draft/releases |  C++ N4917 draft ]] says 
the following:
```
7.3.14 Function pointer conversions [conv.fctptr]
A prvalue of type “pointer to noexcept function” can be converted to a prvalue 
of type “pointer to function”.
The result is a pointer to the function. A prvalue of type “pointer to member 
of type noexcept function” can
be converted to a prvalue of type “pointer to member of type function”. The 
result designates the member
function.
```



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:306
+
+  if (From->isMemberPointerType() || To->isMemberPointerType())
 return false;

Please cover this line with both positive and negative test cases.

Also upon looking up both [[ 
https://www.open-std.org/jtc1/sc22/wg21/docs/standards | N4849 ]] (C++ 20 
draft) and [[ https://github.com/cplusplus/draft/releases | N4917]] (C++ 23 
draft), they both say for qualification conversion that 


> each P_i is ... pointer to member of class C_i of type, ...

Why are we not allowing them if the standard is at least C++ 20?



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:160-162
+// FIXME: Two function pointers can differ in 'noexcept', but they still
+// should be considered to be same, now this triggers false-positive 
because
+// Type* != Type*.

PiotrZSL wrote:
> isuckatcs wrote:
> > Are you sure `noexcept` is stored in the type? The test case you modified 
> > `throw_noexcept_catch_regular()` tests this scenario and in that case the 
> > types seem to be the same even though one of the is noexcept an the other 
> > is not.
> > 
> > If the FIXME is valid the proper way would be to implement it in this patch.
> Yes I'm sure.
> C++11 - no warning
> C++14 - no warning
> C++17 - warning
> Looks like since C++17 noexcept is part of type.
> Initially I wanted only to enable tests under C++17/20.
> That's why "FIXME". I will look into this.
I did some investigation regarding this and if I dump the type of `void 
no_throw() noexcept` both with `-std=c++11` and `-std=c++17` I get the same 
result.
```
FunctionProtoType 'void (void) noexcept' cdecl
`-BuiltinType 'void'
```

I agree however that function pointer conversion was not properly implemented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148461

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

[PATCH] D151059: [test] Add C++ ext_vector_type tests

[Cassie Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG590eb76ba3cd: [test] Add C++ ext_vector_type tests (authored 
by porglezomp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151059

Files:
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -545,3 +545,230 @@
   auto b4 = (v4 >= 0x12);
 }
 #endif
+
+namespace all_operators {
+typedef unsigned int v2u __attribute__((ext_vector_type(2)));
+typedef float v2f __attribute__((ext_vector_type(2)));
+
+void test_int_vector_scalar(unsigned int ua, v2u v2ua) {
+  // Operators with one integer vector and one integer scalar operand. The scalar will splat.
+  (void)(v2ua + ua);
+  (void)(ua + v2ua);
+  (void)(v2ua - ua);
+  (void)(ua - v2ua);
+  (void)(v2ua * ua);
+  (void)(ua * v2ua);
+  (void)(v2ua / ua);
+  (void)(ua / v2ua);
+  (void)(v2ua % ua);
+  (void)(ua % v2ua);
+
+  (void)(v2ua == ua);
+  (void)(ua == v2ua);
+  (void)(v2ua != ua);
+  (void)(ua != v2ua);
+  (void)(v2ua <= ua);
+  (void)(ua <= v2ua);
+  (void)(v2ua >= ua);
+  (void)(ua >= v2ua);
+  (void)(v2ua < ua);
+  (void)(ua < v2ua);
+  (void)(v2ua > ua);
+  (void)(ua > v2ua);
+  (void)(v2ua && ua);
+  (void)(ua && v2ua);
+  (void)(v2ua || ua);
+  (void)(ua || v2ua);
+
+  (void)(v2ua & ua);
+  (void)(ua & v2ua);
+  (void)(v2ua | ua);
+  (void)(ua | v2ua);
+  (void)(v2ua ^ ua);
+  (void)(ua ^ v2ua);
+  (void)(v2ua << ua);
+  (void)(ua << v2ua);
+  (void)(v2ua >> ua);
+  (void)(ua >> v2ua);
+
+  v2ua += ua;
+  v2ua -= ua;
+  v2ua *= ua;
+  v2ua /= ua;
+  v2ua %= ua;
+  v2ua &= ua;
+  v2ua |= ua;
+  v2ua ^= ua;
+  v2ua >>= ua;
+  v2ua <<= ua;
+
+  ua += v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua -= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua *= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua /= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua %= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua &= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua |= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua ^= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua >>= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua <<= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+}
+
+void test_float_vector_scalar(float fa, unsigned int ua, v2f v2fa) {
+  // Operators with one float vector and one float scalar operand. The scalar will splat.
+  (void)(v2fa + fa);
+  (void)(fa + v2fa);
+  (void)(v2fa - fa);
+  (void)(fa - v2fa);
+  (void)(v2fa * fa);
+  (void)(fa * v2fa);
+  (void)(v2fa / fa);
+  (void)(fa / v2fa);
+  (void)(v2fa % fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa % v2fa); // expected-error{{invalid operands to binary expression}}
+
+  (void)(v2fa == fa);
+  (void)(fa == v2fa);
+  (void)(v2fa != fa);
+  (void)(fa != v2fa);
+  (void)(v2fa <= fa);
+  (void)(fa <= v2fa);
+  (void)(v2fa >= fa);
+  (void)(fa >= v2fa);
+  (void)(v2fa < fa);
+  (void)(fa < v2fa);
+  (void)(v2fa > fa);
+  (void)(fa > v2fa);
+  (void)(v2fa && fa);
+  (void)(fa && v2fa);
+  (void)(v2fa || fa);
+  (void)(fa || v2fa);
+
+  (void)(v2fa & fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa & v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa | fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa | v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa ^ fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa ^ v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa << fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa << ua); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(fa << v2fa); // expected-error{{used type 'float' where integer is required}}
+  (void)(ua << v2fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa >> fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa >> ua); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(fa >> v2fa); // expected-error{{used type 'float' where integer is required}}
+  (void)(ua >> v2fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer 

[clang] 590eb76 - [test] Add C++ ext_vector_type tests

[Cassie Jones via cfe-commits]

Author: Cassie Jones
Date: 2023-05-22T15:58:01-07:00
New Revision: 590eb76ba3cd668baee7d06940ad820e89f830c4

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

LOG: [test] Add C++ ext_vector_type tests

Add initial tests for the behavior of ext_vector_type vectors for
vector vs scalar ops in C++. Their behavior doesn't agree with the behavior in
C and what the behavior seems like it should be, these are baseline tests before
implementing those changes.

Reviewed By: fhahn

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

Added: 


Modified: 
clang/test/SemaCXX/vector.cpp

Removed: 




diff  --git a/clang/test/SemaCXX/vector.cpp b/clang/test/SemaCXX/vector.cpp
index 9c6e2ebedf6ea..7c8ee89814e57 100644
--- a/clang/test/SemaCXX/vector.cpp
+++ b/clang/test/SemaCXX/vector.cpp
@@ -545,3 +545,230 @@ void triggerIntegerRankCheck() {
   auto b4 = (v4 >= 0x12);
 }
 #endif
+
+namespace all_operators {
+typedef unsigned int v2u __attribute__((ext_vector_type(2)));
+typedef float v2f __attribute__((ext_vector_type(2)));
+
+void test_int_vector_scalar(unsigned int ua, v2u v2ua) {
+  // Operators with one integer vector and one integer scalar operand. The 
scalar will splat.
+  (void)(v2ua + ua);
+  (void)(ua + v2ua);
+  (void)(v2ua - ua);
+  (void)(ua - v2ua);
+  (void)(v2ua * ua);
+  (void)(ua * v2ua);
+  (void)(v2ua / ua);
+  (void)(ua / v2ua);
+  (void)(v2ua % ua);
+  (void)(ua % v2ua);
+
+  (void)(v2ua == ua);
+  (void)(ua == v2ua);
+  (void)(v2ua != ua);
+  (void)(ua != v2ua);
+  (void)(v2ua <= ua);
+  (void)(ua <= v2ua);
+  (void)(v2ua >= ua);
+  (void)(ua >= v2ua);
+  (void)(v2ua < ua);
+  (void)(ua < v2ua);
+  (void)(v2ua > ua);
+  (void)(ua > v2ua);
+  (void)(v2ua && ua);
+  (void)(ua && v2ua);
+  (void)(v2ua || ua);
+  (void)(ua || v2ua);
+
+  (void)(v2ua & ua);
+  (void)(ua & v2ua);
+  (void)(v2ua | ua);
+  (void)(ua | v2ua);
+  (void)(v2ua ^ ua);
+  (void)(ua ^ v2ua);
+  (void)(v2ua << ua);
+  (void)(ua << v2ua);
+  (void)(v2ua >> ua);
+  (void)(ua >> v2ua);
+
+  v2ua += ua;
+  v2ua -= ua;
+  v2ua *= ua;
+  v2ua /= ua;
+  v2ua %= ua;
+  v2ua &= ua;
+  v2ua |= ua;
+  v2ua ^= ua;
+  v2ua >>= ua;
+  v2ua <<= ua;
+
+  ua += v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua -= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua *= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua /= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua %= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua &= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua |= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua ^= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua >>= v2ua; // expected-error{{assigning to 'unsigned int' from 
incompatible type 'v2u'}}
+  ua <<= v2ua; // expected-error{{assigning to 'unsigned int' from 
incompatible type 'v2u'}}
+}
+
+void test_float_vector_scalar(float fa, unsigned int ua, v2f v2fa) {
+  // Operators with one float vector and one float scalar operand. The scalar 
will splat.
+  (void)(v2fa + fa);
+  (void)(fa + v2fa);
+  (void)(v2fa - fa);
+  (void)(fa - v2fa);
+  (void)(v2fa * fa);
+  (void)(fa * v2fa);
+  (void)(v2fa / fa);
+  (void)(fa / v2fa);
+  (void)(v2fa % fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa % v2fa); // expected-error{{invalid operands to binary expression}}
+
+  (void)(v2fa == fa);
+  (void)(fa == v2fa);
+  (void)(v2fa != fa);
+  (void)(fa != v2fa);
+  (void)(v2fa <= fa);
+  (void)(fa <= v2fa);
+  (void)(v2fa >= fa);
+  (void)(fa >= v2fa);
+  (void)(v2fa < fa);
+  (void)(fa < v2fa);
+  (void)(v2fa > fa);
+  (void)(fa > v2fa);
+  (void)(v2fa && fa);
+  (void)(fa && v2fa);
+  (void)(v2fa || fa);
+  (void)(fa || v2fa);
+
+  (void)(v2fa & fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa & v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa | fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa | v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa ^ fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa ^ v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa << fa); // expected-error{{used type 'v2f' (vector of 2 'float' 
values) where integer is required}}
+  (void)(v2fa << ua); // expected-error{{used type 'v2f' (vector of 2 'float' 
values) where integer is required}}
+  (void)(fa << v2fa); // expected-error{{used type 'float' where integer is 

[PATCH] D123649: Allow flexible array initialization in C++.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D123649#4362418 , @aaron.ballman 
wrote:

> In D123649#4362402 , @efriedma 
> wrote:
>
>> The assertion is correct; without it, your code is getting miscompiled.
>
> The assertion may be correct in its intent, but we still should not be 
> asserting in practice: https://godbolt.org/z/cs5bhvPGr

I wasn't trying to suggest our behavior is correct here, just that focusing 
assertion is correctly indicating an underlying issue with the generated IR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123649

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

[PATCH] D151148: [clang][ExprConstant] fix __builtin_object_size for compound literal

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:11737
 bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
 if (Ty->isStructureType() &&
 Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {

For the second call to CheckHandleSizeof (line 11791), is the type of 
consistent with the type of the variable?  If Ty refers to a field, I'm not 
sure it makes sense to call getFlexibleArrayInitChars().  Something along the 
lines of "struct Z { struct A { int x, y[]; } z; int a; };"



Comment at: clang/lib/AST/ExprConstant.cpp:11740
+  if (const auto *V = LVal.getLValueBase().dyn_cast())
+Result += cast(V)->getFlexibleArrayInitChars(Info.Ctx);
 }

I think I'd like to check `isa` here to be on the safe side, even if I 
don't have a specific example where it would break.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151148

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

[PATCH] D151166: [clangd] Interactive AST matchers with #pragma clang query

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

@kadircet this is a slightly silly feature that I put together on vacation

I'd like your call on whether this is something we should have. If so, feel 
free to hand off the review.

It's definitely a power-user feature and nigh undiscoverable. I'd plan to write 
some docs, but even so I'm not sure how much use it will get.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151166

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

[PATCH] D151166: [clangd] Interactive AST matchers with #pragma clang query

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 524519.
sammccall edited the summary of this revision.
sammccall added a comment.

Add demo link


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151166

Files:
  clang-tools-extra/clangd/ASTMatchers.cpp
  clang-tools-extra/clangd/ASTMatchers.h
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2341,6 +2341,14 @@
   }
 }
 
+TEST(FindReferences, Matchers) {
+  checkFindRefs(R"cpp(
+#pragma clang query \
+  ^parmVarDecl()   
+void foo([[int x]], [[int y]]);
+  )cpp");
+}
+
 TEST(FindReferences, NeedsIndexForSymbols) {
   const char *Header = "int foo();";
   Annotations Main("int main() { [[f^oo]](); }");
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -361,6 +361,40 @@
 diagSource(Diag::ClangTidy), diagName("llvm-include-order");
 }
 
+TEST(DiagnosticsTest, Matchers) {
+  Annotations Test(R"cpp(
+;
+#pragma clang query parmVarDecl()
+void foo([[int x]], [[int y]]);
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.ranges()[0], "query:3 matches"),
+  Diag(Test.ranges()[1], "query:3 matches")));
+}
+
+TEST(DiagnosticsTest, MatchersPreamble) {
+  Annotations Test(R"cpp(
+#pragma clang query parmVarDecl().bind("parm")
+void foo([[int x]]);
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "parm matches")));
+}
+
+TEST(DiagnosticsTest, MatchersError) {
+  Annotations Test(R"cpp(
+#pragma clang query [[pmVrDcl]]() // error-ok
+void foo(int x);
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(
+  Diag(Test.range(), "Bad matcher: 1:2: Matcher not found: pmVrDcl")));
+}
+
 TEST(DiagnosticTest, TemplatesInHeaders) {
   // Diagnostics from templates defined in headers are placed at the expansion.
   Annotations Main(R"cpp(
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3968,6 +3968,16 @@
   }
 }
 
+TEST(CompletionTest, MatcherPragma) {
+  Annotations Code(R"cpp(
+#pragma clang query parmVarDecl(hasType^)
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+
+  ASSERT_THAT(completions(TU, Code.point()).Completions,
+  ElementsAre(named("hasType"), named("hasTypeLoc")));
+}
+
 TEST(SignatureHelp, DocFormat) {
   Annotations Code(R"cpp(
 // Comment `with` markup.
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -67,6 +67,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -1373,6 +1374,35 @@
 return std::nullopt;
   return Results;
 }
+
+std::optional
+maybeFindMatcherReferences(ParsedAST , Position Pos,
+   URIForFile URIMainFile) {
+  const auto  = AST.getMatchers();
+  auto MatcherOnLine = llvm::find_if(Matchers, [](const MatcherPragma ) {
+return M.LineRange.first <= unsigned(Pos.line + 1) &&
+   unsigned(Pos.line + 1) <= M.LineRange.second;
+  });
+  if (MatcherOnLine == Matchers.end())
+return std::nullopt;
+
+  ReferencesResult Results;
+  for (const auto  : MatcherOnLine->match(AST.getASTContext())) {
+auto Range = AST.getTokens().spelledForExpanded(
+AST.getTokens().expandedTokens(Match.second));
+if (!Range || Range->empty())
+  continue;
+ReferencesResult::Reference Ref;
+Ref.Loc.uri = URIMainFile;
+Ref.Loc.range.start =
+sourceLocToPosition(AST.getSourceManager(), Range->front().location());
+

[PATCH] D151168: [CUDA] plumb through new sm_90-specific builtins.

[Artem Belevich via Phabricator via cfe-commits]

tra created this revision.
Herald added subscribers: mattd, gchakrabarti, asavonic, bixia, yaxunl.
Herald added a project: All.
tra added a reviewer: jlebar.
tra published this revision for review.
Herald added subscribers: cfe-commits, jholewinski.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151168

Files:
  clang/include/clang/Basic/BuiltinsNVPTX.def
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenCUDA/builtins-sm90.cu

Index: clang/test/CodeGenCUDA/builtins-sm90.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/builtins-sm90.cu
@@ -0,0 +1,41 @@
+// RUN: %clang_cc1 "-triple" "nvptx64-nvidia-cuda" "-target-feature" "+ptx78" "-target-cpu" "sm_90" -emit-llvm -fcuda-is-device -o - %s | FileCheck %s
+
+// CHECK: define{{.*}} void @_Z6kernelPlPvj(
+__attribute__((global)) void kernel(long *out, void *ptr, unsigned u) {
+  int i = 0;
+  out[i++] = __nvvm_isspacep_shared_cluster(ptr);
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.clusterid.x()
+  out[i++] = __nvvm_read_ptx_sreg_clusterid_x();
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.clusterid.y()
+  out[i++] = __nvvm_read_ptx_sreg_clusterid_y();
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.clusterid.z()
+  out[i++] = __nvvm_read_ptx_sreg_clusterid_z();
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.clusterid.w()
+  out[i++] = __nvvm_read_ptx_sreg_clusterid_w();
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.nclusterid.x()
+  out[i++] = __nvvm_read_ptx_sreg_nclusterid_x();
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.nclusterid.y()
+  out[i++] = __nvvm_read_ptx_sreg_nclusterid_y();
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.nclusterid.z()
+  out[i++] = __nvvm_read_ptx_sreg_nclusterid_z();
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.nclusterid.w()
+  out[i++] = __nvvm_read_ptx_sreg_nclusterid_w();
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.cluster.ctarank()
+  out[i++] = __nvvm_read_ptx_sreg_cluster_ctarank();
+  // CHECK: call i32 @llvm.nvvm.read.ptx.sreg.cluster.nctarank()
+  out[i++] = __nvvm_read_ptx_sreg_cluster_nctarank();
+  // CHECK: call i1 @llvm.nvvm.is_explicit_cluster()
+  out[i++] = __nvvm_is_explicit_cluster();
+
+  auto * sptr = (__attribute__((address_space(3))) void *)ptr;
+  // CHECK: call ptr @llvm.nvvm.mapa(ptr %{{.*}}, i32 %{{.*}})
+  out[i++] = (long) __nvvm_mapa(ptr, u);
+  // CHECK: call ptr addrspace(3) @llvm.nvvm.mapa.shared.cluster(ptr addrspace(3) %{{.*}}, i32 %{{.*}})
+  out[i++] = (long) __nvvm_mapa_shared_cluster(sptr, u);
+  // CHECK: call i32 @llvm.nvvm.getctarank(ptr {{.*}})
+  out[i++] = __nvvm_getctarank(ptr);
+  // CHECK: call i32 @llvm.nvvm.getctarank.shared.cluster(ptr addrspace(3) {{.*}})
+  out[i++] = __nvvm_getctarank_shared_cluster(sptr);
+
+  // CHECK: ret void
+}
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -18869,6 +18869,59 @@
 return MakeCpAsync(Intrinsic::nvvm_cp_async_cg_shared_global_16,
Intrinsic::nvvm_cp_async_cg_shared_global_16_s, *this, E,
16);
+  case NVPTX::BI__nvvm_read_ptx_sreg_clusterid_x:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_clusterid_x));
+  case NVPTX::BI__nvvm_read_ptx_sreg_clusterid_y:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_clusterid_y));
+  case NVPTX::BI__nvvm_read_ptx_sreg_clusterid_z:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_clusterid_z));
+  case NVPTX::BI__nvvm_read_ptx_sreg_clusterid_w:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_clusterid_w));
+  case NVPTX::BI__nvvm_read_ptx_sreg_nclusterid_x:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_nclusterid_x));
+  case NVPTX::BI__nvvm_read_ptx_sreg_nclusterid_y:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_nclusterid_y));
+  case NVPTX::BI__nvvm_read_ptx_sreg_nclusterid_z:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_nclusterid_z));
+  case NVPTX::BI__nvvm_read_ptx_sreg_nclusterid_w:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_nclusterid_w));
+  case NVPTX::BI__nvvm_read_ptx_sreg_cluster_ctarank:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_cluster_ctarank));
+  case NVPTX::BI__nvvm_read_ptx_sreg_cluster_nctarank:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_read_ptx_sreg_cluster_nctarank));
+  case NVPTX::BI__nvvm_is_explicit_cluster:
+return Builder.CreateCall(
+CGM.getIntrinsic(Intrinsic::nvvm_is_explicit_cluster));
+  case NVPTX::BI__nvvm_isspacep_shared_cluster:
+return 

[PATCH] D151166: [clangd] Interactive AST matchers with #pragma clang query

[Sam McCall via Phabricator via cfe-commits]

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

The idea: you can type in AST matchers, and clangd shows you matches
within the document.

Syntax is like:

  #pragma clang query varDecl().bind("var")

Features:

- matches are shown within the document as diagnostics
- find-references on the pragma will produce a list of matches
- errors within matchers are shown as diagnostics
- code completion is available within matchers

There are some rough edges here, e.g. the diagnostics are warnings and
have no LSP diagnostic code, the diagnostic text for matcher errors
isn't ideal etc.
I think this is a good start as-is and we can work out if we want to
improve these cases later, if this feature ends up being useful.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151166

Files:
  clang-tools-extra/clangd/ASTMatchers.cpp
  clang-tools-extra/clangd/ASTMatchers.h
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/Compiler.cpp
  clang-tools-extra/clangd/Diagnostics.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/ParsedAST.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/Preamble.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2341,6 +2341,14 @@
   }
 }
 
+TEST(FindReferences, Matchers) {
+  checkFindRefs(R"cpp(
+#pragma clang query \
+  ^parmVarDecl()   
+void foo([[int x]], [[int y]]);
+  )cpp");
+}
+
 TEST(FindReferences, NeedsIndexForSymbols) {
   const char *Header = "int foo();";
   Annotations Main("int main() { [[f^oo]](); }");
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -361,6 +361,40 @@
 diagSource(Diag::ClangTidy), diagName("llvm-include-order");
 }
 
+TEST(DiagnosticsTest, Matchers) {
+  Annotations Test(R"cpp(
+;
+#pragma clang query parmVarDecl()
+void foo([[int x]], [[int y]]);
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.ranges()[0], "query:3 matches"),
+  Diag(Test.ranges()[1], "query:3 matches")));
+}
+
+TEST(DiagnosticsTest, MatchersPreamble) {
+  Annotations Test(R"cpp(
+#pragma clang query parmVarDecl().bind("parm")
+void foo([[int x]]);
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "parm matches")));
+}
+
+TEST(DiagnosticsTest, MatchersError) {
+  Annotations Test(R"cpp(
+#pragma clang query [[pmVrDcl]]() // error-ok
+void foo(int x);
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(
+  *TU.build().getDiagnostics(),
+  ElementsAre(
+  Diag(Test.range(), "Bad matcher: 1:2: Matcher not found: pmVrDcl")));
+}
+
 TEST(DiagnosticTest, TemplatesInHeaders) {
   // Diagnostics from templates defined in headers are placed at the expansion.
   Annotations Main(R"cpp(
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -3968,6 +3968,16 @@
   }
 }
 
+TEST(CompletionTest, MatcherPragma) {
+  Annotations Code(R"cpp(
+#pragma clang query parmVarDecl(hasType^)
+  )cpp");
+  auto TU = TestTU::withCode(Code.code());
+
+  ASSERT_THAT(completions(TU, Code.point()).Completions,
+  ElementsAre(named("hasType"), named("hasTypeLoc")));
+}
+
 TEST(SignatureHelp, DocFormat) {
   Annotations Code(R"cpp(
 // Comment `with` markup.
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -67,6 +67,7 @@
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
+#include 
 #include 
 
 namespace clang {
@@ -1373,6 +1374,35 @@
 return std::nullopt;
   return Results;
 }
+
+std::optional
+maybeFindMatcherReferences(ParsedAST , Position Pos,
+   URIForFile URIMainFile) {
+  const auto  = 

[PATCH] D150841: [IR] Make stack protector symbol dso_local according to -f[no-]direct-access-external-data

[Nick Desaulniers via Phabricator via cfe-commits]

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

This also resolves https://github.com/llvm/llvm-project/issues/62482; was that 
intentional? If so, consider adding a link to 
https://github.com/llvm/llvm-project/issues/62482 to this commit description 
(or closing https://github.com/llvm/llvm-project/issues/62482 as a duplicate of 
https://github.com/llvm/llvm-project/issues/60116 if that makes more sense).




Comment at: llvm/lib/IR/Module.cpp:675-684
+bool Module::getDirectAccessExternalData() const {
+  auto *Val = 
cast_or_null(getModuleFlag("direct-access-external-data"));
+  if (Val)
+return cast(Val->getValue())->getZExtValue() > 0;
+  return getPICLevel() == PICLevel::NotPIC;
+}
+

I think the presubmit bot failed due to clang-format changes to code introduced 
in this hunk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150841

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

[PATCH] D151162: Add -Wpacked-non-pod to -Wall

[Zenong Zhang via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151162

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/test/Misc/warning-wall.c


Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -99,6 +99,7 @@
 CHECK-NEXT:  -Wswitch
 CHECK-NEXT:  -Wswitch-bool
 CHECK-NEXT:  -Wmisleading-indentation
+CHECK-NEXT:  -Wpacked-non-pod
 
 
 CHECK-NOT:-W
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1060,7 +1060,7 @@
 // warning should be active _only_ when -Wall is passed in, mark it as
 // DefaultIgnore in addition to putting it here.
 def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool,
-MisleadingIndentation]>;
+MisleadingIndentation, PackedNonPod]>;
 
 // Warnings that should be in clang-cl /w4.
 def : DiagGroup<"CL4", [All, Extra]>;


Index: clang/test/Misc/warning-wall.c
===
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -99,6 +99,7 @@
 CHECK-NEXT:  -Wswitch
 CHECK-NEXT:  -Wswitch-bool
 CHECK-NEXT:  -Wmisleading-indentation
+CHECK-NEXT:  -Wpacked-non-pod
 
 
 CHECK-NOT:-W
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1060,7 +1060,7 @@
 // warning should be active _only_ when -Wall is passed in, mark it as
 // DefaultIgnore in addition to putting it here.
 def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool,
-MisleadingIndentation]>;
+MisleadingIndentation, PackedNonPod]>;
 
 // Warnings that should be in clang-cl /w4.
 def : DiagGroup<"CL4", [All, Extra]>;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151061: [test] Add ext_vector_type tests for C

[Cassie Jones via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGb5b689679e1e: [test] Add more ext_vector_type tests for C 
(authored by porglezomp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151061

Files:
  clang/test/Sema/ext_vector_ops.c
  clang/test/Sema/vecshift.c

Index: clang/test/Sema/vecshift.c
===
--- clang/test/Sema/vecshift.c
+++ clang/test/Sema/vecshift.c
@@ -65,6 +65,20 @@
   vs8 = ui << vs8;
   vus8 = 1 << vus8;
 
+  vc8 = vc8 << c;
+  vuc8 = vuc8 << uc;
+  vs8 = vs8 << s;
+  vus8 = vus8 << us;
+  vi8 = vi8 << i;
+  vui8 = vui8 << ui;
+
+  vc8 = vc8 << i;
+  vuc8 = vuc8 << i;
+  vs8 = vs8 << i;
+  vus8 = vus8 << i;
+  vi8 = vi8 << i;
+  vui8 = vui8 << i;
+
   vc8 = vc8 << vc8;
 #ifdef ERR
   vi8 = vi8 << vuc8; // expected-error {{vector operands do not have the same elements sizes}}
Index: clang/test/Sema/ext_vector_ops.c
===
--- clang/test/Sema/ext_vector_ops.c
+++ clang/test/Sema/ext_vector_ops.c
@@ -25,3 +25,211 @@
   v2u *v2u_ptr = 0;
   v2s *v2s_ptr;
 }
+
+void test_int_vector_scalar(unsigned int ua, v2u v2ua) {
+  // Operations with one integer vector and one scalar. These splat the scalar.
+  (void)(v2ua + ua);
+  (void)(ua + v2ua);
+  (void)(v2ua - ua);
+  (void)(ua - v2ua);
+  (void)(v2ua * ua);
+  (void)(ua * v2ua);
+  (void)(v2ua / ua);
+  (void)(ua / v2ua);
+  (void)(v2ua % ua);
+  (void)(ua % v2ua);
+
+  (void)(v2ua == ua);
+  (void)(ua == v2ua);
+  (void)(v2ua != ua);
+  (void)(ua != v2ua);
+  (void)(v2ua <= ua);
+  (void)(ua <= v2ua);
+  (void)(v2ua >= ua);
+  (void)(ua >= v2ua);
+  (void)(v2ua < ua);
+  (void)(ua < v2ua);
+  (void)(v2ua > ua);
+  (void)(ua > v2ua);
+  (void)(v2ua && ua);
+  (void)(ua && v2ua);
+  (void)(v2ua || ua);
+  (void)(ua || v2ua);
+
+  (void)(v2ua & ua);
+  (void)(ua & v2ua);
+  (void)(v2ua | ua);
+  (void)(ua | v2ua);
+  (void)(v2ua ^ ua);
+  (void)(ua ^ v2ua);
+  (void)(v2ua << ua);
+  (void)(ua << v2ua);
+  (void)(v2ua >> ua);
+  (void)(ua >> v2ua);
+
+  v2ua += ua;
+  v2ua -= ua;
+  v2ua *= ua;
+  v2ua /= ua;
+  v2ua %= ua;
+  v2ua &= ua;
+  v2ua |= ua;
+  v2ua ^= ua;
+  v2ua >>= ua;
+  v2ua <<= ua;
+
+  ua += v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua -= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua *= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua /= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua %= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua &= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua |= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua ^= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua >>= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua <<= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+}
+
+void test_float_vector_scalar(float fa, unsigned int ua, v2f v2fa) {
+  // Operations with one float vector and one scalar. These splat the scalar.
+  (void)(v2fa + fa);
+  (void)(fa + v2fa);
+  (void)(v2fa - fa);
+  (void)(fa - v2fa);
+  (void)(v2fa * fa);
+  (void)(fa * v2fa);
+  (void)(v2fa / fa);
+  (void)(fa / v2fa);
+  (void)(v2fa % fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa % v2fa); // expected-error{{invalid operands to binary expression}}
+
+  (void)(v2fa == fa);
+  (void)(fa == v2fa);
+  (void)(v2fa != fa);
+  (void)(fa != v2fa);
+  (void)(v2fa <= fa);
+  (void)(fa <= v2fa);
+  (void)(v2fa >= fa);
+  (void)(fa >= v2fa);
+  (void)(v2fa < fa);
+  (void)(fa < v2fa);
+  (void)(v2fa > fa);
+  (void)(fa > v2fa);
+  (void)(v2fa && fa);
+  (void)(fa && v2fa);
+  (void)(v2fa || fa);
+  (void)(fa || v2fa);
+
+  (void)(v2fa & fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa & v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa | fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa | v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa ^ fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa ^ v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa << fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa << ua); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(fa << v2fa); // expected-error{{used type 'float' where integer is required}}
+  (void)(ua << v2fa); // expected-error{{used type 'v2f' 

[clang] b5b6896 - [test] Add more ext_vector_type tests for C

[Cassie Jones via cfe-commits]

Author: Cassie Jones
Date: 2023-05-22T15:08:14-07:00
New Revision: b5b689679e1e435b5c82832f468ed939c7b72021

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

LOG: [test] Add more ext_vector_type tests for C

Test that all builtin operators type check successfully with one vector
operand and one scalar operand.

Reviewed By: fhahn

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

Added: 


Modified: 
clang/test/Sema/ext_vector_ops.c
clang/test/Sema/vecshift.c

Removed: 




diff  --git a/clang/test/Sema/ext_vector_ops.c 
b/clang/test/Sema/ext_vector_ops.c
index af4df07e14da9..5dc3047e145f1 100644
--- a/clang/test/Sema/ext_vector_ops.c
+++ b/clang/test/Sema/ext_vector_ops.c
@@ -25,3 +25,211 @@ void test1(v2u v2ua, v2s v2sa, v2f v2fa) {
   v2u *v2u_ptr = 0;
   v2s *v2s_ptr;
 }
+
+void test_int_vector_scalar(unsigned int ua, v2u v2ua) {
+  // Operations with one integer vector and one scalar. These splat the scalar.
+  (void)(v2ua + ua);
+  (void)(ua + v2ua);
+  (void)(v2ua - ua);
+  (void)(ua - v2ua);
+  (void)(v2ua * ua);
+  (void)(ua * v2ua);
+  (void)(v2ua / ua);
+  (void)(ua / v2ua);
+  (void)(v2ua % ua);
+  (void)(ua % v2ua);
+
+  (void)(v2ua == ua);
+  (void)(ua == v2ua);
+  (void)(v2ua != ua);
+  (void)(ua != v2ua);
+  (void)(v2ua <= ua);
+  (void)(ua <= v2ua);
+  (void)(v2ua >= ua);
+  (void)(ua >= v2ua);
+  (void)(v2ua < ua);
+  (void)(ua < v2ua);
+  (void)(v2ua > ua);
+  (void)(ua > v2ua);
+  (void)(v2ua && ua);
+  (void)(ua && v2ua);
+  (void)(v2ua || ua);
+  (void)(ua || v2ua);
+
+  (void)(v2ua & ua);
+  (void)(ua & v2ua);
+  (void)(v2ua | ua);
+  (void)(ua | v2ua);
+  (void)(v2ua ^ ua);
+  (void)(ua ^ v2ua);
+  (void)(v2ua << ua);
+  (void)(ua << v2ua);
+  (void)(v2ua >> ua);
+  (void)(ua >> v2ua);
+
+  v2ua += ua;
+  v2ua -= ua;
+  v2ua *= ua;
+  v2ua /= ua;
+  v2ua %= ua;
+  v2ua &= ua;
+  v2ua |= ua;
+  v2ua ^= ua;
+  v2ua >>= ua;
+  v2ua <<= ua;
+
+  ua += v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua -= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua *= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua /= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua %= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua &= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua |= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua ^= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible 
type 'v2u'}}
+  ua >>= v2ua; // expected-error{{assigning to 'unsigned int' from 
incompatible type 'v2u'}}
+  ua <<= v2ua; // expected-error{{assigning to 'unsigned int' from 
incompatible type 'v2u'}}
+}
+
+void test_float_vector_scalar(float fa, unsigned int ua, v2f v2fa) {
+  // Operations with one float vector and one scalar. These splat the scalar.
+  (void)(v2fa + fa);
+  (void)(fa + v2fa);
+  (void)(v2fa - fa);
+  (void)(fa - v2fa);
+  (void)(v2fa * fa);
+  (void)(fa * v2fa);
+  (void)(v2fa / fa);
+  (void)(fa / v2fa);
+  (void)(v2fa % fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa % v2fa); // expected-error{{invalid operands to binary expression}}
+
+  (void)(v2fa == fa);
+  (void)(fa == v2fa);
+  (void)(v2fa != fa);
+  (void)(fa != v2fa);
+  (void)(v2fa <= fa);
+  (void)(fa <= v2fa);
+  (void)(v2fa >= fa);
+  (void)(fa >= v2fa);
+  (void)(v2fa < fa);
+  (void)(fa < v2fa);
+  (void)(v2fa > fa);
+  (void)(fa > v2fa);
+  (void)(v2fa && fa);
+  (void)(fa && v2fa);
+  (void)(v2fa || fa);
+  (void)(fa || v2fa);
+
+  (void)(v2fa & fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa & v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa | fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa | v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa ^ fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa ^ v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa << fa); // expected-error{{used type 'v2f' (vector of 2 'float' 
values) where integer is required}}
+  (void)(v2fa << ua); // expected-error{{used type 'v2f' (vector of 2 'float' 
values) where integer is required}}
+  (void)(fa << v2fa); // expected-error{{used type 'float' where integer is 
required}}
+  (void)(ua << v2fa); // expected-error{{used type 'v2f' (vector of 2 'float' 
values) where integer is required}}
+  (void)(v2fa >> fa); // expected-error{{used type 'v2f' (vector of 2 'float' 
values) where integer is required}}
+  (void)(v2fa >> 

[PATCH] D123649: Allow flexible array initialization in C++.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

Looks like `Init`'s type is wrong. It should be `{ i32, [1 x i32] }` instead of 
`{ i32, [0 x i32] }` in the case where `S` has two members.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123649

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

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Erich Keane via Phabricator via cfe-commits]

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

In D150875#4362434 , @aaron.ballman 
wrote:

> Precommit CI found various failures that need to be addressed. Also, have you 
> tested to see what breaks in llvm-test-suite?

Woops!  I must have forgotten to re-make my diff before submitting.  I fixed 
those in the patch earlier.

I have not tested this against the llvm-test-suite as I'm not thrilled having 
to figure out how to build that again :)  But I guess I should make a companion 
patch ahead of time.  Last time however I don't recall us needing to add the 
flag for the warning however.


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

https://reviews.llvm.org/D150875

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

[PATCH] D151094: [clang] Implement P2564 "consteval must propagate up"

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 524491.
cor3ntin added a comment.

In FunctionScopeInfo, only track that an immediate-escalating expression 
has been found rather than which one - as that is recorded on 
the expression itself.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151094

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/ScopeInfo.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
  clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp
  clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -363,7 +363,7 @@
 
   consteval needs to propagate up
   https://wg21.link/P2564R3;>P2564R3 (DR)
-  No
+  Clang 17
 
 
   Lifetime extension in range-based for loops
Index: clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -0,0 +1,131 @@
+
+// RUN: %clang_cc1 -std=c++2a -emit-llvm-only -Wno-unused-value %s -verify
+// RUN: %clang_cc1 -std=c++2b -emit-llvm-only -Wno-unused-value %s -verify
+
+consteval int id(int i) { return i; }
+constexpr char id(char c) { return c; }
+
+namespace examples {
+
+template 
+constexpr int f(T t) { // expected-note {{declared here}}
+return t + id(t);  // expected-note {{'f' is an immediate function because its body contains a call to a consteval function 'id' and that call is not a constant expression}}
+}
+auto a = ; // ok, f is not an immediate function
+auto b = ;  // expected-error {{cannot take address of immediate function 'f' outside of an immediate invocation}}
+
+static_assert(f(3) == 6); // ok
+
+template 
+constexpr int g(T t) {// g is not an immediate function
+return t + id(42);// because id(42) is already a constant
+}
+
+template 
+constexpr bool is_not(T t, F f) {
+return not f(t);
+}
+
+consteval bool is_even(int i) { return i % 2 == 0; }
+
+static_assert(is_not(5, is_even));
+
+int x = 0; // expected-note {{declared here}}
+
+template 
+constexpr T h(T t = id(x)) { // expected-note {{read of non-const variable 'x' is not allowed in a constant expression}} \
+ // expected-note {{'hh' is an immediate function because its body contains a call to a consteval function 'id' and that call is not a constant expression}}
+return t;
+}
+
+template 
+constexpr T hh() {   // hh is an immediate function
+return h();
+}
+
+int i = hh(); // expected-error {{call to consteval function 'examples::hh' is not a constant expression}} \
+   // expected-note {{in call to 'hh()'}}
+
+struct A {
+  int x;
+  int y = id(x);
+};
+
+template 
+constexpr int k(int) {
+  return A(42).y;
+}
+
+}
+
+namespace e2{
+template 
+constexpr int f(T t);
+auto a = ;
+auto b = ;
+}
+
+namespace forward_declare_constexpr{
+template 
+constexpr int f(T t);
+
+auto a = ;
+auto b = ;
+
+template 
+constexpr int f(T t) {
+return id(0);
+}
+}
+
+namespace forward_declare_consteval{
+template 
+constexpr int f(T t);  // expected-note {{'f' defined here}}
+
+auto a = ;
+auto b = ; // expected-error {{immediate function 'f' used before it is defined}} \
+  // expected-note {{in instantiation of function template specialization}}
+
+template 
+constexpr int f(T t) {
+return id(t); // expected-note {{'f' is an immediate function because its body contains a call to a consteval function 'id' and that call is not a constant expression}}
+}
+}
+
+namespace constructors {
+consteval int f(int) {
+  return 0;
+}
+struct S {
+  constexpr S(auto i) {
+f(i);
+  }
+};
+constexpr void g(auto i) {
+  [[maybe_unused]] S s{i};
+}
+void test() {
+  g(0);
+}
+}
+
+namespace aggregate {
+consteval 

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Precommit CI found various failures that need to be addressed. Also, have you 
tested to see what breaks in llvm-test-suite?




Comment at: clang/docs/ReleaseNotes.rst:58
   Clang will only search for std::coroutine_traits for coroutines then.
+- Clang no longer allows dereferencing of a ``void*`` as an extension. Clang 16
+  converted this to a default-error as ``-Wvoid-ptr-dereference``, as well as a




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

https://reviews.llvm.org/D150875

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

[PATCH] D123649: Allow flexible array initialization in C++.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D123649#4362402 , @efriedma wrote:

> The assertion is correct; without it, your code is getting miscompiled.

The assertion may be correct in its intent, but we still should not be 
asserting in practice: https://godbolt.org/z/cs5bhvPGr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123649

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

[PATCH] D149516: [Sema] `setInvalidDecl` for error deduction declaration

[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 for a comment.




Comment at: clang/lib/Sema/SemaDeclCXX.cpp:11196
   if (D.isFunctionDefinition())
 Diag(D.getIdentifierLoc(), diag::err_deduction_guide_defines_function);
+  return IsValid;

HerrCai0907 wrote:
> rsmith wrote:
> > Should we return that we had an error after this one too?
> if `D.isFunctionDefinition()`, we can still create valid deduction guide. I 
> think keeping it can avoid lots of meaningless error later.
I think it's worth leaving that as a comment in the code so folks know it's 
done on purpose and why.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149516

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

[PATCH] D150403: [clang-format] Adjust braced list detection (try 2)

[Galen Elias via Phabricator via cfe-commits]

galenelias updated this revision to Diff 524484.
galenelias edited the summary of this revision.
galenelias added a comment.

Thanks for the additional review comments.  Hopefully everything if fixed.

In D150403#4358845 , @owenpan wrote:

> Can you put your authorship in the 'John Doe ' 
> 
>  format?

Galen Elias 

I'm not sure if there is somewhere I should be putting that in the summary or 
diff itself, or just here in the comments?


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

https://reviews.llvm.org/D150403

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

Index: clang/unittests/Format/TokenAnnotatorTest.cpp
===
--- clang/unittests/Format/TokenAnnotatorTest.cpp
+++ clang/unittests/Format/TokenAnnotatorTest.cpp
@@ -40,6 +40,8 @@
   EXPECT_EQ((FormatTok)->getType(), Type) << *(FormatTok)
 #define EXPECT_TOKEN_PRECEDENCE(FormatTok, Prec)   \
   EXPECT_EQ((FormatTok)->getPrecedence(), Prec) << *(FormatTok)
+#define EXPECT_BRACE_KIND(FormatTok, Kind) \
+  EXPECT_EQ(FormatTok->getBlockKind(), Kind) << *(FormatTok)
 #define EXPECT_TOKEN(FormatTok, Kind, Type)\
   do { \
 EXPECT_TOKEN_KIND(FormatTok, Kind);\
@@ -1783,6 +1785,22 @@
   EXPECT_TOKEN(Tokens[3], tok::colon, TT_CaseLabelColon);
 }
 
+TEST_F(TokenAnnotatorTest, UnderstandsNestedBlocks) {
+  // The closing braces are not annotated. It doesn't seem to cause a problem.
+  // So we only test for the opening braces.
+  auto Tokens = annotate("{\n"
+ "  {\n"
+ "{ int a = 0; }\n"
+ "  }\n"
+ "  {}\n"
+ "}");
+  ASSERT_EQ(Tokens.size(), 14u) << Tokens;
+  EXPECT_BRACE_KIND(Tokens[0], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[1], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[2], BK_Block);
+  EXPECT_BRACE_KIND(Tokens[10], BK_Block);
+}
+
 } // namespace
 } // namespace format
 } // namespace clang
Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13731,6 +13731,26 @@
"  struct Dummy {};\n"
"  f(v);\n"
"}");
+  verifyFormat("void foo() {\n"
+   "  { // asdf\n"
+   "{ int a; }\n"
+   "  }\n"
+   "  {\n"
+   "{ int b; }\n"
+   "  }\n"
+   "}");
+  verifyFormat("namespace n {\n"
+   "void foo() {\n"
+   "  {\n"
+   "{\n"
+   "  statement();\n"
+   "  if (false) {\n"
+   "  }\n"
+   "}\n"
+   "  }\n"
+   "  {}\n"
+   "}\n"
+   "} // namespace n");
 
   // Long lists should be formatted in columns even if they are nested.
   verifyFormat(
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -491,7 +491,11 @@
   // Keep a stack of positions of lbrace tokens. We will
   // update information about whether an lbrace starts a
   // braced init list or a different block during the loop.
-  SmallVector LBraceStack;
+  struct StackEntry {
+FormatToken *Tok;
+const FormatToken *PrevTok;
+  };
+  SmallVector LBraceStack;
   assert(Tok->is(tok::l_brace));
   do {
 // Get next non-comment token.
@@ -521,12 +525,12 @@
   } else {
 Tok->setBlockKind(BK_Unknown);
   }
-  LBraceStack.push_back(Tok);
+  LBraceStack.push_back({Tok, PrevTok});
   break;
 case tok::r_brace:
   if (LBraceStack.empty())
 break;
-  if (LBraceStack.back()->is(BK_Unknown)) {
+  if (LBraceStack.back().Tok->is(BK_Unknown)) {
 bool ProbablyBracedList = false;
 if (Style.Language == FormatStyle::LK_Proto) {
   ProbablyBracedList = NextTok->isOneOf(tok::comma, tok::r_square);
@@ -554,7 +558,7 @@
 
   // If we already marked the opening brace as braced list, the closing
   // must also be part of it.
-  ProbablyBracedList = LBraceStack.back()->is(TT_BracedListLBrace);
+  ProbablyBracedList = LBraceStack.back().Tok->is(TT_BracedListLBrace);
 
   ProbablyBracedList = ProbablyBracedList ||
(Style.isJavaScript() &&
@@ -570,8 +574,14 @@
  

[PATCH] D123649: Allow flexible array initialization in C++.

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

The assertion is correct; without it, your code is getting miscompiled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123649

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

[PATCH] D123649: Allow flexible array initialization in C++.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

The assertion also fails when the following code is compiled:

  struct S { int i[]; };
  S value{0};

According to C99, flexible array members have to belong to structs that have 
more than one named member. But clang allows that as a GNU extension although 
gcc seems to reject it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123649

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

[PATCH] D150913: [Clang][Bfloat16] Upgrade __bf16 to arithmetic type, change mangling, and extend excess precision support.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added inline comments.



Comment at: clang/test/CodeGen/X86/bfloat16.cpp:2-3
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --version 2
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -target-feature +fullbf16 
-S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | 
FileCheck -check-prefix=CHECK-NBF16 %s
+

codemzs wrote:
> zahiraam wrote:
> > pengfei wrote:
> > > The backend has already support lowering of `bfloat`, I don't think it's 
> > > necessary to do extra work in FE unless for excess-precision.
> > > The backend has already support lowering of `bfloat`, I don't think it's 
> > > necessary to do extra work in FE unless for excess-precision.
> > 
> > +1.
> @pengfei @zahiraam I added this test to verify bfloat16 IR gen functionality, 
> considering both scenarios: with and without native bfloat16 support. 
> However, if you believe it's more beneficial to omit it, I'm open to doing 
> so. Happy to also move this test to another target that doesn't have backend 
> support for emulation. 
I think that's fine. You can leave it. 


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

https://reviews.llvm.org/D150913

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

[PATCH] D150913: [Clang][Bfloat16] Upgrade __bf16 to arithmetic type, change mangling, and extend excess precision support.

[M. Zeeshan Siddiqui via Phabricator via cfe-commits]

codemzs added inline comments.



Comment at: clang/lib/Basic/Targets/X86.cpp:362-363
   HasX87 = true;
+} else if (Feature == "+fullbf16") {
+  HasFullBFloat16 = true;
 }

pengfei wrote:
> Maybe not need it.
Clarified on the other thread but if you have questions please feel free to 
post here and I will address them.



Comment at: clang/lib/Basic/Targets/X86.cpp:381
 
 HasBFloat16 = SSELevel >= SSE2;
 

pengfei wrote:
> zahiraam wrote:
> > codemzs wrote:
> > > rjmccall wrote:
> > > > pengfei wrote:
> > > > > I'm not sure if I understand the meaning of `HasFullBFloat16`. If it 
> > > > > is used for target that supports arithmetic `__bf16`, we should not 
> > > > > use `+fullbf16` but always enable it for SSE2, i.e., `HasFullBFloat16 
> > > > > = SSELevel >= SSE2`. Because X86 GCC already supports arithmetic for 
> > > > > `__bf16`.
> > > > > 
> > > > > If this is used in the way like `HasLegalHalfType`, we should enable 
> > > > > it once we have a full BF16 ISA on X86. `fullbf16` doesn't make much 
> > > > > sense to me.
> > > > At the moment, we haven't done the work to emulate BFloat16 arithmetic 
> > > > in any of the three ways we can do that: Clang doesn't promote it in 
> > > > IRGen, LLVM doesn't promote it in legalization, and we don't have 
> > > > compiler-rt functions for it.  If we emit these instructions, they'll 
> > > > just sail through LLVM and fail in the backend.  So in the short term, 
> > > > we have to restrict this to targets that directly support BFloat16 
> > > > arithmetic in hardware, which doesn't include x86.
> > > > 
> > > > Once we have that emulation support, I agree that the x86 targets 
> > > > should enable this whenever they would enable `__bf16`.
> > > @rjmccall, I concur and just wanted to confirm this change indeed intends 
> > > to provide `BFloat16` emulation support, utilizing excess precision for 
> > > promotion to `float`. The `HasFullBFloat16` switch is designed to 
> > > determine excess precision support automatically when the hardware does 
> > > not natively support `bfloat16` arithmetic.
> > > At the moment, we haven't done the work to emulate BFloat16 arithmetic in 
> > > any of the three ways we can do that: Clang doesn't promote it in IRGen, 
> > > LLVM doesn't promote it in legalization, and we don't have compiler-rt 
> > > functions for it.  If we emit these instructions, they'll just sail 
> > > through LLVM and fail in the backend.  So in the short term, we have to 
> > > restrict this to targets that directly support BFloat16 arithmetic in 
> > > hardware, which doesn't include x86.
> > > 
> > > Once we have that emulation support, I agree that the x86 targets should 
> > > enable this whenever they would enable `__bf16`.
> > 
> > Would be nice to add a comment to clarify it. 
> > LLVM doesn't promote it in legalization, and we don't have compiler-rt 
> > functions for it.
> 
> That's not true: https://godbolt.org/z/jxf5E83vG.
> 
> > The `HasFullBFloat16` switch is designed to determine excess precision 
> > support automatically when the hardware does not natively support bfloat16 
> > arithmetic.
> 
> Makes sense to me.
@pengfei, you're right. As part of D126953, the x86 backend received `bfloat16` 
emulation support. Also, I hope my explanation about the `HasFullBFloat16` flag 
addressed your questions. Please let me know if further clarification/change is 
needed.



Comment at: clang/test/CodeGen/X86/bfloat16.cpp:2-3
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --version 2
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -target-feature +fullbf16 
-S -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | 
FileCheck -check-prefix=CHECK-NBF16 %s
+

zahiraam wrote:
> pengfei wrote:
> > The backend has already support lowering of `bfloat`, I don't think it's 
> > necessary to do extra work in FE unless for excess-precision.
> > The backend has already support lowering of `bfloat`, I don't think it's 
> > necessary to do extra work in FE unless for excess-precision.
> 
> +1.
@pengfei @zahiraam I added this test to verify bfloat16 IR gen functionality, 
considering both scenarios: with and without native bfloat16 support. However, 
if you believe it's more beneficial to omit it, I'm open to doing so. Happy to 
also move this test to another target that doesn't have backend support for 
emulation. 



Comment at: clang/test/CodeGen/X86/fexcess-precision-bfloat16.c:7
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown \
+// RUN: -fbfloat16-excess-precision=fast -target-feature +fullbf16 \
+// RUN: -emit-llvm -o - %s | FileCheck -check-prefixes=CHECK-NO-EXT %s

pengfei wrote:
> The tests here make me guess you want to use `fullbf16` the same as 
> `HasLegalHalfType`.
Yes that is correct it is just to 

[PATCH] D150913: [Clang][Bfloat16] Upgrade __bf16 to arithmetic type, change mangling, and extend excess precision support.

[M. Zeeshan Siddiqui via Phabricator via cfe-commits]

codemzs updated this revision to Diff 524467.
codemzs marked 12 inline comments as done.
codemzs added a comment.

@pengfei, @zahiraam, I appreciate your feedback.

@pengfei, the `HasFullBFloat16` flag is primarily for identifying hardware with 
native `bfloat16` support to facilitate automatic excess precision support. I 
concur that since x86 possesses backend bfloat16 emulation (as noted in D126953 
), front-end emulation might not be 
necessary. The test's purpose was to provide coverage for this change. However, 
I am open to either removing it entirely or relocating it to a more suitable 
target as per your recommendation.


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

https://reviews.llvm.org/D150913

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/FPOptions.def
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/TargetInfo.h
  clang/include/clang/Driver/Options.td
  clang/lib/AST/Type.cpp
  clang/lib/Basic/TargetInfo.cpp
  clang/lib/Basic/Targets/AMDGPU.h
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/NVPTX.h
  clang/lib/Basic/Targets/X86.cpp
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CodeGen/X86/avx512bf16-error.c
  clang/test/CodeGen/X86/bfloat-mangle.cpp
  clang/test/CodeGen/X86/bfloat16.cpp
  clang/test/CodeGen/X86/fexcess-precision-bfloat16.c
  clang/test/CodeGenCUDA/amdgpu-bf16.cu
  clang/test/CodeGenCUDA/bf16.cu
  clang/test/Driver/fexcess-precision.c
  clang/test/Sema/arm-bf16-forbidden-ops.c
  clang/test/Sema/arm-bf16-forbidden-ops.cpp
  clang/test/Sema/arm-bfloat.cpp
  clang/test/SemaCUDA/amdgpu-bf16.cu
  clang/test/SemaCUDA/bf16.cu

Index: clang/test/SemaCUDA/bf16.cu
===
--- clang/test/SemaCUDA/bf16.cu
+++ clang/test/SemaCUDA/bf16.cu
@@ -2,32 +2,32 @@
 // REQUIRES: x86-registered-target
 
 // RUN: %clang_cc1 "-triple" "x86_64-unknown-linux-gnu" "-aux-triple" "nvptx64-nvidia-cuda" \
-// RUN:"-target-cpu" "x86-64" -fsyntax-only -verify=scalar %s
+// RUN:"-target-cpu" "x86-64" -fsyntax-only -verify=scalar -Wno-unused %s
 // RUN: %clang_cc1 "-aux-triple" "x86_64-unknown-linux-gnu" "-triple" "nvptx64-nvidia-cuda" \
-// RUN:-fcuda-is-device "-aux-target-cpu" "x86-64" -fsyntax-only -verify=scalar %s
+// RUN:-fcuda-is-device "-aux-target-cpu" "x86-64" -fsyntax-only -verify=scalar -Wno-unused %s
 
 #include "Inputs/cuda.h"
 
 __device__ void test(bool b, __bf16 *out, __bf16 in) {
   __bf16 bf16 = in; // No error on using the type itself.
 
-  bf16 + bf16; // scalar-error {{invalid operands to binary expression ('__bf16' and '__bf16')}}
-  bf16 - bf16; // scalar-error {{invalid operands to binary expression ('__bf16' and '__bf16')}}
-  bf16 * bf16; // scalar-error {{invalid operands to binary expression ('__bf16' and '__bf16')}}
-  bf16 / bf16; // scalar-error {{invalid operands to binary expression ('__bf16' and '__bf16')}}
+  bf16 + bf16;
+  bf16 - bf16;
+  bf16 * bf16;
+  bf16 / bf16;
 
   __fp16 fp16;
 
-  bf16 + fp16; // scalar-error {{invalid operands to binary expression ('__bf16' and '__fp16')}}
-  fp16 + bf16; // scalar-error {{invalid operands to binary expression ('__fp16' and '__bf16')}}
-  bf16 - fp16; // scalar-error {{invalid operands to binary expression ('__bf16' and '__fp16')}}
-  fp16 - bf16; // scalar-error {{invalid operands to binary expression ('__fp16' and '__bf16')}}
-  bf16 * fp16; // scalar-error {{invalid operands to binary expression ('__bf16' and '__fp16')}}
-  fp16 * bf16; // scalar-error {{invalid operands to binary expression ('__fp16' and '__bf16')}}
-  bf16 / fp16; // scalar-error {{invalid operands to binary expression ('__bf16' and '__fp16')}}
-  fp16 / bf16; // scalar-error {{invalid operands to binary expression ('__fp16' and '__bf16')}}
+  bf16 + fp16;
+  fp16 + bf16;
+  bf16 - fp16;
+  fp16 - bf16;
+  bf16 * fp16;
+  fp16 * bf16;
+  bf16 / fp16;
+  fp16 / bf16;
   bf16 = fp16; // scalar-error {{assigning to '__bf16' from incompatible type '__fp16'}}
   fp16 = bf16; // scalar-error {{assigning to '__fp16' from incompatible type '__bf16'}}
-  bf16 + (b ? fp16 : bf16); // scalar-error {{incompatible operand types ('__fp16' and '__bf16')}}
+  bf16 + (b ? fp16 : bf16);
   *out = bf16;
 }
Index: clang/test/SemaCUDA/amdgpu-bf16.cu
===
--- clang/test/SemaCUDA/amdgpu-bf16.cu
+++ clang/test/SemaCUDA/amdgpu-bf16.cu
@@ -1,13 +1,8 @@
 // REQUIRES: amdgpu-registered-target
 // REQUIRES: x86-registered-target
 
-// RUN: %clang_cc1 "-triple" "x86_64-unknown-linux-gnu" "-aux-triple" "amdgcn-amd-amdhsa"\
-// RUN:"-target-cpu" "x86-64" -fsyntax-only -verify=amdgcn %s
-// RUN: %clang_cc1 "-aux-triple" 

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

We should at least be able to `reinterpret_cast` between cases we know are 
compatible, as the OpenCL check does. One of the problems with the numerical 
address space is it doesn't have any information to know if that's strictly 
legal or not. I'm not sure if casting away an address space is always legal, 
it's generally what we do in LLVM-IR.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151087

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

[PATCH] D123649: Allow flexible array initialization in C++.

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

We found another case where the assertion fails.

$ cat test.cpp

  struct S {
int len;
int i[];
  };
  
  S value{0, 0};

$ clang++ -std=c++11 -c test.cpp

`CstSize` is only 4 while `VarSize` is 8.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123649

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

[PATCH] D150892: [clang][ExprConstant] fix __builtin_object_size for flexible array members

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:11740
+  LVal.getLValueBase().dyn_cast());
+  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+}

efriedma wrote:
> nickdesaulniers wrote:
> > efriedma wrote:
> > > nickdesaulniers wrote:
> > > > nickdesaulniers wrote:
> > > > > erichkeane wrote:
> > > > > > nickdesaulniers wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > Isn't this a possible null-deref?  
> > > > > > > I don't think so; in fact, I can use `cast` and `get` rather than 
> > > > > > > `dyn_cast_or_null` and `dyn_cast` here.
> > > > > > > 
> > > > > > > Just because we have a pointer doesn't mean it's possibly 
> > > > > > > `nullptr`; I don't think we can reach this code patch for 
> > > > > > > evaluating the `__builtin_object_size` of a struct with a 
> > > > > > > flexible array member if the LValue doesn't have a corresponding 
> > > > > > > VarDecl.
> > > > > > Of course that is a possibility.  You shouldn't use dyn_cast right 
> > > > > > before a dereference, this should definitely be using `cast`/`get`, 
> > > > > > since they assert.
> > > > > > 
> > > > > > What does the LValue have when the dynamic struct does not have an 
> > > > > > initializer?  Or just an empty one? 
> > > > > s/code patch/code path/
> > > > > What does the LValue have when the dynamic struct does not have an 
> > > > > initializer? Or just an empty one?
> > > > 
> > > > Tested:
> > > > ```
> > > > struct foo {
> > > > int x;
> > > > int y[];
> > > > };
> > > > 
> > > > static struct foo instance = {
> > > > .x = 3,
> > > > // .y = { 5, 10, 15, },
> > > > };
> > > > 
> > > > unsigned long foo (void) {
> > > > return __builtin_object_size(, 1);
> > > > }
> > > > ```
> > > > With the following values for `.y` in `instance`:
> > > > 1. `.y = { 5, 10, 15 }`: `foo` returns `16`.
> > > > 2. `.y = {},`: `foo` returns `4`.
> > > > 3. <`.y` is not specified or explicitly initialized>: `foo` returns `4`.
> > > This might have been missed in the discussion here, but what actually 
> > > ensures we have a VarDecl here?  I guess most other LValueBases can't 
> > > actually have a struct type, but a CompoundLiteralExpr can. (Looking 
> > > briefly at the code, I can't find any other possibilities at the moment, 
> > > but in any case, the assumption seems pretty fragile.)
> > I assume that the code works as such:
> > ```
> > struct foo my_foo;
> > return __builtin_object_size(, 1);
> > ```
> > `foo` is a `VarDecl` otherwise how do you take the address?  What other 
> > expression could be passed to `__builtin_object_size` that isn't a 
> > reference to a variable?
> `__builtin_object_size(&(struct S){}, 1)`?  Not really a useful construct, 
> but we don't reject it.
https://reviews.llvm.org/D151148


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150892

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

[PATCH] D151148: [clang][ExprConstant] fix __builtin_object_size for compound literal

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: efriedma, erichkeane.
Herald added a project: All.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fixup to D150892 .


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151148

Files:
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGen/object-size.c


Index: clang/test/CodeGen/object-size.c
===
--- clang/test/CodeGen/object-size.c
+++ clang/test/CodeGen/object-size.c
@@ -551,6 +551,10 @@
   // CHECK: ret i64 16
   return OBJECT_SIZE_BUILTIN(, 1);
 }
+unsigned long test35(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(&(struct DynStructVar){}, 1);
+}
 
 // CHECK-LABEL: @PR30346
 void PR30346(void) {
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11736,9 +11736,8 @@
 bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
 if (Ty->isStructureType() &&
 Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {
-  const auto *VD =
-  cast(LVal.getLValueBase().get());
-  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+  if (const auto *V = LVal.getLValueBase().dyn_cast())
+Result += cast(V)->getFlexibleArrayInitChars(Info.Ctx);
 }
 return Ret;
   };


Index: clang/test/CodeGen/object-size.c
===
--- clang/test/CodeGen/object-size.c
+++ clang/test/CodeGen/object-size.c
@@ -551,6 +551,10 @@
   // CHECK: ret i64 16
   return OBJECT_SIZE_BUILTIN(, 1);
 }
+unsigned long test35(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(&(struct DynStructVar){}, 1);
+}
 
 // CHECK-LABEL: @PR30346
 void PR30346(void) {
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11736,9 +11736,8 @@
 bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
 if (Ty->isStructureType() &&
 Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {
-  const auto *VD =
-  cast(LVal.getLValueBase().get());
-  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+  if (const auto *V = LVal.getLValueBase().dyn_cast())
+Result += cast(V)->getFlexibleArrayInitChars(Info.Ctx);
 }
 return Ret;
   };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D151087#4361237 , @ebevhan wrote:

> What is now a reinterpret_cast? An address space conversion, or a bitcast? 
> It's not as straightforward as it might seem.

This is the most straightforward part. It's a bitcast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151087

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

[PATCH] D151145: Add disabled unittest reproducing TextProto formatting issue.

[Ben J via Phabricator via cfe-commits]

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

Github Issue 

Basically adding an empty `TextProto` section causes the formatting of 
`TextProto` raw strings to change and spaces to be added after the `: `. 
This diff adds a test that reproduces this issus.
It is currently disabled as it is failing, here is the output:

  ❯ build/tools/clang/unittests/Format/FormatTests 
--gtest_filter='FormatTestRawStrings.DISABLED*' --gtest_also_run_disabled_tests
  Note: Google Test filter = FormatTestRawStrings.DISABLED*
  [==] Running 1 test from 1 test suite.
  [--] Global test environment set-up.
  [--] 1 test from FormatTestRawStrings
  [ RUN  ] 
FormatTestRawStrings.DISABLED_FormatsCorrectlyWhenTextProtoIsInOwnSection
  
/home/bjudd/code/llvm-project/clang/unittests/Format/FormatTestRawStrings.cpp:134:
 Failure
  Expected equality of these values:
Expected
  Which is: "\nt = R\"pb(item: 1)pb\";"
Actual
  Which is: "\nt = R\"pb(item : 1)pb\";"
  With diff:
  @@ -1,2 +1,2 @@
  
  -t = R\"pb(item: 1)pb\";
  +t = R\"pb(item : 1)pb\";
  
  [  FAILED  ] 
FormatTestRawStrings.DISABLED_FormatsCorrectlyWhenTextProtoIsInOwnSection (7 ms)
  [--] 1 test from FormatTestRawStrings (7 ms total)
  
  [--] Global test environment tear-down
  [==] 1 test from 1 test suite ran. (7 ms total)
  [  PASSED  ] 0 tests.
  [  FAILED  ] 1 test, listed below:
  [  FAILED  ] 
FormatTestRawStrings.DISABLED_FormatsCorrectlyWhenTextProtoIsInOwnSection
  
   1 FAILED TEST


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151145

Files:
  clang/unittests/Format/FormatTestRawStrings.cpp


Index: clang/unittests/Format/FormatTestRawStrings.cpp
===
--- clang/unittests/Format/FormatTestRawStrings.cpp
+++ clang/unittests/Format/FormatTestRawStrings.cpp
@@ -75,6 +75,31 @@
 return Style;
   }

+  FormatStyle getRawStringPbStyleSeparateSection() {
+constexpr const char *FormatStr = R"format(---
+BasedOnStyle: LLVM
+ColumnLimit: 40
+RawStringFormats:
+  - Language: TextProto
+Delimiters:
+  - 'pb'
+BasedOnStyle: google
+---
+Language: TextProto
+# Don't set any options.
+...
+)format";
+llvm::MemoryBufferRef ConfigBuffer(FormatStr, "ClangFormatConfig");
+FormatStyle Style = {};
+Style.Language = FormatStyle::LK_Cpp;
+if (auto Err = parseConfiguration(ConfigBuffer, )) {
+  llvm::dbgs() << Err.value();
+  llvm::dbgs() << Err.message();
+  assert(Err);
+}
+return Style;
+  }
+
   FormatStyle getRawStringLLVMCppStyleBasedOn(std::string BasedOnStyle) {
 FormatStyle Style = getLLVMStyle();
 Style.RawStringFormats = {
@@ -164,6 +189,18 @@
getRawStringPbStyleWithColumns(40)));
 }

+// Currently a bug in how configs are processed:
+// See: https://github.com/llvm/llvm-project/issues/62871
+TEST_F(FormatTestRawStrings,
+   DISABLED_FormatsCorrectlyWhenTextProtoIsInOwnSection) {
+  // TextProto formatting works when in a RawString.
+  expect_eq(R"test(
+t = R"pb(item: 1)pb";)test",
+format(R"test(
+t = R"pb(item:1)pb";)test",
+   getRawStringPbStyleSeparateSection()));
+}
+
 TEST_F(FormatTestRawStrings, RespectsClangFormatOff) {
   expect_eq(R"test(
 // clang-format off


Index: clang/unittests/Format/FormatTestRawStrings.cpp
===
--- clang/unittests/Format/FormatTestRawStrings.cpp
+++ clang/unittests/Format/FormatTestRawStrings.cpp
@@ -75,6 +75,31 @@
 return Style;
   }

+  FormatStyle getRawStringPbStyleSeparateSection() {
+constexpr const char *FormatStr = R"format(---
+BasedOnStyle: LLVM
+ColumnLimit: 40
+RawStringFormats:
+  - Language: TextProto
+Delimiters:
+  - 'pb'
+BasedOnStyle: google
+---
+Language: TextProto
+# Don't set any options.
+...
+)format";
+llvm::MemoryBufferRef ConfigBuffer(FormatStr, "ClangFormatConfig");
+FormatStyle Style = {};
+Style.Language = FormatStyle::LK_Cpp;
+if (auto Err = parseConfiguration(ConfigBuffer, )) {
+  llvm::dbgs() << Err.value();
+  llvm::dbgs() << Err.message();
+  assert(Err);
+}
+return Style;
+  }
+
   FormatStyle getRawStringLLVMCppStyleBasedOn(std::string BasedOnStyle) {
 FormatStyle Style = getLLVMStyle();
 Style.RawStringFormats = {
@@ -164,6 +189,18 @@
getRawStringPbStyleWithColumns(40)));
 }

+// Currently a bug in how configs are processed:
+// See: https://github.com/llvm/llvm-project/issues/62871
+TEST_F(FormatTestRawStrings,
+   DISABLED_FormatsCorrectlyWhenTextProtoIsInOwnSection) {
+  // TextProto formatting works when in a RawString.
+  expect_eq(R"test(
+t = 

[PATCH] D150892: [clang][ExprConstant] fix __builtin_object_size for flexible array members

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:11740
+  LVal.getLValueBase().dyn_cast());
+  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+}

nickdesaulniers wrote:
> efriedma wrote:
> > nickdesaulniers wrote:
> > > nickdesaulniers wrote:
> > > > erichkeane wrote:
> > > > > nickdesaulniers wrote:
> > > > > > erichkeane wrote:
> > > > > > > Isn't this a possible null-deref?  
> > > > > > I don't think so; in fact, I can use `cast` and `get` rather than 
> > > > > > `dyn_cast_or_null` and `dyn_cast` here.
> > > > > > 
> > > > > > Just because we have a pointer doesn't mean it's possibly 
> > > > > > `nullptr`; I don't think we can reach this code patch for 
> > > > > > evaluating the `__builtin_object_size` of a struct with a flexible 
> > > > > > array member if the LValue doesn't have a corresponding VarDecl.
> > > > > Of course that is a possibility.  You shouldn't use dyn_cast right 
> > > > > before a dereference, this should definitely be using `cast`/`get`, 
> > > > > since they assert.
> > > > > 
> > > > > What does the LValue have when the dynamic struct does not have an 
> > > > > initializer?  Or just an empty one? 
> > > > s/code patch/code path/
> > > > What does the LValue have when the dynamic struct does not have an 
> > > > initializer? Or just an empty one?
> > > 
> > > Tested:
> > > ```
> > > struct foo {
> > > int x;
> > > int y[];
> > > };
> > > 
> > > static struct foo instance = {
> > > .x = 3,
> > > // .y = { 5, 10, 15, },
> > > };
> > > 
> > > unsigned long foo (void) {
> > > return __builtin_object_size(, 1);
> > > }
> > > ```
> > > With the following values for `.y` in `instance`:
> > > 1. `.y = { 5, 10, 15 }`: `foo` returns `16`.
> > > 2. `.y = {},`: `foo` returns `4`.
> > > 3. <`.y` is not specified or explicitly initialized>: `foo` returns `4`.
> > This might have been missed in the discussion here, but what actually 
> > ensures we have a VarDecl here?  I guess most other LValueBases can't 
> > actually have a struct type, but a CompoundLiteralExpr can. (Looking 
> > briefly at the code, I can't find any other possibilities at the moment, 
> > but in any case, the assumption seems pretty fragile.)
> I assume that the code works as such:
> ```
> struct foo my_foo;
> return __builtin_object_size(, 1);
> ```
> `foo` is a `VarDecl` otherwise how do you take the address?  What other 
> expression could be passed to `__builtin_object_size` that isn't a reference 
> to a variable?
`__builtin_object_size(&(struct S){}, 1)`?  Not really a useful construct, but 
we don't reject it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150892

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

[PATCH] D150892: [clang][ExprConstant] fix __builtin_object_size for flexible array members

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:11740
+  LVal.getLValueBase().dyn_cast());
+  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+}

efriedma wrote:
> nickdesaulniers wrote:
> > nickdesaulniers wrote:
> > > erichkeane wrote:
> > > > nickdesaulniers wrote:
> > > > > erichkeane wrote:
> > > > > > Isn't this a possible null-deref?  
> > > > > I don't think so; in fact, I can use `cast` and `get` rather than 
> > > > > `dyn_cast_or_null` and `dyn_cast` here.
> > > > > 
> > > > > Just because we have a pointer doesn't mean it's possibly `nullptr`; 
> > > > > I don't think we can reach this code patch for evaluating the 
> > > > > `__builtin_object_size` of a struct with a flexible array member if 
> > > > > the LValue doesn't have a corresponding VarDecl.
> > > > Of course that is a possibility.  You shouldn't use dyn_cast right 
> > > > before a dereference, this should definitely be using `cast`/`get`, 
> > > > since they assert.
> > > > 
> > > > What does the LValue have when the dynamic struct does not have an 
> > > > initializer?  Or just an empty one? 
> > > s/code patch/code path/
> > > What does the LValue have when the dynamic struct does not have an 
> > > initializer? Or just an empty one?
> > 
> > Tested:
> > ```
> > struct foo {
> > int x;
> > int y[];
> > };
> > 
> > static struct foo instance = {
> > .x = 3,
> > // .y = { 5, 10, 15, },
> > };
> > 
> > unsigned long foo (void) {
> > return __builtin_object_size(, 1);
> > }
> > ```
> > With the following values for `.y` in `instance`:
> > 1. `.y = { 5, 10, 15 }`: `foo` returns `16`.
> > 2. `.y = {},`: `foo` returns `4`.
> > 3. <`.y` is not specified or explicitly initialized>: `foo` returns `4`.
> This might have been missed in the discussion here, but what actually ensures 
> we have a VarDecl here?  I guess most other LValueBases can't actually have a 
> struct type, but a CompoundLiteralExpr can. (Looking briefly at the code, I 
> can't find any other possibilities at the moment, but in any case, the 
> assumption seems pretty fragile.)
I assume that the code works as such:
```
struct foo my_foo;
return __builtin_object_size(, 1);
```
`foo` is a `VarDecl` otherwise how do you take the address?  What other 
expression could be passed to `__builtin_object_size` that isn't a reference to 
a variable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150892

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

[PATCH] D148573: Allow -fsanitize=function on all targets

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D148573#4361894 , @sbc100 wrote:

> In D148573#4361509 , @MaskRay wrote:
>
>> In D148573#4361396 , @sbc100 wrote:
>>
>>> This change seems to be causing problems on the emscripten auto-roller:  
>>> https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8780394114149321217/overview
>>>
>>> Failures show up in ubsan tests and look like this:
>>>
>>>   error: symbol '_Z4testi' unsupported subtraction expression used in 
>>> relocation in code section.
>>>   error: symbol '__main_argc_argv' unsupported subtraction expression used 
>>> in relocation in code section.
>>>   fatal error: error in backend: function sections must contain one 
>>> function each
>>>
>>> It seems like enabling this sanitizer perhaps uses features we don't yet 
>>> support?  I will keep investigating but perhaps we can find a way to revert 
>>> he effect on the wasm backend for now?
>>
>> wasm seems to use `-fsanitize=undefined`, which includes 
>> `-fsanitize=function`.
>> wasm doesn't allow data words before the function entry, so we need to 
>> unsupport `-fsanitize=function` for wasm...
>
> That makes sense to me. The wasm specification (and therefore the wasm 
> runtimes) already enforce signature checking for indirect function calls so 
> there should be no need for this sanitizer there anyway.Do you want to 
> make that change or should I?

Thanks for the additional context (and I shall learn about it). Having been 
done in 39ba913d13ab15c76cb6b5aa066fa111ddfe944b 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148573

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

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

The address space feature is governed by TR 18037 
(https://standards.iso.org/ittf/PubliclyAvailableStandards/c051126_ISO_IEC_TR_18037_2008.zip),
 see section 5. The TR says (in part):

> A non-null pointer into an address space A can be cast to a pointer into 
> another address space B,
> but such a cast is undefined if the source pointer does not point to a 
> location in B. Note that if A is a
> subset of B, the cast is always valid; however, if B is a subset of A, the 
> cast is valid only if the
> source pointer refers to a location in B. A null pointer into one address 
> space can be cast to a null
> pointer into any overlapping address space.

It's pretty well-established that C++ provides a named cast to perform almost 
any operation that can be done in a C-style cast, so I think this is a good 
discussion on what named cast to surface this functionality under.

`reinterpret_cast` in C++ is intended to give some measure of better guarantees 
than a C-style cast, but it is not allowed to "fail" (in the sense of a 
`dynamic_cast`). Because not all address space casts can be correctly handled 
(due to pointer size differences, for example), it seems to me that allowing 
the conversion through `reinterpret_cast` would not be appropriate; it stops 
being a bit-cast operation and starts being a conversion operation instead.

I looked to see if WG21 has made any mention about address spaces within the 
context of the standard, and I did not see anything relevant (but it's possible 
I missed something on one of the various study group mailing lists). So I did 
the next best thing and started quietly asking a few relevant WG21 folks for 
opinions and learned some things.

The current SG1 position within WG21 is that address spaces are not something 
that ISO C++ should have. However, this is not a universally held opinion, some 
SG1 members would like to see address spaces added. That said, the address 
space is part of the pointer's provenance and only a few address space casts 
can produce provenance that works at all. You could build up from casts to and 
from uintptr_t/intptr_t that round-trip and then document which address spaces 
can be converted through this mechanism (with the default answer being "none"). 
When the address spaces are nested, then the casts are desirable and should be 
reasonably ergonomic. There was sentiment to go with `__addrspace_cast` that 
doesn't let you change any other property of the pointer but its address space 
and make it UB at the point of cast if that conversion is going to do bad 
things (so we can diagnose immediately at the cast site).

Based on all this, I think we should go with `__addrspace_cast` as a named cast 
and not allow the conversion through `reinterpret_cast` unless going to/from a 
`[u]intptr_t`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151087

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

[PATCH] D150892: [clang][ExprConstant] fix __builtin_object_size for flexible array members

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:11740
+  LVal.getLValueBase().dyn_cast());
+  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+}

nickdesaulniers wrote:
> nickdesaulniers wrote:
> > erichkeane wrote:
> > > nickdesaulniers wrote:
> > > > erichkeane wrote:
> > > > > Isn't this a possible null-deref?  
> > > > I don't think so; in fact, I can use `cast` and `get` rather than 
> > > > `dyn_cast_or_null` and `dyn_cast` here.
> > > > 
> > > > Just because we have a pointer doesn't mean it's possibly `nullptr`; I 
> > > > don't think we can reach this code patch for evaluating the 
> > > > `__builtin_object_size` of a struct with a flexible array member if the 
> > > > LValue doesn't have a corresponding VarDecl.
> > > Of course that is a possibility.  You shouldn't use dyn_cast right before 
> > > a dereference, this should definitely be using `cast`/`get`, since they 
> > > assert.
> > > 
> > > What does the LValue have when the dynamic struct does not have an 
> > > initializer?  Or just an empty one? 
> > s/code patch/code path/
> > What does the LValue have when the dynamic struct does not have an 
> > initializer? Or just an empty one?
> 
> Tested:
> ```
> struct foo {
> int x;
> int y[];
> };
> 
> static struct foo instance = {
> .x = 3,
> // .y = { 5, 10, 15, },
> };
> 
> unsigned long foo (void) {
> return __builtin_object_size(, 1);
> }
> ```
> With the following values for `.y` in `instance`:
> 1. `.y = { 5, 10, 15 }`: `foo` returns `16`.
> 2. `.y = {},`: `foo` returns `4`.
> 3. <`.y` is not specified or explicitly initialized>: `foo` returns `4`.
This might have been missed in the discussion here, but what actually ensures 
we have a VarDecl here?  I guess most other LValueBases can't actually have a 
struct type, but a CompoundLiteralExpr can. (Looking briefly at the code, I 
can't find any other possibilities at the moment, but in any case, the 
assumption seems pretty fragile.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150892

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

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D147844#4335598 , @dblaikie wrote:

> In D147844#4329497 , @aaron.ballman 
> wrote:
>
>> In general, I think this is incremental progress on the diagnostic behavior. 
>> However, it's clear that there is room for interpretation on what is or is 
>> not a false positive diagnostic for this,
>
> I hope we can agree on what a false positive is here - when the warning fires 
> but the code is what the developer intended (ie: the existing code with the 
> existing language semantics produce the desired result, the "fix" is to add 
> parentheses that explicitly encode the language's existing rules/behavior 
> anyway).

I agree with that definition -- that's a useful way to approach this, thank you!

> Not that we don't have warnings that do this - that encourage parens to 
> reinforce what the language already does to be more explicit/intentional 
> about it, and in some cases it's not that uncommon that the user will be 
> adding parens that reinforce the precedence rules anyway.

Yup, which is largely what this patch is about.

> Like, I think all the fixes in libc++, llvm, etc, are false positives? (none 
> of them found bugs/unintended behavior)

Yes, they all are false positives by the above definition.

> Are there any examples of bugs being found by this warning in a codebase? (& 
> how many false positives in such a codebase did it also flag?)

This would be good to know, but a bit of a heavy lift to require of @chaitanyav 
because they were working on this issue 
(https://github.com/llvm/llvm-project/issues/61943) with a "good first issue" 
label that is starting to look a bit like it was misleading (sorry about 
that!). However, if you're able to try compiling some larger projects with your 
patch applied to see if it spots any bugs in real world code, that would be 
very helpful!

>> so we should pay close attention to user feedback during the 17.x release 
>> cycle. If it seems we're getting significant push back, we may need to come 
>> back and rethink.
>>
>> Specifically, I think we've improved the situation for code like this:
>>
>>   // `p` is a pointer, `x` is an `int`, and `b` is a `bool`
>>   p + x ? 1 : 2; // previously didn't warn, now warns
>>   p + b ? 1 : 2; // always warned
>>
>> Does anyone feel we should not move forward with accepting the patch in its 
>> current form?
>
> *goes to look*
>
> Mixed feelings - you're right, incremental improvement/adding missed cases to 
> an existing warning (especially if that warning's already stylistic in 
> nature) is a lower bar/doesn't necessarily need the false positive 
> assessment. But it looks like this case might've been intentionally 
> suppressed in the initial warning implementation? (anyone linked to the 
> original warning implementation/review/design to check if this was 
> intentional?)

I tried to chase down where this got added to see what review comments there 
were, but it seems to predate my involvement (I'm seeing mentions of this in 
2011).

> But, yeah, seems marginal enough I don't have strong feelings either way.

Thank you for the opinion! I think pointer + int is a far more common code 
pattern than pointer + bool, so it makes some sense to me that we would silence 
the first case while diagnosing the second case. Given the general lack of 
enthusiasm for the new diagnostics, it may boil down to answering whether this 
finds any true positives or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

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

[PATCH] D151047: [clang-format] Fix indentation for selective formatting.

[Björn Schäpers via Phabricator via cfe-commits]

HazardyKnusperkeks accepted this revision.
HazardyKnusperkeks added a comment.

Just a side note, please link to commits, or even better the reviews here when 
talking about old commits. I found it through the github issue, but I think the 
other way around is better. I was interested in the commit, and only afterwards 
in what it thought to fix (aka the issue).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151047

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

[PATCH] D143675: Discussion: Darwin Sanitizers Stable ABI

[Roy Sundahl via Phabricator via cfe-commits]

rsundahl added a comment.

Hello Egenii,

Thank you for your time and consideration of this PR. Since you last commented, 
@vitalybuka has approved the PR and added @maskray and yourself as blocking 
reviewers. @maskray has approved and we are awaiting your approval if you 
remain positive to it (or let us know if you have any new reservations).

Thank you,
-Roy Sundahl


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143675

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

[PATCH] D151060: [Clang][Sema] Generate vector vs scalar builtin overloads

[Cassie Jones via Phabricator via cfe-commits]

porglezomp updated this revision to Diff 524432.
porglezomp added a comment.

Rebasing onto modified tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151060

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -728,12 +728,10 @@
   (void)(ea | v2ua); // expected-error{{cannot convert between vector values of different size}}
   (void)(v2ua ^ ea); // expected-error{{cannot convert between vector values of different size}}
   (void)(ea ^ v2ua); // expected-error{{cannot convert between vector values of different size}}
-  // FIXME: Vector/scalar shifts cause an assertion failure
-  // https://github.com/llvm/llvm-project/issues/62870
-  // (void)(v2ua << ea);
-  // (void)(ea << v2ua);
-  // (void)(v2ua >> ea);
-  // (void)(ea >> v2ua);
+  (void)(v2ua << ea);
+  (void)(ea << v2ua);
+  (void)(v2ua >> ea);
+  (void)(ea >> v2ua);
 
   v2ua += ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua -= ea; // expected-error{{cannot convert between vector values of different size}}
@@ -743,10 +741,8 @@
   v2ua &= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua |= ea; // expected-error{{cannot convert between vector values of different size}}
   v2ua ^= ea; // expected-error{{cannot convert between vector values of different size}}
-  // FIXME: Vector/scalar shifts cause an assertion failure
-  // https://github.com/llvm/llvm-project/issues/62870
-  // v2ua >>= ea;
-  // v2ua <<= ea;
+  v2ua >>= ea;
+  v2ua <<= ea;
 
   ea += v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea -= v2ua; // expected-error{{cannot convert between vector values of different size}}
@@ -756,10 +752,8 @@
   ea &= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea |= v2ua; // expected-error{{cannot convert between vector values of different size}}
   ea ^= v2ua; // expected-error{{cannot convert between vector values of different size}}
-  // FIXME: Vector/scalar shifts cause an assertion failure
-  // https://github.com/llvm/llvm-project/issues/62870
-  // ea >>= v2ua; // not-expected-error{{assigning to 'enum Enum' from incompatible type 'v2u'}}
-  // ea <<= v2ua; // not-expected-error{{assigning to 'enum Enum' from incompatible type 'v2u'}}
+  ea >>= v2ua; // expected-error{{assigning to 'Enum' from incompatible type 'v2u'}}
+  ea <<= v2ua; // expected-error{{assigning to 'Enum' from incompatible type 'v2u'}}
 }
 
 #if __cplusplus >= 201103L // C++11 or later
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9024,6 +9024,30 @@
 S.AddBuiltinCandidate(LandR, Args, CandidateSet);
   }
 }
+
+// Extension: Add the binary bitwise operators for vector types
+if (!CandidateTypes[0].vector_types().empty() &&
+!CandidateTypes[1].vector_types().empty()) {
+  // If both candidates have vector types, then add them pairwise...
+  for (QualType Vec1Ty : CandidateTypes[0].vector_types()) {
+for (QualType Vec2Ty : CandidateTypes[1].vector_types()) {
+  QualType LandR[2] = {Vec1Ty, Vec2Ty};
+  S.AddBuiltinCandidate(LandR, Args, CandidateSet);
+}
+  }
+} else {
+  // ...but if only one side has vector candidates, use that set of
+  // vector candidates pairwise for both sides of the operator
+  // (allowing splatting the scalar to a vector).
+  for (unsigned Candidate = 0; Candidate < 2; ++Candidate) {
+for (QualType Vec1Ty : CandidateTypes[Candidate].vector_types()) {
+  for (QualType Vec2Ty : CandidateTypes[Candidate].vector_types()) {
+QualType LandR[2] = {Vec1Ty, Vec2Ty};
+S.AddBuiltinCandidate(LandR, Args, CandidateSet);
+  }
+}
+  }
+}
   }
 
   // C++ [over.built]p20:
@@ -9250,6 +9274,25 @@
 });
   }
 }
+
+// Extension: Add the binary operators %=, <<=, >>=, &=, ^=, |= for vector types.
+for (QualType Vec1Ty : CandidateTypes[0].vector_types())
+  for (QualType Vec2Ty : CandidateTypes[0].vector_types()) {
+QualType ParamTypes[2];
+ParamTypes[1] = Vec2Ty;
+// Add this built-in operator as a candidate (VQ is empty).
+ParamTypes[0] = S.Context.getLValueReferenceType(Vec1Ty);
+S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
+  /*IsAssignmentOperator=*/false);
+
+// Add this built-in operator as a candidate (VQ is 'volatile').
+if (VisibleTypeConversionsQuals.hasVolatile()) {
+  ParamTypes[0] = 

[PATCH] D151059: [test] Add C++ ext_vector_type tests

[Florian Hahn via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151059

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

[PATCH] D151137: ]NFC][Clang] Fix Coverity bug with dereference null return value in clang::​CodeGen::​CodeGenFunction::​EmitOMPArraySectionExpr()

[Soumi Manna via Phabricator via cfe-commits]

Manna added a comment.

Thank you @erichkeane for reviews!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151137

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

[PATCH] D151059: [test] Add C++ ext_vector_type tests

[Cassie Jones via Phabricator via cfe-commits]

porglezomp updated this revision to Diff 524429.
porglezomp added a comment.

Address review comments.
File GitHub issues and reference them with the FIXMEs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151059

Files:
  clang/test/SemaCXX/vector.cpp

Index: clang/test/SemaCXX/vector.cpp
===
--- clang/test/SemaCXX/vector.cpp
+++ clang/test/SemaCXX/vector.cpp
@@ -545,3 +545,230 @@
   auto b4 = (v4 >= 0x12);
 }
 #endif
+
+namespace all_operators {
+typedef unsigned int v2u __attribute__((ext_vector_type(2)));
+typedef float v2f __attribute__((ext_vector_type(2)));
+
+void test_int_vector_scalar(unsigned int ua, v2u v2ua) {
+  // Operators with one integer vector and one integer scalar operand. The scalar will splat.
+  (void)(v2ua + ua);
+  (void)(ua + v2ua);
+  (void)(v2ua - ua);
+  (void)(ua - v2ua);
+  (void)(v2ua * ua);
+  (void)(ua * v2ua);
+  (void)(v2ua / ua);
+  (void)(ua / v2ua);
+  (void)(v2ua % ua);
+  (void)(ua % v2ua);
+
+  (void)(v2ua == ua);
+  (void)(ua == v2ua);
+  (void)(v2ua != ua);
+  (void)(ua != v2ua);
+  (void)(v2ua <= ua);
+  (void)(ua <= v2ua);
+  (void)(v2ua >= ua);
+  (void)(ua >= v2ua);
+  (void)(v2ua < ua);
+  (void)(ua < v2ua);
+  (void)(v2ua > ua);
+  (void)(ua > v2ua);
+  (void)(v2ua && ua);
+  (void)(ua && v2ua);
+  (void)(v2ua || ua);
+  (void)(ua || v2ua);
+
+  (void)(v2ua & ua);
+  (void)(ua & v2ua);
+  (void)(v2ua | ua);
+  (void)(ua | v2ua);
+  (void)(v2ua ^ ua);
+  (void)(ua ^ v2ua);
+  (void)(v2ua << ua);
+  (void)(ua << v2ua);
+  (void)(v2ua >> ua);
+  (void)(ua >> v2ua);
+
+  v2ua += ua;
+  v2ua -= ua;
+  v2ua *= ua;
+  v2ua /= ua;
+  v2ua %= ua;
+  v2ua &= ua;
+  v2ua |= ua;
+  v2ua ^= ua;
+  v2ua >>= ua;
+  v2ua <<= ua;
+
+  ua += v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua -= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua *= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua /= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua %= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua &= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua |= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua ^= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua >>= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua <<= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+}
+
+void test_float_vector_scalar(float fa, unsigned int ua, v2f v2fa) {
+  // Operators with one float vector and one float scalar operand. The scalar will splat.
+  (void)(v2fa + fa);
+  (void)(fa + v2fa);
+  (void)(v2fa - fa);
+  (void)(fa - v2fa);
+  (void)(v2fa * fa);
+  (void)(fa * v2fa);
+  (void)(v2fa / fa);
+  (void)(fa / v2fa);
+  (void)(v2fa % fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa % v2fa); // expected-error{{invalid operands to binary expression}}
+
+  (void)(v2fa == fa);
+  (void)(fa == v2fa);
+  (void)(v2fa != fa);
+  (void)(fa != v2fa);
+  (void)(v2fa <= fa);
+  (void)(fa <= v2fa);
+  (void)(v2fa >= fa);
+  (void)(fa >= v2fa);
+  (void)(v2fa < fa);
+  (void)(fa < v2fa);
+  (void)(v2fa > fa);
+  (void)(fa > v2fa);
+  (void)(v2fa && fa);
+  (void)(fa && v2fa);
+  (void)(v2fa || fa);
+  (void)(fa || v2fa);
+
+  (void)(v2fa & fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa & v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa | fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa | v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa ^ fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa ^ v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa << fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa << ua); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(fa << v2fa); // expected-error{{used type 'float' where integer is required}}
+  (void)(ua << v2fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa >> fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa >> ua); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(fa >> v2fa); // expected-error{{used type 'float' where integer is required}}
+  (void)(ua >> v2fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is 

[PATCH] D148573: Allow -fsanitize=function on all targets

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

In D148573#4361509 , @MaskRay wrote:

> In D148573#4361396 , @sbc100 wrote:
>
>> This change seems to be causing problems on the emscripten auto-roller:  
>> https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8780394114149321217/overview
>>
>> Failures show up in ubsan tests and look like this:
>>
>>   error: symbol '_Z4testi' unsupported subtraction expression used in 
>> relocation in code section.
>>   error: symbol '__main_argc_argv' unsupported subtraction expression used 
>> in relocation in code section.
>>   fatal error: error in backend: function sections must contain one function 
>> each
>>
>> It seems like enabling this sanitizer perhaps uses features we don't yet 
>> support?  I will keep investigating but perhaps we can find a way to revert 
>> he effect on the wasm backend for now?
>
> wasm seems to use `-fsanitize=undefined`, which includes 
> `-fsanitize=function`.
> wasm doesn't allow data words before the function entry, so we need to 
> unsupport `-fsanitize=function` for wasm...

That makes sense to me. The wasm specification (and therefore the wasm 
runtimes) already enforce signature checking for indirect function calls so 
there should be no need for this sanitizer there anyway.Do you want to make 
that change or should I?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148573

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

[PATCH] D151130: [NFC][CLANG] Fix static code analyzer concerns with dereference null return value

[Soumi Manna via Phabricator via cfe-commits]

Manna added a comment.

Thank you @erichkeane for reviews!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151130

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

[PATCH] D151088: [flang][hlfir] Separate -emit-fir and -emit-hlfir for flang-new

[Slava Zakharin via Phabricator via cfe-commits]

vzakhari accepted this revision.
vzakhari added a comment.

Thank you for the follow-up!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151088

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

[PATCH] D151137: ]NFC][Clang] Fix Coverity bug with dereference null return value in clang::​CodeGen::​CodeGenFunction::​EmitOMPArraySectionExpr()

[Soumi Manna via Phabricator via cfe-commits]

Manna created this revision.
Manna added a reviewer: erichkeane.
Herald added a project: All.
Manna requested review of this revision.

Reported by Coverity:

Inside  "CGExpr.cpp" file, in 
clang::​CodeGen::​CodeGenFunction::​EmitOMPArraySectionExpr(clang::​OMPArraySectionExpr
 const *, bool): Return value of function which returns null is dereferenced 
without checking.

} else {
//returned_null: getAsConstantArrayType returns nullptr (checked 83 out 
of 95 times).
// var_assigned: Assigning: CAT = nullptr return value from 
getAsConstantArrayType.
  auto *CAT = C.getAsConstantArrayType(ArrayTy);
//identity_transfer: Member function call CAT->getSize() returns an 
offset off CAT (this). 

 // Dereference null return value (NULL_RETURNS)
 //dereference: Dereferencing a pointer that might be nullptr 
CAT->getSize() when calling APInt. 
 ConstLength = CAT->getSize();
}

This patch adds an assert to resolve the bug.


https://reviews.llvm.org/D151137

Files:
  clang/lib/CodeGen/CGExpr.cpp


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4102,6 +4102,7 @@
 }
   } else {
 auto *CAT = C.getAsConstantArrayType(ArrayTy);
+assert(CAT && "unexpected type for array initializer");
 ConstLength = CAT->getSize();
   }
   if (Length) {


Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -4102,6 +4102,7 @@
 }
   } else {
 auto *CAT = C.getAsConstantArrayType(ArrayTy);
+assert(CAT && "unexpected type for array initializer");
 ConstLength = CAT->getSize();
   }
   if (Length) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151034: [clang] Add test for CWG1397

[Vlad Serebrennikov via Phabricator via cfe-commits]

Endill added a comment.

Thank you for the review!




Comment at: clang/test/CXX/drs/dr13xx.cpp:488
+#if __cplusplus == 201103L
+  // expected-error@#dr1397-struct-A {{default member initializer for 'p' 
needed within definition of enclosing class 'A' outside of member functions}}
+  // expected-note@#dr1397-void-p {{in evaluation of exception specification 
for 'dr1397::A::A' needed here}}

shafik wrote:
> These diagnostic are pretty bad, maybe provide a comment above explaining 
> them a little better or maybe better refactor the diagnostic?
Totally agree about diagnostic not being accessible. I filed [[ 
https://github.com/compiler-explorer/infra/blob/fa7c4cf145898d989cb3a33e49f2a1223107eeac/bin/yaml/cpp.yaml#L648
 | a bug]] for that, but not sure how to refactor it.

I don't think it's a good place to explain complicated diagnostic messages, 
though. If someone is interested enough, intent could be seen on this review 
and in CWG1397 itself. We better just fix the diagnostic, and expected message 
together with it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151034

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

[PATCH] D146368: [clang-tidy] Add readability-reference-to-constructed-temporary check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146368

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

[PATCH] D148216: Add support for annotations in UpdateTestChecks (NFC)

[Nikita Popov via Phabricator via cfe-commits]

nikic added inline comments.



Comment at: 
clang/test/utils/update_cc_test_checks/Inputs/annotations.c.expected:12
+// CHECK-NEXT:[[TMP1:%.*]] = load i32, ptr [[X]], align 4
+// CHECK-NEXT:ret i32 [[TMP1]]
+//

hnrklssn wrote:
> hnrklssn wrote:
> > nikic wrote:
> > > hnrklssn wrote:
> > > > delcypher wrote:
> > > > > @hnrklssn I just noticed we don't have a `CHECK` for what `META2` 
> > > > > actually refers to. Should we?
> > > > > 
> > > > > Not something that has to be fixed in this patch, more just an 
> > > > > observation.
> > > > Indeed this is true for metadata in general, presumably because the RHS 
> > > > often refer to things like other metadata identifiers. In the case of 
> > > > annotations they seem to always refer to simple strings however, so it 
> > > > would be feasible to do a straight match without having to do recursive 
> > > > matching or complex regexes to determine which part of the metadata to 
> > > > match against.
> > > > 
> > > > In many cases with metadata attached to IR nodes, multiple nodes refer 
> > > > to the same metadata node, so at least you verify that they still are 
> > > > consistent. But I agree that verifying the content would be a great 
> > > > future addition.
> > > You need to pass `--check-globals` to check the actual metadata.
> > When I add that to this test case it adds
> > 
> > ```
> > //.
> > // CHECK: attributes #0 = { noinline nounwind optnone 
> > "min-legal-vector-width"="0" "no-trapping-math"="true" 
> > "stack-protector-buffer-size"="8" 
> > "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
> > //.
> > // CHECK: !0 = !{i32 1, !"wchar_size", i32 4}
> > // CHECK: !1 = !{!"clang version 17.0.0 
> > (g...@github.com:llvm/llvm-project.git 
> > 684914f47cf59e9ab6d8b0f73c58ca6272ea28d4)"}
> > // CHECK: !2 = !{!"auto-init"}
> > //.
> > ```
> > 
> > So it seems to just be doing a simple literal matching on all metadata, 
> > regardless of whether we captured that metadata in any filecheck variable. 
> > And it still doesn't use the META2 variable to match the definition. Am I 
> > missing something? If we use the literal metadata names instead of variable 
> > matching for the definitions, there isn't much point in doing variable 
> > matching for the metadata uses either, since the test still very much 
> > relies on the metadata numbering being stable.
> @nikic Do you have more information to add about how metadata definition 
> matchers can be generated without hardcoding everything (which is kind of the 
> opposite of what this patch is trying to do), or in general if you're happy 
> with the state of the PR?
This works fine with update_test_checks, so it must be some bug in 
update_cc_test_checks in particular. From a quick look, I suspect it's because 
https://github.com/llvm/llvm-project/blob/3d05ab6d3e24e76ff53b8d7d623c436b4be5b809/llvm/utils/update_cc_test_checks.py#L447
 hardcodes a True value, while update_test_checks makes this dependent on 
`--preserve-names`, which is disabled by default there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148216

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

[PATCH] D147357: [clang-tidy] Add bugprone-optional-value-conversion check

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147357

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

[PATCH] D148461: [clang-tidy] Support C++17/20 in bugprone-exception-escape

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148461

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

[PATCH] D147889: [clang-tidy] Improve bugprone-branch-clone with support for fallthrough attribute

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL updated this revision to Diff 524419.
PiotrZSL added a comment.

Rebase, Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147889

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-fallthrough.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-fallthrough.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone-fallthrough.cpp
@@ -0,0 +1,65 @@
+// RUN: %check_clang_tidy %s bugprone-branch-clone %t -- -- -std=c++17
+
+void handle(int);
+
+void testSwitchFallthroughAttribute(int value) {
+  switch(value) {
+case 1: [[fallthrough]];
+case 2: [[fallthrough]];
+case 3:
+  handle(value);
+  break;
+default:
+  break;
+  }
+}
+
+void testSwitchFallthroughAttributeAndBraces(int value) {
+  switch(value) {
+case 1: { [[fallthrough]]; }
+case 2: { [[fallthrough]]; }
+case 3: {
+  handle(value);
+  break;
+}
+default: {
+  break;
+}
+  }
+}
+
+void testSwitchWithFallthroughAttributeAndCode(int value) {
+  switch(value) {
+case 1: value += 1; [[fallthrough]];
+case 2: value += 1; [[fallthrough]];
+case 3:
+  handle(value);
+  break;
+default:
+  break;
+  }
+}
+
+void testSwitchWithFallthroughAndCode(int value) {
+  switch(value) {
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: switch has 2 consecutive identical branches [bugprone-branch-clone]
+case 1: value += 1;
+case 2: value += 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:23: note: last of these clones ends here
+case 3:
+  handle(value);
+  break;
+default:
+  break;
+  }
+}
+
+void testSwitchFallthroughAttributeIntoDefault(int value) {
+  switch(value) {
+case 1: [[fallthrough]];
+case 2: [[fallthrough]];
+default:
+  handle(value);
+  break;
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/branch-clone.rst
@@ -77,6 +77,8 @@
 we want to preserve that ``'a'`` is before ``'b'`` and ``default:`` is the last
 branch.
 
+Switch cases marked with the ``[[fallthrough]]`` attribute are ignored.
+
 Finally, the check also examines conditional operators and reports code like:
 
 .. code-block:: c++
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -201,6 +201,11 @@
 
 Changes in existing checks
 ^^
+
+- Fixed false-positive in :doc:`bugprone-branch-clone
+  ` check by ignoring duplicated
+  switch cases marked with the ``[[fallthrough]]`` attribute.
+
 - Improved :doc:`readability-redundant-string-cstr
   ` check to recognise
   unnecessary ``std::string::c_str()`` and ``std::string::data()`` calls in
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.cpp
@@ -9,6 +9,7 @@
 #include "BranchCloneCheck.h"
 #include "../utils/ASTUtils.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Analysis/CloneDetection.h"
 #include "clang/Lex/Lexer.h"
@@ -26,8 +27,8 @@
 /// Determines if the bodies of two branches in a switch statements are Type I
 /// clones of each other. This function only examines the body of the branch
 /// and ignores the `case X:` or `default:` at the start of the branch.
-static bool areSwitchBranchesIdentical(const SwitchBranch LHS,
-   const SwitchBranch RHS,
+static bool areSwitchBranchesIdentical(const SwitchBranch ,
+   const SwitchBranch ,
const ASTContext ) {
   if (LHS.size() != RHS.size())
 return false;
@@ -46,6 +47,55 @@
   return true;
 }
 
+static bool isFallthroughSwitchBranch(const SwitchBranch ) {
+  struct SwitchCaseVisitor : RecursiveASTVisitor {
+using RecursiveASTVisitor::DataRecursionQueue;
+
+bool TraverseLambdaExpr(LambdaExpr *, DataRecursionQueue * = nullptr) {
+  // Ignore lambdas
+  return true;
+}
+
+bool TraverseDecl(Decl *) {
+  // No need to check declarations
+  return true;
+}
+
+bool TraverseSwitchStmt(SwitchStmt *, DataRecursionQueue * = 

[PATCH] D151133: [clang-tidy] Ignore implicit code in bugprone-branch-clone

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Implicit code like, template instances, compiler generated
code are not excluded in this check by using
TK_IgnoreUnlessSpelledInSource.

Fixes #62693


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151133

Files:
  clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
@@ -1024,3 +1024,37 @@
   SEMICOLON_CASE_COLON(3);
   }
 }
+
+namespace PR62693 {
+  class Object {
+public:
+  template 
+bool ConvertableTo() const;
+
+  template 
+void Handle();
+  };
+
+  template 
+  void update(Object ) {
+if (a.ConvertableTo()) {
+  a.Handle();
+} else {
+  a.Handle();
+}
+  }
+
+  template 
+  void update2(Object ) {
+if (a.ConvertableTo()) {
+  a.Handle();
+} else {
+  a.Handle();
+}
+  }
+
+  void foo(Object ) {
+update(a);
+update2(a);
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -201,6 +201,11 @@
 
 Changes in existing checks
 ^^
+
+- Fixed false-positive in :doc:`bugprone-branch-clone
+  ` check by ignoring auto-generated
+  code, template instances, and implicit code patterns.
+
 - Improved :doc:`readability-redundant-string-cstr
   ` check to recognise
   unnecessary ``std::string::c_str()`` and ``std::string::data()`` calls in
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.h
@@ -27,6 +27,9 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  std::optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace clang::tidy::bugprone


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/branch-clone.cpp
@@ -1024,3 +1024,37 @@
   SEMICOLON_CASE_COLON(3);
   }
 }
+
+namespace PR62693 {
+  class Object {
+public:
+  template 
+bool ConvertableTo() const;
+
+  template 
+void Handle();
+  };
+
+  template 
+  void update(Object ) {
+if (a.ConvertableTo()) {
+  a.Handle();
+} else {
+  a.Handle();
+}
+  }
+
+  template 
+  void update2(Object ) {
+if (a.ConvertableTo()) {
+  a.Handle();
+} else {
+  a.Handle();
+}
+  }
+
+  void foo(Object ) {
+update(a);
+update2(a);
+  }
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -201,6 +201,11 @@
 
 Changes in existing checks
 ^^
+
+- Fixed false-positive in :doc:`bugprone-branch-clone
+  ` check by ignoring auto-generated
+  code, template instances, and implicit code patterns.
+
 - Improved :doc:`readability-redundant-string-cstr
   ` check to recognise
   unnecessary ``std::string::c_str()`` and ``std::string::data()`` calls in
Index: clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.h
===
--- clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.h
+++ clang-tools-extra/clang-tidy/bugprone/BranchCloneCheck.h
@@ -27,6 +27,9 @@
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  std::optional getCheckTraversalKind() const override {
+return TK_IgnoreUnlessSpelledInSource;
+  }
 };
 
 } // namespace clang::tidy::bugprone
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D150892: [clang][ExprConstant] fix __builtin_object_size for flexible array members

[Nick Desaulniers via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG57c5c1ab2a18: [clang][ExprConstant] fix 
__builtin_object_size for flexible array members (authored by nickdesaulniers).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150892

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/ExprConstant.cpp
  clang/test/CodeGen/object-size.c


Index: clang/test/CodeGen/object-size.c
===
--- clang/test/CodeGen/object-size.c
+++ clang/test/CodeGen/object-size.c
@@ -525,6 +525,33 @@
   gi = OBJECT_SIZE_BUILTIN([9].snd[0], 1);
 }
 
+// CHECK-LABEL: @test32
+static struct DynStructVar D32 = {
+  .fst = {},
+  .snd = { 0, 1, 2, },
+};
+unsigned long test32(void) {
+  // CHECK: ret i64 19
+  return OBJECT_SIZE_BUILTIN(, 1);
+}
+// CHECK-LABEL: @test33
+static struct DynStructVar D33 = {
+  .fst = {},
+  .snd = {},
+};
+unsigned long test33(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(, 1);
+}
+// CHECK-LABEL: @test34
+static struct DynStructVar D34 = {
+  .fst = {},
+};
+unsigned long test34(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(, 1);
+}
+
 // CHECK-LABEL: @PR30346
 void PR30346(void) {
   struct sa_family_t {};
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11733,7 +11733,14 @@
   auto CheckedHandleSizeof = [&](QualType Ty, CharUnits ) {
 if (Ty.isNull() || Ty->isIncompleteType() || Ty->isFunctionType())
   return false;
-return HandleSizeof(Info, ExprLoc, Ty, Result);
+bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
+if (Ty->isStructureType() &&
+Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {
+  const auto *VD =
+  cast(LVal.getLValueBase().get());
+  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+}
+return Ret;
   };
 
   // We want to evaluate the size of the entire object. This is a valid 
fallback
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -51,6 +51,11 @@
 
 foo: asm goto ("# %0 %1"::"i"(&)::foo);
 
+- ``__builtin_object_size`` and ``__builtin_dynamic_object_size`` now add the
+  ``sizeof`` the elements specified in designated initializers of flexible
+  array members for structs that contain them. This change is more consistent
+  with the behavior of GCC.
+
 C++ Specific Potentially Breaking Changes
 -
 - Clang won't search for coroutine_traits in std::experimental namespace any 
more.
@@ -422,6 +427,10 @@
   (`#61746 `_).
 - Clang `TextNodeDumper` enabled through `-ast-dump` flag no longer evaluates 
the
   initializer of constexpr `VarDecl` if the declaration has a dependent type.
+- Match GCC's behavior for ``__builtin_object_size`` and
+  ``__builtin_dynamic_object_size`` on structs containing flexible array
+  members.
+  (`#62789 `_).
 
 Bug Fixes to Compiler Builtins
 ^^


Index: clang/test/CodeGen/object-size.c
===
--- clang/test/CodeGen/object-size.c
+++ clang/test/CodeGen/object-size.c
@@ -525,6 +525,33 @@
   gi = OBJECT_SIZE_BUILTIN([9].snd[0], 1);
 }
 
+// CHECK-LABEL: @test32
+static struct DynStructVar D32 = {
+  .fst = {},
+  .snd = { 0, 1, 2, },
+};
+unsigned long test32(void) {
+  // CHECK: ret i64 19
+  return OBJECT_SIZE_BUILTIN(, 1);
+}
+// CHECK-LABEL: @test33
+static struct DynStructVar D33 = {
+  .fst = {},
+  .snd = {},
+};
+unsigned long test33(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(, 1);
+}
+// CHECK-LABEL: @test34
+static struct DynStructVar D34 = {
+  .fst = {},
+};
+unsigned long test34(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(, 1);
+}
+
 // CHECK-LABEL: @PR30346
 void PR30346(void) {
   struct sa_family_t {};
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -11733,7 +11733,14 @@
   auto CheckedHandleSizeof = [&](QualType Ty, CharUnits ) {
 if (Ty.isNull() || Ty->isIncompleteType() || Ty->isFunctionType())
   return false;
-return HandleSizeof(Info, ExprLoc, Ty, Result);
+bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
+if (Ty->isStructureType() &&
+Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {
+  const auto *VD =
+  cast(LVal.getLValueBase().get());
+  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+}
+return Ret;
   };
 
   // We want to 

[clang] 57c5c1a - [clang][ExprConstant] fix __builtin_object_size for flexible array members

[Nick Desaulniers via cfe-commits]

Author: Nick Desaulniers
Date: 2023-05-22T11:38:38-07:00
New Revision: 57c5c1ab2a188b7962c9de5ac0f95e3c7441940a

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

LOG: [clang][ExprConstant] fix __builtin_object_size for flexible array members

As reported by @kees, GCC treats __builtin_object_size of structures
containing flexible array members (aka arrays with incomplete type) not
just as the sizeof the underlying type, but additionally the size of the
members in a designated initializer list.

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

Reviewed By: erichkeane

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/AST/ExprConstant.cpp
clang/test/CodeGen/object-size.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a55e969fa21c..483b2d0110b7 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -51,6 +51,11 @@ C/C++ Language Potentially Breaking Changes
 
 foo: asm goto ("# %0 %1"::"i"(&)::foo);
 
+- ``__builtin_object_size`` and ``__builtin_dynamic_object_size`` now add the
+  ``sizeof`` the elements specified in designated initializers of flexible
+  array members for structs that contain them. This change is more consistent
+  with the behavior of GCC.
+
 C++ Specific Potentially Breaking Changes
 -
 - Clang won't search for coroutine_traits in std::experimental namespace any 
more.
@@ -422,6 +427,10 @@ Bug Fixes in This Version
   (`#61746 `_).
 - Clang `TextNodeDumper` enabled through `-ast-dump` flag no longer evaluates 
the
   initializer of constexpr `VarDecl` if the declaration has a dependent type.
+- Match GCC's behavior for ``__builtin_object_size`` and
+  ``__builtin_dynamic_object_size`` on structs containing flexible array
+  members.
+  (`#62789 `_).
 
 Bug Fixes to Compiler Builtins
 ^^

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index ce3c5257e711..f1a30da1ff77 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -11733,7 +11733,14 @@ static bool determineEndOffset(EvalInfo , 
SourceLocation ExprLoc,
   auto CheckedHandleSizeof = [&](QualType Ty, CharUnits ) {
 if (Ty.isNull() || Ty->isIncompleteType() || Ty->isFunctionType())
   return false;
-return HandleSizeof(Info, ExprLoc, Ty, Result);
+bool Ret = HandleSizeof(Info, ExprLoc, Ty, Result);
+if (Ty->isStructureType() &&
+Ty->getAsStructureType()->getDecl()->hasFlexibleArrayMember()) {
+  const auto *VD =
+  cast(LVal.getLValueBase().get());
+  Result += VD->getFlexibleArrayInitChars(Info.Ctx);
+}
+return Ret;
   };
 
   // We want to evaluate the size of the entire object. This is a valid 
fallback

diff  --git a/clang/test/CodeGen/object-size.c 
b/clang/test/CodeGen/object-size.c
index 157926dd719d..ac1551343d6f 100644
--- a/clang/test/CodeGen/object-size.c
+++ b/clang/test/CodeGen/object-size.c
@@ -525,6 +525,33 @@ void test31(void) {
   gi = OBJECT_SIZE_BUILTIN([9].snd[0], 1);
 }
 
+// CHECK-LABEL: @test32
+static struct DynStructVar D32 = {
+  .fst = {},
+  .snd = { 0, 1, 2, },
+};
+unsigned long test32(void) {
+  // CHECK: ret i64 19
+  return OBJECT_SIZE_BUILTIN(, 1);
+}
+// CHECK-LABEL: @test33
+static struct DynStructVar D33 = {
+  .fst = {},
+  .snd = {},
+};
+unsigned long test33(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(, 1);
+}
+// CHECK-LABEL: @test34
+static struct DynStructVar D34 = {
+  .fst = {},
+};
+unsigned long test34(void) {
+  // CHECK: ret i64 16
+  return OBJECT_SIZE_BUILTIN(, 1);
+}
+
 // CHECK-LABEL: @PR30346
 void PR30346(void) {
   struct sa_family_t {};



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

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

It's weird to have C-style casts that can't be done with any combination of 
named casts, so I think this makes some sense.  I do think it should be limited 
to numbered address spaces of the same size, though, rather than basing it on 
whether we're in OpenCL mode.  Target address spaces should not generally be 
forced into this rule.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151087

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

[PATCH] D151094: [clang][wip] Implement P2564 "consteval must propagate up"

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 524408.
cor3ntin marked 9 inline comments as done.
cor3ntin added a comment.
Herald added a subscriber: martong.
Herald added a reviewer: shafik.

- Address Aaron's comments
- Track which expressions are immediately escalating to offer better diagnostics


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151094

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/Decl.cpp
  clang/lib/AST/DeclPrinter.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/VTableBuilder.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/ScopeInfo.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/CodeGenCXX/cxx20-consteval-crash.cpp
  clang/test/SemaCXX/cxx2a-consteval-default-params.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp
  clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -363,7 +363,7 @@
 
   consteval needs to propagate up
   https://wg21.link/P2564R3;>P2564R3 (DR)
-  No
+  Clang 17
 
 
   Lifetime extension in range-based for loops
Index: clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -0,0 +1,131 @@
+
+// RUN: %clang_cc1 -std=c++2a -emit-llvm-only -Wno-unused-value %s -verify
+// RUN: %clang_cc1 -std=c++2b -emit-llvm-only -Wno-unused-value %s -verify
+
+consteval int id(int i) { return i; }
+constexpr char id(char c) { return c; }
+
+namespace examples {
+
+template 
+constexpr int f(T t) { // expected-note {{declared here}}
+return t + id(t);  // expected-note {{'f' is an immediate function because its body contains a call to a consteval function 'id' and that call is not a constant expression}}
+}
+auto a = ; // ok, f is not an immediate function
+auto b = ;  // expected-error {{cannot take address of immediate function 'f' outside of an immediate invocation}}
+
+static_assert(f(3) == 6); // ok
+
+template 
+constexpr int g(T t) {// g is not an immediate function
+return t + id(42);// because id(42) is already a constant
+}
+
+template 
+constexpr bool is_not(T t, F f) {
+return not f(t);
+}
+
+consteval bool is_even(int i) { return i % 2 == 0; }
+
+static_assert(is_not(5, is_even));
+
+int x = 0; // expected-note {{declared here}}
+
+template 
+constexpr T h(T t = id(x)) { // expected-note {{read of non-const variable 'x' is not allowed in a constant expression}} \
+ // expected-note {{'hh' is an immediate function because its body contains a call to a consteval function 'id' and that call is not a constant expression}}
+return t;
+}
+
+template 
+constexpr T hh() {   // hh is an immediate function
+return h();
+}
+
+int i = hh(); // expected-error {{call to consteval function 'examples::hh' is not a constant expression}} \
+   // expected-note {{in call to 'hh()'}}
+
+struct A {
+  int x;
+  int y = id(x);
+};
+
+template 
+constexpr int k(int) {
+  return A(42).y;
+}
+
+}
+
+namespace e2{
+template 
+constexpr int f(T t);
+auto a = ;
+auto b = ;
+}
+
+namespace forward_declare_constexpr{
+template 
+constexpr int f(T t);
+
+auto a = ;
+auto b = ;
+
+template 
+constexpr int f(T t) {
+return id(0);
+}
+}
+
+namespace forward_declare_consteval{
+template 
+constexpr int f(T t);  // expected-note {{'f' defined here}}
+
+auto a = ;
+auto b = ; // expected-error {{immediate function 'f' used before it is defined}} \
+  // expected-note {{in instantiation of function template specialization}}
+
+template 
+constexpr int f(T t) {
+return id(t); // expected-note {{'f' is an immediate function because its body contains a call to a consteval function 'id' and that call is not a constant expression}}
+}
+}
+
+namespace constructors {
+consteval int f(int) {
+  return 0;
+}
+struct S {
+  constexpr S(auto i) {
+f(i);
+  }
+};
+constexpr void g(auto i) {
+  [[maybe_unused]] S s{i};
+}
+void 

[PATCH] D151061: [test] Add ext_vector_type tests for C

[Florian Hahn via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151061

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

[PATCH] D151130: [NFC][CLANG] Fix static code analyzer concerns with dereference null return value

[Soumi Manna via Phabricator via cfe-commits]

Manna created this revision.
Herald added subscribers: manas, ASDenysPetrov, dkrupp, donat.nagy, Szelethus, 
a.sidorin, baloghadamsoftware.
Herald added a project: All.
Manna requested review of this revision.
Herald added a project: clang.

Reported by Static Analyzer Tool:

Inside "SemaExprMember.cpp" file, in 
clang::​Sema::​BuildMemberReferenceExpr(clang::​Expr *, clang::​QualType, 
clang::​SourceLocation, bool, clang::​CXXScopeSpec &, clang::​SourceLocation, 
clang::​NamedDecl *, clang::​DeclarationNameInfo const &, 
clang::​TemplateArgumentListInfo const *, clang::​Scope const *, 
clang::​Sema::​ActOnMemberAccessExtraArgs *): Return value of function which 
returns null is dereferenced without checking

  //Condition !Base, taking true branch.
  if (!Base) {
TypoExpr *TE = nullptr;
QualType RecordTy = BaseType;
  
//Condition IsArrow, taking true branch.
 if (IsArrow) RecordTy = RecordTy->castAs()->getPointeeType();
//returned_null: getAs returns nullptr (checked 279 out of 294 times). 
[show details]
//Condition TemplateArgs != NULL, taking true branch.

 //Dereference null return value (NULL_RETURNS)
 //dereference: Dereferencing a pointer that might be nullptr 
RecordTy->getAs() when calling LookupMemberExprInRecord. [show details]
 if (LookupMemberExprInRecord(
   *this, R, nullptr, RecordTy->getAs(), OpLoc, IsArrow,
   SS, TemplateArgs != nullptr, TemplateKWLoc, TE))
return ExprError();
 if (TE)
   return TE;

This patch uses castAs instead of getAs which will assert if the type doesn't 
match.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151130

Files:
  clang/lib/Sema/SemaExprMember.cpp


Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -767,7 +767,7 @@
 QualType RecordTy = BaseType;
 if (IsArrow) RecordTy = RecordTy->castAs()->getPointeeType();
 if (LookupMemberExprInRecord(
-*this, R, nullptr, RecordTy->getAs(), OpLoc, IsArrow,
+*this, R, nullptr, RecordTy->castAs(), OpLoc, IsArrow,
 SS, TemplateArgs != nullptr, TemplateKWLoc, TE))
   return ExprError();
 if (TE)


Index: clang/lib/Sema/SemaExprMember.cpp
===
--- clang/lib/Sema/SemaExprMember.cpp
+++ clang/lib/Sema/SemaExprMember.cpp
@@ -767,7 +767,7 @@
 QualType RecordTy = BaseType;
 if (IsArrow) RecordTy = RecordTy->castAs()->getPointeeType();
 if (LookupMemberExprInRecord(
-*this, R, nullptr, RecordTy->getAs(), OpLoc, IsArrow,
+*this, R, nullptr, RecordTy->castAs(), OpLoc, IsArrow,
 SS, TemplateArgs != nullptr, TemplateKWLoc, TE))
   return ExprError();
 if (TE)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144911: adding bf16 support to NVPTX

[Artem Belevich via Phabricator via cfe-commits]

tra added inline comments.



Comment at: llvm/lib/Target/NVPTX/NVPTXAsmPrinter.cpp:315-318
-} else if (RC == ::BFloat16RegsRegClass) {
-  Ret = (9 << 28);
-} else if (RC == ::BFloat16x2RegsRegClass) {
-  Ret = (10 << 28);

There's still something odd with the patch. It appears that it's a diff vs a 
previous set of the changes which did introduce BFloat16RegsRegClass.

Can you please update the diff to be vs the recent LLVM tree HEAD?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144911

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

[PATCH] D145403: [Pipeline] Don't run EarlyFPM in LTO post link

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 524406.
aeubanks added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145403

Files:
  llvm/lib/Passes/PassBuilderPipelines.cpp
  llvm/test/Other/new-pm-pgo.ll
  llvm/test/Other/new-pm-thinlto-postlink-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
  llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll

Index: llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-samplepgo-defaults.ll
@@ -29,30 +29,23 @@
 ; CHECK-NOEXT: {{^}}
 
 ; CHECK-EP-PIPELINE-START: Running pass: NoOpModulePass
-; CHECK-O: Running pass: InferFunctionAttrsPass
+; CHECK-O: Running pass: SampleProfileLoaderPass
 ; CHECK-O-NEXT: Running analysis: InnerAnalysisManagerProxy
-; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
-; CHECK-O-NEXT: Running pass: CoroEarlyPass
-; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass
-; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O-NEXT: Running analysis: TargetIRAnalysis
-; CHECK-O-NEXT: Running analysis: AssumptionAnalysis
-; CHECK-O-NEXT: Running pass: SROAPass
-; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis
-; CHECK-O-NEXT: Running pass: EarlyCSEPass
-; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
-; CHECK-O3-NEXT: Running pass: CallSiteSplittingPass
-; CHECK-O-NEXT: Running pass: SampleProfileLoaderPass
 ; CHECK-O-NEXT: Running analysis: ProfileSummaryAnalysis
 ; CHECK-O-NEXT: Running analysis: LazyCallGraphAnalysis
+; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
 ; CHECK-O-NEXT: Running pass: RequireAnalysisPass<{{.*}}ProfileSummaryAnalysis
 ; CHECK-O-NEXT: Running pass: PGOIndirectCallPromotion
 ; CHECK-O-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis
 ; CHECK-O-NEXT: Running pass: OpenMPOptPass
 ; CHECK-O-NEXT: Running pass: LowerTypeTestsPass
 ; CHECK-O-NEXT: Running pass: IPSCCPPass
+; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis
+; CHECK-O-NEXT: Running analysis: AssumptionAnalysis
+; CHECK-O-NEXT: Running analysis: TargetIRAnalysis
 ; CHECK-O-NEXT: Running pass: CalledValuePropagationPass
 ; CHECK-O-NEXT: Running pass: GlobalOptPass
+; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
 ; CHECK-O-NEXT: Running pass: PromotePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
 ; CHECK-O-NEXT: Running analysis: AAManager on foo
Index: llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-pgo-defaults.ll
@@ -28,25 +28,18 @@
 ; CHECK-O-NEXT: Running analysis: ProfileSummaryAnalysis
 ; CHECK-O-NEXT: Running analysis: InnerAnalysisManagerProxy
 ; CHECK-O-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis
-; CHECK-O-NEXT: Running pass: InferFunctionAttrsPass
-; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
-; CHECK-O-NEXT: Running pass: CoroEarlyPass
-; CHECK-O-NEXT: Running pass: LowerExpectIntrinsicPass
-; CHECK-O-NEXT: Running pass: SimplifyCFGPass
-; CHECK-O-NEXT: Running analysis: TargetIRAnalysis
-; CHECK-O-NEXT: Running analysis: AssumptionAnalysis
-; CHECK-O-NEXT: Running pass: SROAPass
-; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis
-; CHECK-O-NEXT: Running pass: EarlyCSEPass
-; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
-; CHECK-O3-NEXT: Running pass: CallSiteSplittingPass
 ; CHECK-O-NEXT: Running pass: OpenMPOptPass
 ; CHECK-O-NEXT: Running pass: LowerTypeTestsPass
 ; CHECK-O-NEXT: Running pass: IPSCCPPass
+; CHECK-O-NEXT: Running analysis: DominatorTreeAnalysis
+; CHECK-O-NEXT: Running analysis: AssumptionAnalysis
+; CHECK-O-NEXT: Running analysis: TargetIRAnalysis
 ; CHECK-O-NEXT: Running pass: CalledValuePropagationPass
 ; CHECK-O-NEXT: Running pass: GlobalOptPass
+; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
 ; CHECK-O-NEXT: Running pass: PromotePass
 ; CHECK-O-NEXT: Running pass: InstCombinePass
+; CHECK-O-NEXT: Running analysis: TargetLibraryAnalysis
 ; CHECK-O-NEXT: Running analysis: AAManager
 ; CHECK-O-NEXT: Running analysis: BasicAA
 ; CHECK-O-NEXT: Running analysis: ScopedNoAliasAA
Index: llvm/test/Other/new-pm-thinlto-postlink-defaults.ll
===
--- llvm/test/Other/new-pm-thinlto-postlink-defaults.ll
+++ llvm/test/Other/new-pm-thinlto-postlink-defaults.ll
@@ -43,25 +43,18 @@
 ; CHECK-POSTLINK-O-NEXT: Running analysis: ProfileSummaryAnalysis
 ; CHECK-POSTLINK-O-NEXT: Running analysis: InnerAnalysisManagerProxy
 ; CHECK-POSTLINK-O-NEXT: Running analysis: OptimizationRemarkEmitterAnalysis
-; CHECK-O-NEXT: Running pass: InferFunctionAttrsPass
-; CHECK-O-NEXT: Running analysis: 

[PATCH] D151061: [test] Add ext_vector_type tests for C

[Cassie Jones via Phabricator via cfe-commits]

porglezomp updated this revision to Diff 524405.
porglezomp added a comment.

Address the comment suggestions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151061

Files:
  clang/test/Sema/ext_vector_ops.c
  clang/test/Sema/vecshift.c

Index: clang/test/Sema/vecshift.c
===
--- clang/test/Sema/vecshift.c
+++ clang/test/Sema/vecshift.c
@@ -65,6 +65,20 @@
   vs8 = ui << vs8;
   vus8 = 1 << vus8;
 
+  vc8 = vc8 << c;
+  vuc8 = vuc8 << uc;
+  vs8 = vs8 << s;
+  vus8 = vus8 << us;
+  vi8 = vi8 << i;
+  vui8 = vui8 << ui;
+
+  vc8 = vc8 << i;
+  vuc8 = vuc8 << i;
+  vs8 = vs8 << i;
+  vus8 = vus8 << i;
+  vi8 = vi8 << i;
+  vui8 = vui8 << i;
+
   vc8 = vc8 << vc8;
 #ifdef ERR
   vi8 = vi8 << vuc8; // expected-error {{vector operands do not have the same elements sizes}}
Index: clang/test/Sema/ext_vector_ops.c
===
--- clang/test/Sema/ext_vector_ops.c
+++ clang/test/Sema/ext_vector_ops.c
@@ -25,3 +25,211 @@
   v2u *v2u_ptr = 0;
   v2s *v2s_ptr;
 }
+
+void test_int_vector_scalar(unsigned int ua, v2u v2ua) {
+  // Operations with one integer vector and one scalar. These splat the scalar.
+  (void)(v2ua + ua);
+  (void)(ua + v2ua);
+  (void)(v2ua - ua);
+  (void)(ua - v2ua);
+  (void)(v2ua * ua);
+  (void)(ua * v2ua);
+  (void)(v2ua / ua);
+  (void)(ua / v2ua);
+  (void)(v2ua % ua);
+  (void)(ua % v2ua);
+
+  (void)(v2ua == ua);
+  (void)(ua == v2ua);
+  (void)(v2ua != ua);
+  (void)(ua != v2ua);
+  (void)(v2ua <= ua);
+  (void)(ua <= v2ua);
+  (void)(v2ua >= ua);
+  (void)(ua >= v2ua);
+  (void)(v2ua < ua);
+  (void)(ua < v2ua);
+  (void)(v2ua > ua);
+  (void)(ua > v2ua);
+  (void)(v2ua && ua);
+  (void)(ua && v2ua);
+  (void)(v2ua || ua);
+  (void)(ua || v2ua);
+
+  (void)(v2ua & ua);
+  (void)(ua & v2ua);
+  (void)(v2ua | ua);
+  (void)(ua | v2ua);
+  (void)(v2ua ^ ua);
+  (void)(ua ^ v2ua);
+  (void)(v2ua << ua);
+  (void)(ua << v2ua);
+  (void)(v2ua >> ua);
+  (void)(ua >> v2ua);
+
+  v2ua += ua;
+  v2ua -= ua;
+  v2ua *= ua;
+  v2ua /= ua;
+  v2ua %= ua;
+  v2ua &= ua;
+  v2ua |= ua;
+  v2ua ^= ua;
+  v2ua >>= ua;
+  v2ua <<= ua;
+
+  ua += v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua -= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua *= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua /= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua %= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua &= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua |= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua ^= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua >>= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+  ua <<= v2ua; // expected-error{{assigning to 'unsigned int' from incompatible type 'v2u'}}
+}
+
+void test_float_vector_scalar(float fa, unsigned int ua, v2f v2fa) {
+  // Operations with one float vector and one scalar. These splat the scalar.
+  (void)(v2fa + fa);
+  (void)(fa + v2fa);
+  (void)(v2fa - fa);
+  (void)(fa - v2fa);
+  (void)(v2fa * fa);
+  (void)(fa * v2fa);
+  (void)(v2fa / fa);
+  (void)(fa / v2fa);
+  (void)(v2fa % fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa % v2fa); // expected-error{{invalid operands to binary expression}}
+
+  (void)(v2fa == fa);
+  (void)(fa == v2fa);
+  (void)(v2fa != fa);
+  (void)(fa != v2fa);
+  (void)(v2fa <= fa);
+  (void)(fa <= v2fa);
+  (void)(v2fa >= fa);
+  (void)(fa >= v2fa);
+  (void)(v2fa < fa);
+  (void)(fa < v2fa);
+  (void)(v2fa > fa);
+  (void)(fa > v2fa);
+  (void)(v2fa && fa);
+  (void)(fa && v2fa);
+  (void)(v2fa || fa);
+  (void)(fa || v2fa);
+
+  (void)(v2fa & fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa & v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa | fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa | v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa ^ fa); // expected-error{{invalid operands to binary expression}}
+  (void)(fa ^ v2fa); // expected-error{{invalid operands to binary expression}}
+  (void)(v2fa << fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(v2fa << ua); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  (void)(fa << v2fa); // expected-error{{used type 'float' where integer is required}}
+  (void)(ua << v2fa); // expected-error{{used type 'v2f' (vector of 2 'float' values) where integer is required}}
+  

[PATCH] D151128: [clangd] Show size, offset and padding for bit fields on hover

[SR_team via Phabricator via cfe-commits]

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

In some cases, we want to view the bytes offset of a bit field block, like in:

  struct SomeFlags{
// ~32 named bit flags, before foo
bool foo : 1;
   // ~ 31 named bit flags, after foo
  };

With this diff in hover showed offset for `foo` in bytes. This offset is 
floored (e.g. offset 9 bits floored to 8 bits and convert to bytes).
Size of bit fields is ceiled to convert in bytes (e.g. 9 bits ceiled to 16 
bits).

Logic to calculate padding not changed, so it showed for floored bits (e.g. 9 
bits floored to 8 bits and present as 1 byte)


https://reviews.llvm.org/D151128

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


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -125,6 +125,8 @@
  HI.Kind = index::SymbolKind::Field;
  HI.Definition = "int x : 1";
  HI.Type = "int";
+ HI.Offset = 0;
+ HI.Size = 1;
  HI.AccessSpecifier = "public";
}},
   // Local to class method.
@@ -1285,6 +1287,26 @@
  HI.Kind = index::SymbolKind::Field;
  HI.Definition = "m_int arr[Size]";
  HI.Type = {"m_int[Size]", "int[Size]"};
+   }},
+  {// Bitfield offset, size and padding
+   R"cpp(
+struct Foo {
+  char x;
+  char [[^y]] : 1;
+  int z;
+};
+  )cpp",
+   [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.LocalScope = "Foo::";
+ HI.Name = "y";
+ HI.Kind = index::SymbolKind::Field;
+ HI.Definition = "char y : 1";
+ HI.Type = "char";
+ HI.Offset = 1;
+ HI.Size = 1;
+ HI.Padding = 2;
+ HI.AccessSpecifier = "public";
}}};
   for (const auto  : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -1008,12 +1008,15 @@
 const auto *Record = FD->getParent();
 if (Record)
   Record = Record->getDefinition();
-if (Record && !Record->isInvalidDecl() && !Record->isDependentType() &&
-!FD->isBitField()) {
+if (Record && !Record->isInvalidDecl() && !Record->isDependentType()) {
   const ASTRecordLayout  = Ctx.getASTRecordLayout(Record);
   HI.Offset = Layout.getFieldOffset(FD->getFieldIndex()) / 8;
-  if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType())) {
+  if (FD->isBitField()) {
+const auto SizeInBits = FD->getBitWidthValue(Ctx);
+HI.Size = (SizeInBits + 7) >> 3;
+  } else if (auto Size = Ctx.getTypeSizeInCharsIfKnown(FD->getType()))
 HI.Size = FD->isZeroSize(Ctx) ? 0 : Size->getQuantity();
+  if (HI.Size) {
 unsigned EndOfField = *HI.Offset + *HI.Size;
 
 // Calculate padding following the field.


Index: clang-tools-extra/clangd/unittests/HoverTests.cpp
===
--- clang-tools-extra/clangd/unittests/HoverTests.cpp
+++ clang-tools-extra/clangd/unittests/HoverTests.cpp
@@ -125,6 +125,8 @@
  HI.Kind = index::SymbolKind::Field;
  HI.Definition = "int x : 1";
  HI.Type = "int";
+ HI.Offset = 0;
+ HI.Size = 1;
  HI.AccessSpecifier = "public";
}},
   // Local to class method.
@@ -1285,6 +1287,26 @@
  HI.Kind = index::SymbolKind::Field;
  HI.Definition = "m_int arr[Size]";
  HI.Type = {"m_int[Size]", "int[Size]"};
+   }},
+  {// Bitfield offset, size and padding
+   R"cpp(
+struct Foo {
+  char x;
+  char [[^y]] : 1;
+  int z;
+};
+  )cpp",
+   [](HoverInfo ) {
+ HI.NamespaceScope = "";
+ HI.LocalScope = "Foo::";
+ HI.Name = "y";
+ HI.Kind = index::SymbolKind::Field;
+ HI.Definition = "char y : 1";
+ HI.Type = "char";
+ HI.Offset = 1;
+ HI.Size = 1;
+ HI.Padding = 2;
+ HI.AccessSpecifier = "public";
}}};
   for (const auto  : Cases) {
 SCOPED_TRACE(Case.Code);
Index: clang-tools-extra/clangd/Hover.cpp
===
--- clang-tools-extra/clangd/Hover.cpp
+++ clang-tools-extra/clangd/Hover.cpp
@@ -1008,12 +1008,15 @@
 const auto *Record = FD->getParent();
 if (Record)
   Record = Record->getDefinition();
-if 

[PATCH] D151034: [clang] Add test for CWG1397

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/test/CXX/drs/dr13xx.cpp:488
+#if __cplusplus == 201103L
+  // expected-error@#dr1397-struct-A {{default member initializer for 'p' 
needed within definition of enclosing class 'A' outside of member functions}}
+  // expected-note@#dr1397-void-p {{in evaluation of exception specification 
for 'dr1397::A::A' needed here}}

These diagnostic are pretty bad, maybe provide a comment above explaining them 
a little better or maybe better refactor the diagnostic?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151034

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

[PATCH] D151034: [clang] Add test for CWG1397

[Shafik Yaghmour via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151034

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

[PATCH] D141907: [CMake] Ensure `CLANG_RESOURCE_DIR` is respected

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

In D141907#4355229 , @paperchalice 
wrote:

> In D141907#4355228 , @tstellar 
> wrote:
>
>> @paperchalice Do you need someone to commit this for you?
>
> Sure, I don't have the commit access.

What name / email address do you want me to use for the commit author field?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141907

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

[PATCH] D151032: [clang] Add test for CWG2213

[Shafik Yaghmour via Phabricator via cfe-commits]

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

Thank you, I think more examples are never bad, LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151032

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

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

I don't feel `reinterpret_cast` is the right fit for this as it's expected to 
do just reinterpretation but `addrspacecast` LLVM instruction which Clang 
generates might result in some more operations in particular some special 
instructions for address calculation. Would it work if you enable 
`addrspace_cast` with double underscore prefix directly in C++ or something 
like this? Otherwise C-cast would be just as good as changing reinterpret cast 
in this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151087

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

[PATCH] D145183: [libc++] Implement `stop_token`

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.
Herald added a subscriber: JDevlieghere.

Something seems wrong with this patch. It contains multiple unrelated changes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145183

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

[PATCH] D148573: Allow -fsanitize=function on all targets

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D148573#4361396 , @sbc100 wrote:

> This change seems to be causing problems on the emscripten auto-roller:  
> https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8780394114149321217/overview
>
> Failures show up in ubsan tests and look like this:
>
>   error: symbol '_Z4testi' unsupported subtraction expression used in 
> relocation in code section.
>   error: symbol '__main_argc_argv' unsupported subtraction expression used in 
> relocation in code section.
>   fatal error: error in backend: function sections must contain one function 
> each
>
> It seems like enabling this sanitizer perhaps uses features we don't yet 
> support?  I will keep investigating but perhaps we can find a way to revert 
> he effect on the wasm backend for now?

wasm seems to use `-fsanitize=undefined`, which includes `-fsanitize=function`.
wasm doesn't allow data words before the function entry, so we need to 
unsupport `-fsanitize=function` for wasm...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148573

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

[clang] 39ba913 - [Driver] -fsanitize=function: unsupport wasm after D148573

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2023-05-22T10:40:04-07:00
New Revision: 39ba913d13ab15c76cb6b5aa066fa111ddfe944b

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

LOG: [Driver] -fsanitize=function: unsupport wasm after D148573

D148573 made x86-specific -fsanitize=function available to all targets,
but wasm doesn't allow placing data words before the function label,
so unsupport wasm.

Added: 


Modified: 
clang/lib/Driver/ToolChains/WebAssembly.cpp
clang/test/Driver/wasm-toolchain.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp 
b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index a1c4cd9ef9c79..016b70b1c2ede 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -459,6 +459,9 @@ SanitizerMask WebAssembly::getSupportedSanitizers() const {
   if (getTriple().isOSEmscripten()) {
 Res |= SanitizerKind::Vptr | SanitizerKind::Leak | SanitizerKind::Address;
   }
+  // -fsanitize=function places two words before the function label, which are
+  // -unsupported.
+  Res &= ~SanitizerKind::Function;
   return Res;
 }
 

diff  --git a/clang/test/Driver/wasm-toolchain.c 
b/clang/test/Driver/wasm-toolchain.c
index f391b2802308d..bfe696d7846e7 100644
--- a/clang/test/Driver/wasm-toolchain.c
+++ b/clang/test/Driver/wasm-toolchain.c
@@ -146,6 +146,9 @@
 // CHECK-ASAN-EMSCRIPTEN: "-fsanitize=address"
 // CHECK-ASAN-EMSCRIPTEN: "-fsanitize-address-globals-dead-stripping"
 
+// RUN: %clang -### %s -fsanitize=function --target=wasm32-unknown-emscripten 
2>&1 | FileCheck --check-prefix=FUNCTION %s
+// FUNCTION: error: unsupported option '-fsanitize=function' for target 
'wasm32-unknown-emscripten'
+
 // Basic exec-model tests.
 
 // RUN: %clang -### %s --target=wasm32-unknown-unknown 
--sysroot=%s/no-sysroot-there -mexec-model=command 2>&1 \



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

[PATCH] D145183: [libc++] Implement `stop_token`

[Hui via Phabricator via cfe-commits]

huixie90 updated this revision to Diff 524387.
huixie90 added a comment.
Herald added subscribers: cfe-commits, llvm-commits, lldb-commits, Sanitizers, 
TinaAMD, hoy, bviyer, wlei, jplehr, mgehre-amd, luke, hanchung, jsetoain, 
Moerafaat, zero9178, pcwang-thead, anlunx, Enna1, bzcheeseman, mattd, 
gchakrabarti, ThomasRaoux, awarzynski, sdasgup3, wenzhicui, wrengr, snehasish, 
armkevincheng, ormris, foad, jsmolens, sjarus, eric-k256, cota, mravishankar, 
teijeong, frasercrmck, rdzhabarov, tatianashp, wenlei, mehdi_amini, okura, 
jdoerfert, msifontes, sstefan1, jurahul, kuter, Kayjukh, vkmr, grosul1, 
jvesely, Joonsoo, stephenneuendorffer, kerbowa, liufengdb, aartbik, mgester, 
arpith-jacob, csigg, nicolasvasilache, antiagainst, shauheen, rriddle, 
luismarques, apazos, sameer.abuasal, pengfei, s.egerton, Jim, asbirlea, 
kadircet, jocewei, PkmX, arphaman, the_o, brucehoult, MartinMosbeck, rogfer01, 
steven_wu, mgrang, edward-jones, zzheng, MaskRay, jrtc27, gbedwell, niosHD, 
sabuasal, simoncook, johnrusso, rbar, asb, fedor.sergeev, hiraditya, nemanjai, 
arsenm, jholewinski.
Herald added a reviewer: andreadb.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: jdoerfert.
Herald added a reviewer: sstefan1.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: herhut.
Herald added a reviewer: rriddle.
Herald added a reviewer: aartbik.
Herald added a reviewer: sscalpone.
Herald added a reviewer: ftynse.
Herald added a reviewer: aartbik.
Herald added a reviewer: clementval.
Herald added a reviewer: ThomasRaoux.
Herald added a reviewer: NoQ.
Herald added a reviewer: dcaballe.
Herald added a reviewer: ftynse.
Herald added a reviewer: dcaballe.
Herald added a reviewer: kiranchandramohan.
Herald added projects: clang, Sanitizers, LLDB, MLIR, LLVM, clang-tools-extra, 
Flang.

- [libc++] Implement `stop_token`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145183

Files:
  clang-tools-extra/clangd/refactor/Rename.cpp
  clang-tools-extra/clangd/unittests/RenameTests.cpp
  clang/docs/ControlFlowIntegrity.rst
  clang/docs/LanguageExtensions.rst
  clang/docs/ReleaseNotes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/Basic/RISCVVTypes.def
  clang/include/clang/Basic/riscv_vector.td
  clang/include/clang/Basic/riscv_vector_common.td
  clang/include/clang/Driver/Options.td
  clang/include/clang/Support/RISCVVIntrinsicUtils.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenTypes.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaInit.cpp
  clang/lib/Sema/SemaRISCVVectorLookup.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/Support/RISCVVIntrinsicUtils.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/CodeGen/RISCV/rvb-intrinsics/riscv64-zbb.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vget_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vloxseg2ei32_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vlseg2e32_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vlseg2e32ff_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vlsseg2e32_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vluxseg2ei32_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vset_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsoxseg2ei32_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsseg2e32_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vssseg2e32_tuple.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vsuxseg2ei32_tuple.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/rvv-tuple-type.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vget-index-out-of-range.c
  clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted/vset-index-out-of-range.c
  clang/test/CodeGen/assignment-tracking/flag.cpp
  clang/test/CodeGen/ubsan-function.c
  clang/test/PCH/asm-label.cpp
  clang/test/Sema/flexible-array-in-union.c
  clang/test/Sema/riscv-types.c
  clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
  

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

In D151087#4361367 , @ebevhan wrote:

> I don't think the standard argument really holds. It doesn't mention 
> restrictions on address spaces because it doesn't have to, since they don't 
> exist in C++. If they did, I'm pretty sure that reinterpret_cast would 
> disallow arbitrary address space-modifying casts, since they wouldn't 
> necessarily be bitcasts.

The reason cited for this change was that the standard does not allow 
`reinterpret_cast` to drop qualifiers, I don't think that's true so we're free 
to define our own behavior. Whether or not this is safe or desirable is up to 
us, and I'm arguing that for C++ it should be permitted.

> Like @arsenm said, any behaviors that you're using or observing regarding 
> conversion of target address spaces in both C and C++ are purely 
> coincidental. I don't think it's a great idea to add more such coincidental 
> behaviors. The result will be that future code will become dependent on these 
> arbitrary, less restrictive behaviors, and it will be much harder to properly 
> define sensible semantics later on.

The behavior isn't coincidental, it does exactly what you expect if you were to 
use the same address space in LLVM-IR. The problem is that there are no 
semantic checks on them so if you use them incorrectly it will break. I feel 
like this is a separate issue and I don't know why it would prevent us from 
doing *any* kind of address space cast in C++.  There is no alternative in C++ 
and we already permit this with C-style casts, we cannot use OpenCL extensions 
like `addrspace_cast` so that leaves us unable to use C++ to write programs we 
want to write. We already use numerical address spaces in the OpenMP GPU 
runtime, and internally there we need to use C to do all the address space 
casts because of this.

There would be definite value in using the target's information to map address 
spaces to the known OpenCL ones so we could share some of their checks, but I 
feel like that's a separate issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151087

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

[PATCH] D147844: [clang][Sema]Print diagnostic warning about precedence when integer expression is used without parentheses in an conditional operator expression

[NagaChaitanya Vellanki via Phabricator via cfe-commits]

chaitanyav updated this revision to Diff 524376.
chaitanyav added a comment.

Rebase with upstream


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147844

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Analysis/uninit-vals.c
  clang/test/Sema/integer-overflow.c
  clang/test/Sema/parentheses.c
  clang/test/Sema/parentheses.cpp
  clang/test/SemaCXX/array-bounds.cpp
  clang/test/SemaCXX/integer-overflow.cpp
  libcxx/include/__chrono/duration.h
  libcxx/include/strstream
  libcxx/test/libcxx/algorithms/nth_element_stability.pass.cpp
  libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
  libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
  libcxxabi/src/cxa_personality.cpp

Index: libcxxabi/src/cxa_personality.cpp
===
--- libcxxabi/src/cxa_personality.cpp
+++ libcxxabi/src/cxa_personality.cpp
@@ -718,9 +718,7 @@
 if (actionEntry == 0)
 {
 // Found a cleanup
-results.reason = actions & _UA_SEARCH_PHASE
- ? _URC_CONTINUE_UNWIND
- : _URC_HANDLER_FOUND;
+results.reason = (actions & _UA_SEARCH_PHASE) ? _URC_CONTINUE_UNWIND : _URC_HANDLER_FOUND;
 return;
 }
 // Convert 1-based byte offset into
@@ -832,9 +830,8 @@
 // End of action list. If this is phase 2 and we have found
 // a cleanup (ttypeIndex=0), return _URC_HANDLER_FOUND;
 // otherwise return _URC_CONTINUE_UNWIND.
-results.reason = hasCleanup && actions & _UA_CLEANUP_PHASE
- ? _URC_HANDLER_FOUND
- : _URC_CONTINUE_UNWIND;
+results.reason =
+(hasCleanup && (actions & _UA_CLEANUP_PHASE)) ? _URC_HANDLER_FOUND : _URC_CONTINUE_UNWIND;
 return;
 }
 // Go to next action
@@ -1243,10 +1240,9 @@
 {
 const __shim_type_info* excpType =
 static_cast(new_exception_header->exceptionType);
-adjustedPtr =
-__getExceptionClass(_exception_header->unwindHeader) == kOurDependentExceptionClass ?
-((__cxa_dependent_exception*)new_exception_header)->primaryException :
-new_exception_header + 1;
+adjustedPtr = (__getExceptionClass(_exception_header->unwindHeader) == kOurDependentExceptionClass)
+  ? ((__cxa_dependent_exception*)new_exception_header)->primaryException
+  : new_exception_header + 1;
 if (!exception_spec_can_catch(ttypeIndex, classInfo, ttypeEncoding,
   excpType, adjustedPtr,
   unwind_exception, base))
Index: libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   std::less comp;
   std::__sort_dispatch(v.begin(), v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto deterministic_v = deterministic();
   std::sort(v.begin(), v.end());
@@ -62,7 +62,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = kSize / 2 - i * (i % 2 ? -1 : 1);
+v[i].value = kSize / 2 - i * ((i % 2) ? -1 : 1);
   }
   auto snapshot_v = v;
   auto snapshot_custom_v = v;
Index: libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
===
--- libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
+++ libcxx/test/libcxx/algorithms/partial_sort_stability.pass.cpp
@@ -32,7 +32,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto comp = std::less();
   std::__partial_sort_impl(v.begin(), v.begin() + kSize / 2, v.end(), comp);
@@ -44,7 +44,7 @@
   std::vector v;
   v.resize(kSize);
   for (int i = 0; i < kSize; ++i) {
-v[i].value = (i % 2 ? 1 : kSize / 2 + i);
+v[i].value = ((i % 2) ? 1 : kSize / 2 + i);
   }
   auto deterministic_v = deterministic();
   

[PATCH] D148573: Allow -fsanitize=function on all targets

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

(I guess this also mean we are lacking some upstream testing for ubsan + wasm + 
mc?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148573

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

[PATCH] D148573: Allow -fsanitize=function on all targets

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

This change seems to be causing problems on the emscripten auto-roller:  
https://ci.chromium.org/ui/p/emscripten-releases/builders/try/linux/b8780394114149321217/overview

Failures show up in ubsan tests and look like this:

  error: symbol '_Z4testi' unsupported subtraction expression used in 
relocation in code section.
  error: symbol '__main_argc_argv' unsupported subtraction expression used in 
relocation in code section.
  fatal error: error in backend: function sections must contain one function 
each

It seems like enabling this sanitizer perhaps uses features we don't yet 
support?  I will keep investigating but perhaps we can find a way to revert he 
effect on the wasm backend for now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148573

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

[PATCH] D148827: -fsanitize=function: support C

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG279a4d0d67c8: -fsanitize=function: support C (authored by 
MaskRay).

Changed prior to commit:
  https://reviews.llvm.org/D148827?vs=524110=524374#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148827

Files:
  clang/docs/ControlFlowIntegrity.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/ubsan-function.c
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
  compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py

Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
===
--- compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py
@@ -1,5 +1,8 @@
 if config.host_os not in ['Darwin', 'FreeBSD', 'Linux', 'NetBSD']:
   config.unsupported = True
+# Work around "Cannot represent a difference across sections"
+if config.target_arch == 'powerpc64':
+  config.unsupported = True
 # Work around "library ... not found: needed by main executable" in qemu.
 if config.android and config.target_arch not in ['x86', 'x86_64']:
   config.unsupported = True
Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
===
--- compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
@@ -1,6 +1,3 @@
-// Work around "Cannot represent a difference across sections"
-// UNSUPPORTED: target=powerpc64-{{.*}}
-
 // RUN: %clangxx -DDETERMINE_UNIQUE %s -o %t-unique
 // RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -DSHARED_LIB -fPIC -shared -o %t-so.so
 // RUN: %clangxx -std=c++17 -fsanitize=function %s -O3 -g -o %t %t-so.so
Index: compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c
===
--- /dev/null
+++ compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c
@@ -0,0 +1,14 @@
+// RUN: %clang -g -fsanitize=function %s -o %t
+// RUN: %run %t 2>&1 | FileCheck %s --check-prefix=CHECK --implicit-check-not='runtime error:'
+
+void f(void (*fp)(int (*)[])) { fp(0); }
+
+void callee0(int (*a)[]) {}
+void callee1(int (*a)[1]) {}
+
+int main() {
+  int a[1];
+  f(callee0);
+  // CHECK: runtime error: call to function callee1 through pointer to incorrect function type 'void (*)(int (*)[])'
+  f(callee1); // compatible type in C, but flagged
+}
Index: clang/test/CodeGen/ubsan-function.c
===
--- /dev/null
+++ clang/test/CodeGen/ubsan-function.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -emit-llvm -triple x86_64 -std=c17 -fsanitize=function %s -o - | FileCheck %s
+
+// CHECK-LABEL: define{{.*}} @call_no_prototype(
+// CHECK-NOT: __ubsan_handle_function_type_mismatch
+void call_no_prototype(void (*f)()) { f(); }
+
+// CHECK-LABEL: define{{.*}} @call_prototype(
+// CHECK: __ubsan_handle_function_type_mismatch
+void call_prototype(void (*f)(void)) { f(); }
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -572,10 +572,11 @@
 CodeGenFunction::getUBSanFunctionTypeHash(QualType Ty) const {
   // Remove any (C++17) exception specifications, to allow calling e.g. a
   // noexcept function through a non-noexcept pointer.
-  auto ProtoTy = getContext().getFunctionTypeWithExceptionSpec(Ty, EST_None);
+  if (!isa(Ty))
+Ty = getContext().getFunctionTypeWithExceptionSpec(Ty, EST_None);
   std::string Mangled;
   llvm::raw_string_ostream Out(Mangled);
-  CGM.getCXXABI().getMangleContext().mangleTypeName(ProtoTy, Out, false);
+  CGM.getCXXABI().getMangleContext().mangleTypeName(Ty, Out, false);
   return llvm::ConstantInt::get(CGM.Int32Ty,
 static_cast(llvm::xxHash64(Mangled)));
 }
@@ -945,7 +946,7 @@
 
   // If we are checking function types, emit a function type signature as
   // prologue data.
-  if (FD && getLangOpts().CPlusPlus && SanOpts.has(SanitizerKind::Function)) {
+  if (FD && SanOpts.has(SanitizerKind::Function)) {
 if (llvm::Constant *PrologueSig = getPrologueSignature(CGM, FD)) {
   llvm::LLVMContext  = Fn->getContext();
   llvm::MDBuilder MDB(Ctx);
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5349,8 +5349,9 @@
 
   CGCallee Callee = OrigCallee;
 
-  if (getLangOpts().CPlusPlus && 

[clang] 279a4d0 - -fsanitize=function: support C

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2023-05-22T10:11:30-07:00
New Revision: 279a4d0d67c874e80c171666822f2fabdd6fa926

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

LOG: -fsanitize=function: support C

With D148785, -fsanitize=function no longer uses C++ RTTI objects and therefore
can support C. The rationale for reporting errors is C11 6.5.2.2p9:

> If the function is defined with a type that is not compatible with the type 
> (of the expression) pointed to by the expression that denotes the called 
> function, the behavior is undefined.

The mangled types approach we use does not exactly match the C type
compatibility (see `f(callee1)` below).
This is probably fine as the rules are unlikely leveraged in practice. In
addition, the call is warned by -Wincompatible-function-pointer-types-strict.

```
void callee0(int (*a)[]) {}
void callee1(int (*a)[1]) {}
void f(void (*fp)(int (*)[])) { fp(0); }
int main() {
  int a[1];
  f(callee0);
  f(callee1); // compatible but flagged by -fsanitize=function, 
-fsanitize=kcfi, and -Wincompatible-function-pointer-types-strict
}
```

Skip indirect call sites of a function type without a prototype to avoid deal
with C11 6.5.2.2p6. -fsanitize=kcfi skips such calls as well.

Reviewed By: #sanitizers, vitalybuka

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

Added: 
clang/test/CodeGen/ubsan-function.c
compiler-rt/test/ubsan/TestCases/TypeCheck/Function/c.c

Modified: 
clang/docs/ControlFlowIntegrity.rst
clang/docs/UndefinedBehaviorSanitizer.rst
clang/lib/CodeGen/CGExpr.cpp
clang/lib/CodeGen/CodeGenFunction.cpp
compiler-rt/test/ubsan/TestCases/TypeCheck/Function/function.cpp
compiler-rt/test/ubsan/TestCases/TypeCheck/Function/lit.local.cfg.py

Removed: 




diff  --git a/clang/docs/ControlFlowIntegrity.rst 
b/clang/docs/ControlFlowIntegrity.rst
index f375199f40617..7de805e2154db 100644
--- a/clang/docs/ControlFlowIntegrity.rst
+++ b/clang/docs/ControlFlowIntegrity.rst
@@ -314,10 +314,8 @@ to find bugs in local development builds, whereas 
``-fsanitize=cfi-icall``
 is a security hardening mechanism designed to be deployed in release builds.
 
 ``-fsanitize=function`` has a higher space and time overhead due to a more
-complex type check at indirect call sites, as well as a need for run-time
-type information (RTTI), which may make it unsuitable for deployment. Because
-of the need for RTTI, ``-fsanitize=function`` can only be used with C++
-programs, whereas ``-fsanitize=cfi-icall`` can protect both C and C++ programs.
+complex type check at indirect call sites, which may make it unsuitable for
+deployment.
 
 On the other hand, ``-fsanitize=function`` conforms more closely with the C++
 standard and user expectations around interaction with shared libraries;

diff  --git a/clang/docs/UndefinedBehaviorSanitizer.rst 
b/clang/docs/UndefinedBehaviorSanitizer.rst
index ff02ce8ad892d..bfcdeba0632e5 100644
--- a/clang/docs/UndefinedBehaviorSanitizer.rst
+++ b/clang/docs/UndefinedBehaviorSanitizer.rst
@@ -100,7 +100,7 @@ Available checks are:
  by Clang (and by ISO/IEC/IEEE 60559 / IEEE 754) as producing either an
  infinity or NaN value, so is not included in ``-fsanitize=undefined``.
   -  ``-fsanitize=function``: Indirect call of a function through a
- function pointer of the wrong type (C++ only).
+ function pointer of the wrong type.
   -  ``-fsanitize=implicit-unsigned-integer-truncation``,
  ``-fsanitize=implicit-signed-integer-truncation``: Implicit conversion 
from
  integer of larger bit width to smaller bit width, if that results in data

diff  --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index a915849a9822d..736f3e8bf2d16 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -5349,8 +5349,9 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, 
const CGCallee 
 
   CGCallee Callee = OrigCallee;
 
-  if (getLangOpts().CPlusPlus && SanOpts.has(SanitizerKind::Function) &&
-  (!TargetDecl || !isa(TargetDecl))) {
+  if (SanOpts.has(SanitizerKind::Function) &&
+  (!TargetDecl || !isa(TargetDecl)) &&
+  !isa(PointeeType)) {
 if (llvm::Constant *PrefixSig =
 CGM.getTargetCodeGenInfo().getUBSanFunctionSignature(CGM)) {
   SanitizerScope SanScope(this);

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index e52c4831087f7..a0a6299039db3 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -572,10 +572,11 @@ llvm::ConstantInt *
 CodeGenFunction::getUBSanFunctionTypeHash(QualType Ty) const {
   // Remove any (C++17) exception specifications, to allow calling e.g. a
   // noexcept function through a non-noexcept pointer.
-  auto 

[PATCH] D151087: [Clang] Permit address space casts with 'reinterpret_cast' in C++

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

In D151087#4361263 , @jhuber6 wrote:

> I'd rather have an operation whose semantics are a little dangerous than 
> something that doesn't work at all. As it stands we need to use C-style casts 
> for this and I don't think there's a good reason to forbid this at least from 
> the C++ standard point of view. For OpenCL where we have the concept of 
> address spaces it makes sense, but not for C++.

I don't think the standard argument really holds. It doesn't mention 
restrictions on address spaces because it doesn't have to, since they don't 
exist in C++. If they did, I'm pretty sure that reinterpret_cast would disallow 
arbitrary address space-modifying casts, since they wouldn't necessarily be 
bitcasts.

Like @arsenm said, any behaviors that you're using or observing regarding 
conversion of target address spaces in both C and C++ are purely coincidental. 
I don't think it's a great idea to add more such coincidental behaviors. The 
result will be that future code will become dependent on these arbitrary, less 
restrictive behaviors, and it will be much harder to properly define sensible 
semantics later on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151087

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

[PATCH] D145403: [Pipeline] Don't run EarlyFPM in LTO post link

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

I was trying to get this landed with https://reviews.llvm.org/D145265 so I 
could measure the impact of the two together. But also now there are some merge 
conflicts that I'd rather deal with after submitting D145265 
. But that patch is more likely to get 
reverted anyway so yeah this should go in first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145403

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

[PATCH] D151121: [Clang][UBSan] Fix the crash caused by __builtin_assume_aligned with -no-opaque-pointers enabled

[Yurong via Phabricator via cfe-commits]

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

Signed-off-by: yronglin 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151121

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/catch-alignment-assumption-array.c
  
clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-polymorphism-no-opaque-ptr.cpp

Index: clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-polymorphism-no-opaque-ptr.cpp
===
--- /dev/null
+++ clang/test/CodeGen/catch-alignment-assumption-builtin_assume_aligned-polymorphism-no-opaque-ptr.cpp
@@ -0,0 +1,61 @@
+// RUN: %clang_cc1 -no-opaque-pointers -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fno-sanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-NORECOVER,CHECK-SANITIZE-UNREACHABLE
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-recover=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-ANYRECOVER,CHECK-SANITIZE-RECOVER
+// RUN: %clang_cc1 -no-opaque-pointers -fsanitize=alignment -fsanitize-trap=alignment -emit-llvm %s -o - -triple x86_64-linux-gnu | FileCheck %s -implicit-check-not="call void @__ubsan_handle_alignment_assumption" --check-prefixes=CHECK,CHECK-SANITIZE,CHECK-SANITIZE-TRAP,CHECK-SANITIZE-UNREACHABLE
+
+// CHECK-SANITIZE-ANYRECOVER: @[[CHAR:.*]] = {{.*}} c"'B *'\00" }
+// CHECK-SANITIZE-ANYRECOVER: @[[LINE_100_ALIGNMENT_ASSUMPTION:.*]] = {{.*}}, i32 100, i32 35 }, {{.*}} @[[CHAR]] }
+
+struct A { int n; };
+struct B { int n; };
+struct C : A, B {};
+
+void *f(C *c) {
+  // CHECK: define {{.*}} i8* @{{.*}}(%struct.C* noundef %[[C:.*]]) {{.*}} {
+  // CHECK-NEXT:[[ENTRY:.*]]:
+  // CHECK-NEXT:  %[[C_ADDR:.*]] = alloca %struct.C*
+  // CHECK-NEXT:  store  %struct.C* %[[C]], %struct.C** %[[C_ADDR]]
+  // CHECK-NEXT:  %[[C_RELOAD:.*]] = load %struct.C*, %struct.C** %[[C_ADDR]]
+  // CHECK-NEXT:  %[[IS_NULL:.*]] = icmp eq %struct.C* %[[C_RELOAD]], null
+  // CHECK-NEXT:  br i1 %[[IS_NULL]], label %[[CAST_END:[^,]+]], label %[[CAST_NOT_NULL:[^,]+]]
+  // CHECK: [[CAST_NOT_NULL]]:
+  // CHECK-NOSANITIZE-NEXT:   %[[ADD_PTR:.*]] = getelementptr inbounds i8, ptr %[[C_RELOAD]], i64 4
+  // CHECK-NOSANITIZE-NEXT:   br label %[[CAST_END]]
+  // CHECK-SANITIZE-NEXT: %[[PTRTOINT:.*]] = ptrtoint %struct.C* %[[C_RELOAD]] to i64
+  // CHECK-SANITIZE-NEXT: %[[MASKEDPTR:.*]] = and i64 %[[PTRTOINT]], 3
+  // CHECK-SANITIZE-NEXT: %[[MASKCOND:.*]] = icmp eq i64 %[[MASKEDPTR]], 0
+  // CHECK-SANITIZE-NEXT: br i1 %[[MASKCOND]], label %[[CONT:[^,]+]], label %[[HANDLER_TYPE_MISMATCH:[^,]+]]
+  // CHECK-SANITIZE:[[HANDLER_TYPE_MISMATCH]]:
+  // CHECK-SANITIZE-NORECOVER-NEXT:   call void @__ubsan_handle_type_mismatch_v1_abort(
+  // CHECK-SANITIZE-RECOVER-NEXT: call void @__ubsan_handle_type_mismatch_v1(
+  // CHECK-SANITIZE-TRAP-NEXT:call void @llvm.ubsantrap(
+  // CHECK-SANITIZE-UNREACHABLE-NEXT: unreachable
+  // CHECK-SANITIZE:[[CONT]]:
+  // CHECK-SANITIZE-NEXT: %[[CONT_BITCAST:.*]] = bitcast %struct.C* %0 to i8*
+  // CHECK-SANITIZE-NEXT: %[[ADD_PTR:.*]] = getelementptr inbounds i8, i8* %[[CONT_BITCAST]], i64 4
+  // CHECK-SANITIZE-NEXT: %[[CONT_BITCAST1:.*]] = bitcast i8* %[[ADD_PTR]] to %struct.B*
+  // CHECK-SANITIZE-NEXT: br label %[[CAST_END]]
+  // CHECK: [[CAST_END]]:
+  // CHECK-NOSANITIZE-NEXT:   %[[CAST_RESULT:.*]] = phi %struct.B* [ %[[CONT_BITCAST1]], %[[CAST_NOT_NULL]] ], [ null, %[[ENTRY]] ]
+  // CHECK-NOSANITIZE-NEXT:   call void @llvm.assume(i1 true) [ "align"(ptr %[[CAST_RESULT]], i64 8) ]
+  // CHECK-NOSANITIZE-NEXT:   ret ptr %[[CAST_RESULT]]
+  // CHECK-NOSANITIZE-NEXT:  }
+  // CHECK-SANITIZE-NEXT: %[[CAST_RESULT:.*]] = phi %struct.B* [ %[[CONT_BITCAST1]], %[[CONT]] ], [ null, %[[ENTRY]] ]
+  // CHECK-SANITIZE-NEXT: %[[CAST_RESULT1:.*]] = bitcast %struct.B* %[[CAST_RESULT]] to i8*
+  // CHECK-SANITIZE-NEXT: