[PATCH] D130701: [clang-tidy] Rename a local cmake variables to match the new tool name. NFC.

[Sam McCall via Phabricator via cfe-commits]

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

Sorry Martin, I took some time off work and missed this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130701

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

[PATCH] D131763: [OpenMP] Add lit test for metadirective device arch inspired from sollve

[Saiyedul Islam via Phabricator via cfe-commits]

saiislam added a comment.

In D131763#3719323 , @jdoerfert wrote:

> In D131763#3719140 , @saiislam 
> wrote:
>
>> In D131763#3719132 , @jdoerfert 
>> wrote:
>>
>>> This doesn't actually test much, only once case/compilation is covered. In 
>>> the second function nothing specific to LLVM as impl is checked.
>>
>> The second function, is the only place in llvm-project where vendor(llvm) is 
>> being tested for a non-error test.
>
> Really?
>
>   ag 'vendor\(llvm\)' clang/test/OpenMP --files-with-matches  
>   
> 
>   
>   clang/test/OpenMP/begin_declare_variant_messages.c
>   clang/test/OpenMP/begin_declare_variant_using_messages.cpp
>   clang/test/OpenMP/declare_variant_ast_print.c
>   clang/test/OpenMP/declare_variant_ast_print.cpp
>   clang/test/OpenMP/declare_variant_implementation_vendor_codegen.cpp
>   clang/test/OpenMP/declare_variant_messages.cpp
>   clang/test/OpenMP/declare_variant_mixed_codegen.cpp
>   clang/test/OpenMP/metadirective_ast_print.c
>   clang/test/OpenMP/metadirective_implementation_codegen.c
>   clang/test/OpenMP/nvptx_declare_variant_implementation_vendor_codegen.cpp
>   clang/test/OpenMP/declare_variant_messages.c
>   clang/test/OpenMP/metadirective_empty.cpp
>   clang/test/OpenMP/metadirective_implementation_codegen.cpp
>
> That said, the above function still doesn't test anything wrt. llvm as impl 
> anyway. We could just as well match amd or nvidia and the check lines still 
> match just fine.

My mistake. You are right about more tests being present. I searched in the 
wrong branch locally :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131763

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

[PATCH] D131385: [clangd] Support for standard type hierarchy

[Utkarsh Saxena via Phabricator via cfe-commits]

usaxena95 added inline comments.



Comment at: clang-tools-extra/clangd/Protocol.cpp:1228
+Result["parents"] = RP.parents;
+  return std::move(Result);
+}

Nit: Allow RVO.



Comment at: clang-tools-extra/clangd/Protocol.h:1415
+  /// The range that should be selected and revealed when this symbol is being
+  /// picked, e.g. the name of a function. Must be contained by the range.
   Range selectionRange;

nit.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1954-1964
   [&AST](const SelectionTree::Node *N) -> std::vector {
 std::vector LocatedSymbols;
 
 // NOTE: unwrapFindType might return duplicates for something like
-// unique_ptr>. Let's *not* remove them, because it gives 
you some
-// information about the type you may have not known before
-// (since unique_ptr> != unique_ptr).
-for (const QualType& Type : unwrapFindType(typeForNode(N), 
AST.getHeuristicResolver()))
-llvm::copy(locateSymbolForType(AST, Type), 
std::back_inserter(LocatedSymbols));
+// unique_ptr>. Let's *not* remove them, because it gives you
+// some information about the type you may have not known before (since
+// unique_ptr> != unique_ptr).

Can you revert the formatting changes in l:1894-1964




Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:780
+  withResolveParents(HasValue(UnorderedElementsAre(
+  withResolveID(Result.front().data.symbolID.str(;
+}

It would be clearer if we use `getSymbolID(&findDecl(AST, "Parent1"))` here and 
in `SuperTypes`.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131385

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

[clang-tools-extra] 0b90e13 - [pseudo] Style tweaks forgotten in D130337. NFC

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-08-16T10:26:25+02:00
New Revision: 0b90e136eee928071656948b7fe1edc5ff36fcdf

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

LOG: [pseudo] Style tweaks forgotten in D130337. NFC

Added: 


Modified: 
clang-tools-extra/pseudo/lib/cxx/cxx.bnf

Removed: 




diff  --git a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf 
b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
index 697e79d609aa..bc6599c4e3c4 100644
--- a/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ b/clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -375,18 +375,19 @@ simple-type-specifier := nested-name-specifier TEMPLATE 
simple-template-id
 simple-type-specifier := decltype-specifier
 simple-type-specifier := placeholder-type-specifier
 simple-type-specifier := nested-name-specifier_opt template-name
+simple-type-specifier := SHORT
+simple-type-specifier := LONG
+simple-type-specifier := SIGNED
+simple-type-specifier := UNSIGNED
 simple-type-specifier := builtin-type
+#! builtin-type added to aid in classifying which specifiers may combined.
 builtin-type := CHAR
 builtin-type := CHAR8_T
 builtin-type := CHAR16_T
 builtin-type := CHAR32_T
 builtin-type := WCHAR_T
 builtin-type := BOOL
-simple-type-specifier := SHORT
 builtin-type := INT
-simple-type-specifier := LONG
-simple-type-specifier := SIGNED
-simple-type-specifier := UNSIGNED
 builtin-type := FLOAT
 builtin-type := DOUBLE
 builtin-type := VOID



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

[PATCH] D131938: [C++20] [Coroutines] Disable to take the address of labels in coroutines

[Adrian Vogelsgesang via Phabricator via cfe-commits]

avogelsgesang added a comment.

looks good to me on a high-level, but I don't know clang well enough to 
confidently approve this change




Comment at: clang/lib/Sema/SemaCoroutine.cpp:1107
+
+  // Corotuines will get splitted into pieces. The GNU address of label
+  // extension wouldn't be meaningful in coroutines.

Corotuines -> Coroutines



Comment at: clang/test/SemaCXX/addr-label-in-coroutines.cpp:5
+
+struct Allocator {};
+

unused


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

https://reviews.llvm.org/D131938

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

[PATCH] D129231: [Builtins] Do not claim all libfuncs are readnone with trapping math.

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

@john.brawn Are you OK with treating those functions as not setting inexact, as 
per the C23 clarification, as the committed version does?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129231

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

[PATCH] D131762: [pseudo][wip] Enforce the C++ rule for the optional init-declarator-list.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

> Basically it implements the https://eel.is/c++draft/dcl.pre#5 rule.

Just to make sure we're on the same page, my understanding of that rule is 
roughly "declarations must declare something", and it has two parts:

- the declarators can only be omitted if a named class/enum is declared (this 
patch)
- "shall (re)introduce one or more names": if the declarators are omitted, 
either the class/enum must be named or the enum must provide enumerators (not 
in this patch)

This corresponds to clang's `-Wmissing-declarations`, which is a warning rather 
than an error! I think this is an argument to allow these parses at least in 
cases when there's no correct parse.
Ideally we'd eliminate the wrong one when there's actual ambiguity, this isn't 
really doable with guards, the closest mechanism we have is "soft" 
disambiguation.

> regress the `const Foo;` case, now we parse it as a declaration with a 
> declarator named Foo and const as the decl-specifier-seq, vs we used to 
> parsed as a declaration without declarator (which is better IMO);

I don't think it's clear. Foo could easily be a easily type with missing 
variable name or a variable name with missing type. A human would interpret 
this based on the other occurrences of `Foo`, based on case, etc. I think this 
is an argument in favor of solving this "softly" with ranking (though not a 
strong one: we can't let 'correctly' parsing broken code drive the parser *too* 
much).

> My feeling is that this degrades the parser a lot for the little benefit we 
> gain, we probably don't move forward with this approach.

I agree, I think we should solve this by downranking e.g. declarator-less 
declarations (so they're only used if needed). It's not ideal (I'd like to 
completely drop the misparse of `enum foo;` and similar cases where there's a 
valid alternative), but this isn't an *incredibly* common pattern that demands 
complete early removal from the forest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131762

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

[PATCH] D131938: [C++20] [Coroutines] Disable to take the address of labels in coroutines

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu updated this revision to Diff 452925.
ChuanqiXu added a comment.

Address comments.


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

https://reviews.llvm.org/D131938

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/ScopeInfo.h
  clang/lib/Sema/ScopeInfo.cpp
  clang/lib/Sema/SemaCoroutine.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/addr-label-in-coroutines.cpp

Index: clang/test/SemaCXX/addr-label-in-coroutines.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/addr-label-in-coroutines.cpp
@@ -0,0 +1,70 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+#include "Inputs/std-coroutine.h"
+
+struct resumable {
+  struct promise_type {
+resumable get_return_object() { return {}; }
+auto initial_suspend() { return std::suspend_always(); }
+auto final_suspend() noexcept { return std::suspend_always(); }
+void unhandled_exception() {}
+void return_void(){};
+  };
+};
+
+resumable f1(int &out, int *inst) {
+static void* dispatch_table[] = {&&inc,  // expected-error {{the GNU address of label extension is not allowed in coroutines.}}
+ &&suspend,  // expected-error {{the GNU address of label extension is not allowed in coroutines.}}
+ &&stop};// expected-error {{the GNU address of label extension is not allowed in coroutines.}}
+#define DISPATCH() goto *dispatch_table[*inst++]
+inc:
+out++;
+DISPATCH();
+
+suspend:
+co_await std::suspend_always{};
+DISPATCH();
+
+stop:
+co_return;
+}
+
+resumable f2(int &out, int *inst) {
+void* dispatch_table[] = {nullptr, nullptr, nullptr};
+dispatch_table[0] = &&inc;  // expected-error {{the GNU address of label extension is not allowed in coroutines.}}
+dispatch_table[1] = &&suspend;  // expected-error {{the GNU address of label extension is not allowed in coroutines.}}
+dispatch_table[2] = &&stop; // expected-error {{the GNU address of label extension is not allowed in coroutines.}}
+#define DISPATCH() goto *dispatch_table[*inst++]
+inc:
+out++;
+DISPATCH();
+
+suspend:
+co_await std::suspend_always{};
+DISPATCH();
+
+stop:
+co_return;
+}
+
+resumable f3(int &out, int *inst) {
+void* dispatch_table[] = {nullptr, nullptr, nullptr};
+[&]() -> resumable {
+dispatch_table[0] = &&inc;  // expected-error {{the GNU address of label extension is not allowed in coroutines.}}
+dispatch_table[1] = &&suspend;  // expected-error {{the GNU address of label extension is not allowed in coroutines.}}
+dispatch_table[2] = &&stop; // expected-error {{the GNU address of label extension is not allowed in coroutines.}}
+#define DISPATCH() goto *dispatch_table[*inst++]
+inc:
+out++;
+DISPATCH();
+
+suspend:
+co_await std::suspend_always{};
+DISPATCH();
+
+stop:
+co_return;
+}();
+
+co_return;
+}
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15840,8 +15840,13 @@
 LabelDecl *TheDecl) {
   TheDecl->markUsed(Context);
   // Create the AST node.  The address of a label always has type 'void*'.
-  return new (Context) AddrLabelExpr(OpLoc, LabLoc, TheDecl,
- Context.getPointerType(Context.VoidTy));
+  auto *Res = new (Context) AddrLabelExpr(
+  OpLoc, LabLoc, TheDecl, Context.getPointerType(Context.VoidTy));
+
+  if (getCurFunction())
+getCurFunction()->AddrLabels.push_back(Res);
+
+  return Res;
 }
 
 void Sema::ActOnStartStmtExpr() {
Index: clang/lib/Sema/SemaCoroutine.cpp
===
--- clang/lib/Sema/SemaCoroutine.cpp
+++ clang/lib/Sema/SemaCoroutine.cpp
@@ -1103,6 +1103,12 @@
 Diag(Fn->FirstCoroutineStmtLoc, diag::note_declared_coroutine_here)
 << Fn->getFirstCoroutineStmtKeyword();
   }
+
+  // Coroutines will get splitted into pieces. The GNU address of label
+  // extension wouldn't be meaningful in coroutines.
+  for (AddrLabelExpr *ALE : Fn->AddrLabels)
+Diag(ALE->getBeginLoc(), diag::err_coro_invalid_addr_of_label);
+
   CoroutineStmtBuilder Builder(*this, *FD, *Fn, Body);
   if (Builder.isInvalid() || !Builder.buildStatements())
 return FD->setInvalidDecl();
Index: clang/lib/Sema/ScopeInfo.cpp
===
--- clang/lib/Sema/ScopeInfo.cpp
+++ clang/lib/Sema/ScopeInfo.cpp
@@ -56,6 +56,7 @@
   ModifiedNonNullParams.clear();
   Blocks.clear();
   ByrefBlockVars.clear();
+  AddrLabels.clear();
 }
 
 static const NamedDecl *getBestPropertyDecl(const ObjCPropertyRefExpr *PropE) {
Index: clang/include/clang/Sema/ScopeI

[PATCH] D131385: [clangd] Support for standard type hierarchy

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 3 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/Protocol.cpp:1228
+Result["parents"] = RP.parents;
+  return std::move(Result);
+}

usaxena95 wrote:
> Nit: Allow RVO.
there's an issue with one of the compilers used by a build-bot which can't find 
the relevant constructor if we don't have std::move here (hence we've the same 
pattern across others). we should definitely check at some point to see if that 
compiler is gone, but i'd rather not do that in the scope of this patch.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1954-1964
   [&AST](const SelectionTree::Node *N) -> std::vector {
 std::vector LocatedSymbols;
 
 // NOTE: unwrapFindType might return duplicates for something like
-// unique_ptr>. Let's *not* remove them, because it gives 
you some
-// information about the type you may have not known before
-// (since unique_ptr> != unique_ptr).
-for (const QualType& Type : unwrapFindType(typeForNode(N), 
AST.getHeuristicResolver()))
-llvm::copy(locateSymbolForType(AST, Type), 
std::back_inserter(LocatedSymbols));
+// unique_ptr>. Let's *not* remove them, because it gives you
+// some information about the type you may have not known before (since
+// unique_ptr> != unique_ptr).

usaxena95 wrote:
> Can you revert the formatting changes in l:1894-1964
> 
oops, thanks for noticing.



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:780
+  withResolveParents(HasValue(UnorderedElementsAre(
+  withResolveID(Result.front().data.symbolID.str(;
+}

usaxena95 wrote:
> It would be clearer if we use `getSymbolID(&findDecl(AST, "Parent1"))` here 
> and in `SuperTypes`.
> 
> 
not sure what you mean by also doing it in `SuperTypes`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131385

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

[PATCH] D131385: [clangd] Support for standard type hierarchy

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 452927.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131385

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/test/type-hierarchy-ext.test
  clang-tools-extra/clangd/test/type-hierarchy.test
  clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp

Index: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
===
--- clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
+++ clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp
@@ -5,6 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
+#include "AST.h"
 #include "Annotations.h"
 #include "Matchers.h"
 #include "ParsedAST.h"
@@ -16,6 +17,7 @@
 #include "llvm/Support/Path.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include 
 
 namespace clang {
 namespace clangd {
@@ -26,6 +28,7 @@
 using ::testing::Field;
 using ::testing::IsEmpty;
 using ::testing::Matcher;
+using ::testing::SizeIs;
 using ::testing::UnorderedElementsAre;
 
 // GMock helpers for matching TypeHierarchyItem.
@@ -45,6 +48,10 @@
 // Note: "not resolved" is different from "resolved but empty"!
 MATCHER(parentsNotResolved, "") { return !arg.parents; }
 MATCHER(childrenNotResolved, "") { return !arg.children; }
+MATCHER_P(withResolveID, SID, "") { return arg.symbolID.str() == SID; }
+MATCHER_P(withResolveParents, M, "") {
+  return testing::ExplainMatchResult(M, arg.data.parents, result_listener);
+}
 
 TEST(FindRecordTypeAt, TypeOrVariable) {
   Annotations Source(R"cpp(
@@ -64,8 +71,10 @@
   auto AST = TU.build();
 
   for (Position Pt : Source.points()) {
-const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
-EXPECT_EQ(&findDecl(AST, "Child2"), static_cast(RD));
+auto Records = findRecordTypeAt(AST, Pt);
+ASSERT_THAT(Records, SizeIs(1));
+EXPECT_EQ(&findDecl(AST, "Child2"),
+  static_cast(Records.front()));
   }
 }
 
@@ -86,8 +95,10 @@
   auto AST = TU.build();
 
   for (Position Pt : Source.points()) {
-const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
-EXPECT_EQ(&findDecl(AST, "Child2"), static_cast(RD));
+auto Records = findRecordTypeAt(AST, Pt);
+ASSERT_THAT(Records, SizeIs(1));
+EXPECT_EQ(&findDecl(AST, "Child2"),
+  static_cast(Records.front()));
   }
 }
 
@@ -107,11 +118,10 @@
   auto AST = TU.build();
 
   for (Position Pt : Source.points()) {
-const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
 // A field does not unambiguously specify a record type
 // (possible associated reocrd types could be the field's type,
 // or the type of the record that the field is a member of).
-EXPECT_EQ(nullptr, RD);
+EXPECT_THAT(findRecordTypeAt(AST, Pt), SizeIs(0));
   }
 }
 
@@ -359,11 +369,11 @@
   for (Position Pt : Source.points()) {
 // Set ResolveLevels to 0 because it's only used for Children;
 // for Parents, getTypeHierarchy() always returns all levels.
-llvm::Optional Result = getTypeHierarchy(
-AST, Pt, /*ResolveLevels=*/0, TypeHierarchyDirection::Parents);
-ASSERT_TRUE(bool(Result));
+auto Result = getTypeHierarchy(AST, Pt, /*ResolveLevels=*/0,
+   TypeHierarchyDirection::Parents);
+ASSERT_THAT(Result, SizeIs(1));
 EXPECT_THAT(
-*Result,
+Result.front(),
 AllOf(
 withName("Child"), withKind(SymbolKind::Struct),
 parents(AllOf(withName("Parent1"), withKind(SymbolKind::Struct),
@@ -398,11 +408,11 @@
   // The parent is reported as "S" because "S<0>" is an invalid instantiation.
   // We then iterate once more and find "S" again before detecting the
   // recursion.
-  llvm::Optional Result = getTypeHierarchy(
-  AST, Source.points()[0], 0, TypeHierarchyDirection::Parents);
-  ASSERT_TRUE(bool(Result));
+  auto Result = getTypeHierarchy(AST, Source.points()[0], 0,
+ TypeHierarchyDirection::Parents);
+  ASSERT_THAT(Result, SizeIs(1));
   EXPECT_THAT(
-  *Result,
+  Result.front(),
   AllOf(withName("S<0>"), withKind(SymbolKind::Struct),
 parents(
 AllOf(withName("S"), withKind(SymbolKind::Struct),
@@ -432,11 +442,11 @@
 
   // Make sure getTypeHierarchy() doesn't get into an infinite recursion
   // for either a concrete starting point or a dependent

[PATCH] D121365: [CFG] Fix crash on CFG building when deriving from a template.

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

Thanks !

In D121365#3720171 , @NoQ wrote:

> Hi, interesting, the code looks great but at a glance I don't see why would 
> you even want a CFG for an uninstantiated template. Every time you want to 
> analyze the actual runtime behavior of a program, you'll have a fully 
> instantiated template AST to build the CFG from.

I'm hitting this for an analysis that is eagerly analyzing templates before 
instantiation, and does not require the base classes. It's always better to not 
crash anyway :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121365

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

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

[Vladimir Plyashkun via Phabricator via cfe-commits]

vladimir.plyashkun updated this revision to Diff 452936.
vladimir.plyashkun added a comment.

- return operand source range in too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131678

Files:
  clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise-integer-literals.cpp
  clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/hicpp/signed-bitwise.cpp
@@ -42,7 +42,7 @@
   UResult = SValue & -1;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
   UResult&= 1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
 
   UResult = UValue & 1u; // Ok
   UResult = UValue & UValue; // Ok
@@ -63,43 +63,43 @@
 
   // More complex expressions.
   UResult = UValue & (SByte1 + (SByte1 | SByte2));
-  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use of a signed integer operand with a binary bitwise operator
   // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: use of a signed integer operand with a binary bitwise operator
 
   // The rest is to demonstrate functionality but all operators are matched equally.
   // Therefore functionality is the same for all binary operations.
   UByte1 = UByte1 | UByte2; // Ok
   UByte1 = UByte1 | SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use of a signed integer operand with a binary bitwise operator
   UByte1|= SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
   UByte1|= UByte2; // Ok
 
   UByte1 = UByte1 ^ UByte2; // Ok
   UByte1 = UByte1 ^ SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:21: warning: use of a signed integer operand with a binary bitwise operator
   UByte1^= SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
   UByte1^= UByte2; // Ok
 
   UByte1 = UByte1 >> UByte2; // Ok
   UByte1 = UByte1 >> SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use of a signed integer operand with a binary bitwise operator
   UByte1>>= SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
   UByte1>>= UByte2; // Ok
 
   UByte1 = UByte1 << UByte2; // Ok
   UByte1 = UByte1 << SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: use of a signed integer operand with a binary bitwise operator
   UByte1<<= SByte2;
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
   UByte1<<= UByte2; // Ok
 
   int SignedInt1 = 1 << 12;
   // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
   int SignedInt2 = 1u << 12;
-  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: use of a signed integer operand with a binary bitwise operator
 }
 
 void f1(unsigned char c) {}
@@ -113,28 +113,28 @@
   UByte1 = ~UByte1; // Ok
   SByte1 = ~UByte1;
   SByte1 = ~SByte1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator
   UByte1 = ~SByte1;
-  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of 

[PATCH] D127293: [clang-tidy] Ignore other members in a union if any member of it is initialized in cppcoreguidelines-pro-type-member-init

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

Hi, @LegalizeAdulthood, are you okay with the way this has progressed?


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

https://reviews.llvm.org/D127293

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

[clang] 672311b - [CFG] Fix crash on CFG building when deriving from a template.

[Clement Courbet via cfe-commits]

Author: Clement Courbet
Date: 2022-08-16T13:01:13+02:00
New Revision: 672311bd77c594888e2660c124d7eae01822fffa

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

LOG: [CFG] Fix crash on CFG building when deriving from a template.

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

Added: 


Modified: 
clang/lib/Analysis/CFG.cpp
clang/unittests/Analysis/CFGBuildResult.h
clang/unittests/Analysis/CFGTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index 5c45264896027..2b99b8e680805 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -1891,7 +1891,7 @@ void CFGBuilder::addImplicitDtorsForDestructor(const 
CXXDestructorDecl *DD) {
 // (which is 
diff erent from the current class) is responsible for
 // destroying them.
 const CXXRecordDecl *CD = VI.getType()->getAsCXXRecordDecl();
-if (!CD->hasTrivialDestructor()) {
+if (CD && !CD->hasTrivialDestructor()) {
   autoCreateBlock();
   appendBaseDtor(Block, &VI);
 }
@@ -1901,7 +1901,7 @@ void CFGBuilder::addImplicitDtorsForDestructor(const 
CXXDestructorDecl *DD) {
   for (const auto &BI : RD->bases()) {
 if (!BI.isVirtual()) {
   const CXXRecordDecl *CD = BI.getType()->getAsCXXRecordDecl();
-  if (!CD->hasTrivialDestructor()) {
+  if (CD && !CD->hasTrivialDestructor()) {
 autoCreateBlock();
 appendBaseDtor(Block, &BI);
   }

diff  --git a/clang/unittests/Analysis/CFGBuildResult.h 
b/clang/unittests/Analysis/CFGBuildResult.h
index 4851d3d7fb6d6..72ad1cc7ce401 100644
--- a/clang/unittests/Analysis/CFGBuildResult.h
+++ b/clang/unittests/Analysis/CFGBuildResult.h
@@ -56,13 +56,15 @@ class CFGCallback : public 
ast_matchers::MatchFinder::MatchCallback {
 TheBuildResult = BuildResult::SawFunctionBody;
 Options.AddImplicitDtors = true;
 if (std::unique_ptr Cfg =
-CFG::buildCFG(nullptr, Body, Result.Context, Options))
+CFG::buildCFG(Func, Body, Result.Context, Options))
   TheBuildResult = {BuildResult::BuiltCFG, Func, std::move(Cfg),
 std::move(AST)};
   }
 };
 
-inline BuildResult BuildCFG(const char *Code, CFG::BuildOptions Options = {}) {
+template 
+BuildResult BuildCFG(const char *Code, CFG::BuildOptions Options = {},
+ FuncMatcherT FuncMatcher = ast_matchers::anything()) {
   std::vector Args = {"-std=c++11",
"-fno-delayed-template-parsing"};
   std::unique_ptr AST = tooling::buildASTFromCodeWithArgs(Code, Args);
@@ -72,7 +74,8 @@ inline BuildResult BuildCFG(const char *Code, 
CFG::BuildOptions Options = {}) {
   CFGCallback Callback(std::move(AST));
   Callback.Options = Options;
   ast_matchers::MatchFinder Finder;
-  Finder.addMatcher(ast_matchers::functionDecl().bind("func"), &Callback);
+  Finder.addMatcher(ast_matchers::functionDecl(FuncMatcher).bind("func"),
+&Callback);
 
   Finder.matchAST(Callback.AST->getASTContext());
   return std::move(Callback.TheBuildResult);

diff  --git a/clang/unittests/Analysis/CFGTest.cpp 
b/clang/unittests/Analysis/CFGTest.cpp
index 1cce8bade42fe..7ce35e3fe4a4f 100644
--- a/clang/unittests/Analysis/CFGTest.cpp
+++ b/clang/unittests/Analysis/CFGTest.cpp
@@ -70,6 +70,27 @@ TEST(CFG, VariableOfIncompleteType) {
   EXPECT_EQ(BuildResult::BuiltCFG, BuildCFG(Code).getStatus());
 }
 
+// Constructing a CFG with a dependent base should not crash.
+TEST(CFG, DependantBaseAddImplicitDtors) {
+  const char *Code = R"(
+template 
+struct Base {
+  virtual ~Base() {}
+};
+
+template 
+struct Derived : public Base {
+  virtual ~Derived() {}
+};
+  )";
+  CFG::BuildOptions Options;
+  Options.AddImplicitDtors = true;
+  Options.setAllAlwaysAdd();
+  EXPECT_EQ(BuildResult::BuiltCFG,
+BuildCFG(Code, Options, ast_matchers::hasName("~Derived"))
+.getStatus());
+}
+
 TEST(CFG, IsLinear) {
   auto expectLinear = [](bool IsLinear, const char *Code) {
 BuildResult B = BuildCFG(Code);



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

[PATCH] D121365: [CFG] Fix crash on CFG building when deriving from a template.

[Clement Courbet via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG672311bd77c5: [CFG] Fix crash on CFG building when deriving 
from a template. (authored by courbet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121365

Files:
  clang/lib/Analysis/CFG.cpp
  clang/unittests/Analysis/CFGBuildResult.h
  clang/unittests/Analysis/CFGTest.cpp


Index: clang/unittests/Analysis/CFGTest.cpp
===
--- clang/unittests/Analysis/CFGTest.cpp
+++ clang/unittests/Analysis/CFGTest.cpp
@@ -70,6 +70,27 @@
   EXPECT_EQ(BuildResult::BuiltCFG, BuildCFG(Code).getStatus());
 }
 
+// Constructing a CFG with a dependent base should not crash.
+TEST(CFG, DependantBaseAddImplicitDtors) {
+  const char *Code = R"(
+template 
+struct Base {
+  virtual ~Base() {}
+};
+
+template 
+struct Derived : public Base {
+  virtual ~Derived() {}
+};
+  )";
+  CFG::BuildOptions Options;
+  Options.AddImplicitDtors = true;
+  Options.setAllAlwaysAdd();
+  EXPECT_EQ(BuildResult::BuiltCFG,
+BuildCFG(Code, Options, ast_matchers::hasName("~Derived"))
+.getStatus());
+}
+
 TEST(CFG, IsLinear) {
   auto expectLinear = [](bool IsLinear, const char *Code) {
 BuildResult B = BuildCFG(Code);
Index: clang/unittests/Analysis/CFGBuildResult.h
===
--- clang/unittests/Analysis/CFGBuildResult.h
+++ clang/unittests/Analysis/CFGBuildResult.h
@@ -56,13 +56,15 @@
 TheBuildResult = BuildResult::SawFunctionBody;
 Options.AddImplicitDtors = true;
 if (std::unique_ptr Cfg =
-CFG::buildCFG(nullptr, Body, Result.Context, Options))
+CFG::buildCFG(Func, Body, Result.Context, Options))
   TheBuildResult = {BuildResult::BuiltCFG, Func, std::move(Cfg),
 std::move(AST)};
   }
 };
 
-inline BuildResult BuildCFG(const char *Code, CFG::BuildOptions Options = {}) {
+template 
+BuildResult BuildCFG(const char *Code, CFG::BuildOptions Options = {},
+ FuncMatcherT FuncMatcher = ast_matchers::anything()) {
   std::vector Args = {"-std=c++11",
"-fno-delayed-template-parsing"};
   std::unique_ptr AST = tooling::buildASTFromCodeWithArgs(Code, Args);
@@ -72,7 +74,8 @@
   CFGCallback Callback(std::move(AST));
   Callback.Options = Options;
   ast_matchers::MatchFinder Finder;
-  Finder.addMatcher(ast_matchers::functionDecl().bind("func"), &Callback);
+  Finder.addMatcher(ast_matchers::functionDecl(FuncMatcher).bind("func"),
+&Callback);
 
   Finder.matchAST(Callback.AST->getASTContext());
   return std::move(Callback.TheBuildResult);
Index: clang/lib/Analysis/CFG.cpp
===
--- clang/lib/Analysis/CFG.cpp
+++ clang/lib/Analysis/CFG.cpp
@@ -1891,7 +1891,7 @@
 // (which is different from the current class) is responsible for
 // destroying them.
 const CXXRecordDecl *CD = VI.getType()->getAsCXXRecordDecl();
-if (!CD->hasTrivialDestructor()) {
+if (CD && !CD->hasTrivialDestructor()) {
   autoCreateBlock();
   appendBaseDtor(Block, &VI);
 }
@@ -1901,7 +1901,7 @@
   for (const auto &BI : RD->bases()) {
 if (!BI.isVirtual()) {
   const CXXRecordDecl *CD = BI.getType()->getAsCXXRecordDecl();
-  if (!CD->hasTrivialDestructor()) {
+  if (CD && !CD->hasTrivialDestructor()) {
 autoCreateBlock();
 appendBaseDtor(Block, &BI);
   }


Index: clang/unittests/Analysis/CFGTest.cpp
===
--- clang/unittests/Analysis/CFGTest.cpp
+++ clang/unittests/Analysis/CFGTest.cpp
@@ -70,6 +70,27 @@
   EXPECT_EQ(BuildResult::BuiltCFG, BuildCFG(Code).getStatus());
 }
 
+// Constructing a CFG with a dependent base should not crash.
+TEST(CFG, DependantBaseAddImplicitDtors) {
+  const char *Code = R"(
+template 
+struct Base {
+  virtual ~Base() {}
+};
+
+template 
+struct Derived : public Base {
+  virtual ~Derived() {}
+};
+  )";
+  CFG::BuildOptions Options;
+  Options.AddImplicitDtors = true;
+  Options.setAllAlwaysAdd();
+  EXPECT_EQ(BuildResult::BuiltCFG,
+BuildCFG(Code, Options, ast_matchers::hasName("~Derived"))
+.getStatus());
+}
+
 TEST(CFG, IsLinear) {
   auto expectLinear = [](bool IsLinear, const char *Code) {
 BuildResult B = BuildCFG(Code);
Index: clang/unittests/Analysis/CFGBuildResult.h
===
--- clang/unittests/Analysis/CFGBuildResult.h
+++ clang/unittests/Analysis/CFGBuildResult.h
@@ -56,13 +56,15 @@
 TheBuildResult = BuildResult::SawFunctionBody;
 Options.AddImplicitDtors = true;
 if (std::unique_ptr Cfg =
-CFG::buildCFG(nullptr, Body, R

[PATCH] D131385: [clangd] Support for standard type hierarchy

[Utkarsh Saxena via Phabricator via cfe-commits]

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

LG. Thanks!




Comment at: clang-tools-extra/clangd/Protocol.cpp:1228
+Result["parents"] = RP.parents;
+  return std::move(Result);
+}

kadircet wrote:
> usaxena95 wrote:
> > Nit: Allow RVO.
> there's an issue with one of the compilers used by a build-bot which can't 
> find the relevant constructor if we don't have std::move here (hence we've 
> the same pattern across others). we should definitely check at some point to 
> see if that compiler is gone, but i'd rather not do that in the scope of this 
> patch.
Interesting. Makes sense to get to it later.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1713
+  fillSuperTypes(*ParentDecl, TUPath, *ParentSym, RPSet);
+  Item.data.parents->emplace_back(ParentSym->data);
+  Item.parents->emplace_back(std::move(*ParentSym));

nit: std::move.



Comment at: clang-tools-extra/clangd/unittests/TypeHierarchyTests.cpp:780
+  withResolveParents(HasValue(UnorderedElementsAre(
+  withResolveID(Result.front().data.symbolID.str(;
+}

kadircet wrote:
> usaxena95 wrote:
> > It would be clearer if we use `getSymbolID(&findDecl(AST, "Parent1"))` here 
> > and in `SuperTypes`.
> > 
> > 
> not sure what you mean by also doing it in `SuperTypes`
Nvm. I was referring to `SuperTypes` test but it does not check for the symbol 
id.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131385

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

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 452957.
wyt marked 9 inline comments as done.
wyt added a comment.

Address comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,22 @@
   std::vector> &BlockStates;
 };
 
+// Runs dataflow analysis (specified from `MakeAnalysis`) and the `PostVisitCFG`
+// function (if provided) on the body of the function that matches
+// `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks that the
+// results from the analysis are correct.
+//
+// Requirements:
+//
+//  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +120,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [&PostVisitStmt, &Context](
-   const CFGStmt &Stmt,
-   const TypeErasedDataflowAnalysisState &State) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [&PostVisitCFG, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +139,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto &BlockStates = *MaybeBlockStates;
@@ -141,6 +150,33 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[&PostVisitStmt](ASTContext &Context, const CFGElement &Element,
+ const TypeErasedDataflowAnalysisState &State) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +193,9 @@
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [&VerifyResults](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +216,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [&Results,
-   &Annotations](const clang::CFGStmt &Stmt,
+   &Annotations](const clang::CFGElement &Element,
  const TypeErasedDataflowAnalysisState &State) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());
 if (It == Annotations.

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

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 452958.
wyt added a comment.
Herald added a subscriber: mgorny.

Move implementation to CFGMatchSwitch.h, add tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131616

Files:
  clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
@@ -5,12 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
-//
-//  This file defines a simplistic version of Constant Propagation as an example
-//  of a forward, monotonic dataflow analysis. The analysis tracks all
-//  variables in the scope, but lacks escape analysis.
-//
-//===--===//
 
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "TestingSupport.h"
Index: clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
===
--- clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_unittest(ClangAnalysisFlowSensitiveTests
+  CFGMatchSwitchTest.cpp
   ChromiumCheckModelTest.cpp
   DataflowAnalysisContextTest.cpp
   DataflowEnvironmentTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
@@ -0,0 +1,132 @@
+//===- unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace dataflow;
+using namespace ast_matchers;
+
+namespace {
+// State for tracking the number of matches on each kind of CFGElement by the
+// CFGMatchSwitch. Currently only tracks CFGStmt and CFGInitializer.
+struct CFGElementMatches {
+  unsigned int StmtMatches = 0;
+  unsigned int InitializerMatches = 0;
+};
+
+// Returns a match switch that counts the number of local variables
+// (singly-declared) and fields initialized to the integer literal 42.
+auto buildCFGMatchSwitch() {
+  return CFGMatchSwitchBuilder()
+  .CaseOfCFGStmt(
+  declStmt(hasSingleDecl(
+  varDecl(hasInitializer(integerLiteral(equals(42)),
+  [](const DeclStmt *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.StmtMatches++; })
+  .CaseOfCFGInit(
+  cxxCtorInitializer(withInitializer(integerLiteral(equals(42,
+  [](const CXXCtorInitializer *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.InitializerMatches++; })
+  .Build();
+}
+
+// Runs the match switch `MS` on the control flow graph generated from `Code`,
+// tracking information in state `S`. For simplicity, this test utility is
+// restricted to CFGs with a single control flow block (excluding entry and
+// exit blocks) - generated by `Code` with sequential flow (i.e. no branching).
+//
+// Requirements:
+//
+//  `Code` must contain a function named `f`, the body of this function will be
+//  used to generate the CFG.
+template 
+void applySwitchToCode(CFGMatchSwitch &MS, State &S,
+   llvm::StringRef Code) {
+  auto Unit = tooling::buildASTFromCodeWithArgs(Code, {"-Wno-unused-value"});
+  auto &Ctx = Unit->getASTContext();
+  const auto *F = selectFirst(
+  "f", match(functionDecl(isDefinition(), hasName("f")).bind("f"), Ctx));
+
+  CFG::BuildOptions BO;
+  BO.AddInitializers = true;
+
+  auto CFG = CFG::buildCFG(F, F->getBody(), &Ctx, BO);
+  auto CFGBlock = *CFG->getEntry().succ_begin();
+  for (auto &Elt : CF

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang/docs/ReleaseNotes.rst:74
   number of arguments cause an assertion fault.
+- Fix `#57151 `_.
+  ``-Wcomma`` is emitted for void returning functions.

inclyc wrote:
> Do we need an NFC commit to unify the format of issues mentioned here? (Maybe 
> it can be scheduled for a later release). The different ways of citing issues 
> here can be confusing.  `Issue ` `# xxx` in this context.
If we wanted to do something like that, I think we should probably do that as 
part of the release process as a checklist item (CC @tstellar) but I don't have 
strong opinions either. We can certainly do it with NFC fixes as folks notice 
issues if we're fine still having some inconsistency when we release (which I'm 
fine with, personally).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131892

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

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

[Fred Tingaud via Phabricator via cfe-commits]

frederic-tingaud-sonarsource added a comment.

@rnk , does this change answer your points?


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

https://reviews.llvm.org/D130709

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

[clang] 8f555a5 - [cmake] Fix tablegen exports

[Sylvestre Ledru via cfe-commits]

Author: Nikita Popov
Date: 2022-08-16T14:17:23+02:00
New Revision: 8f555a52e033ceec4c4508eb800c9a186acec87f

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

LOG: [cmake] Fix tablegen exports

This fixes some fallout from D131282. Currently, add_tablegen() will add the 
tablegen target to LLVM_EXPORTS and associates the install with LLVMExports. 
For non-standalone builds, this means that you end up with mlir-tblgen and 
clang-tblgen in LLVMExports.

However, these projects should instead be using MLIR_EXPORTS/MLIRTargets and 
CLANG_EXPORTS/ClangTargets. To fix this, add an extra EXPORT option and make 
use of get_target_export_arg() to create the correct export argument.

Reviewed By: ashay-github

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

Added: 


Modified: 
clang/utils/TableGen/CMakeLists.txt
llvm/cmake/modules/TableGen.cmake
llvm/utils/TableGen/CMakeLists.txt
mlir/tools/mlir-pdll/CMakeLists.txt
mlir/tools/mlir-tblgen/CMakeLists.txt

Removed: 




diff  --git a/clang/utils/TableGen/CMakeLists.txt 
b/clang/utils/TableGen/CMakeLists.txt
index d2cb29dd23a30..4666d4d7aa5ce 100644
--- a/clang/utils/TableGen/CMakeLists.txt
+++ b/clang/utils/TableGen/CMakeLists.txt
@@ -1,6 +1,8 @@
 set(LLVM_LINK_COMPONENTS Support)
 
-add_tablegen(clang-tblgen CLANG DESTINATION "${CLANG_TOOLS_INSTALL_DIR}"
+add_tablegen(clang-tblgen CLANG
+  DESTINATION "${CLANG_TOOLS_INSTALL_DIR}"
+  EXPORT Clang
   ASTTableGen.cpp
   ClangASTNodesEmitter.cpp
   ClangASTPropertiesEmitter.cpp

diff  --git a/llvm/cmake/modules/TableGen.cmake 
b/llvm/cmake/modules/TableGen.cmake
index 172621d377485..59f6a1466078f 100644
--- a/llvm/cmake/modules/TableGen.cmake
+++ b/llvm/cmake/modules/TableGen.cmake
@@ -2,6 +2,7 @@
 # while LLVM_TARGET_DEPENDS may contain additional file dependencies.
 # Extra parameters for `tblgen' may come after `ofn' parameter.
 # Adds the name of the generated file to TABLEGEN_OUTPUT.
+include(LLVMDistributionSupport)
 
 function(tablegen project ofn)
   cmake_parse_arguments(ARG "" "" "DEPENDS;EXTRA_INCLUDES" ${ARGN})
@@ -140,7 +141,7 @@ function(add_public_tablegen_target target)
 endfunction()
 
 macro(add_tablegen target project)
-  cmake_parse_arguments(ADD_TABLEGEN "" "DESTINATION" "" ${ARGN})
+  cmake_parse_arguments(ADD_TABLEGEN "" "DESTINATION;EXPORT" "" ${ARGN})
 
   set(${target}_OLD_LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS})
   set(LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS} TableGen)
@@ -190,14 +191,12 @@ macro(add_tablegen target project)
   endif()
 
   if (ADD_TABLEGEN_DESTINATION AND NOT LLVM_INSTALL_TOOLCHAIN_ONLY AND 
LLVM_BUILD_UTILS)
-set(export_to_llvmexports)
-if(${target} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
-NOT LLVM_DISTRIBUTION_COMPONENTS)
-  set(export_to_llvmexports EXPORT LLVMExports)
+set(export_arg)
+if(ADD_TABLEGEN_EXPORT)
+  get_target_export_arg(${target} ${ADD_TABLEGEN_EXPORT} export_arg)
 endif()
-
 install(TARGETS ${target}
-${export_to_llvmexports}
+${export_arg}
 COMPONENT ${target}
 RUNTIME DESTINATION "${ADD_TABLEGEN_DESTINATION}")
 if(NOT LLVM_ENABLE_IDE)
@@ -206,5 +205,8 @@ macro(add_tablegen target project)
COMPONENT ${target})
 endif()
   endif()
-  set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${target})
+  if(ADD_TABLEGEN_EXPORT)
+string(TOUPPER ${ADD_TABLEGEN_EXPORT} export_upper)
+set_property(GLOBAL APPEND PROPERTY ${export_upper}_EXPORTS ${target})
+  endif()
 endmacro()

diff  --git a/llvm/utils/TableGen/CMakeLists.txt 
b/llvm/utils/TableGen/CMakeLists.txt
index bf4d51b8dca44..56035bd0e4e0b 100644
--- a/llvm/utils/TableGen/CMakeLists.txt
+++ b/llvm/utils/TableGen/CMakeLists.txt
@@ -2,7 +2,9 @@ add_subdirectory(GlobalISel)
 
 set(LLVM_LINK_COMPONENTS Support)
 
-add_tablegen(llvm-tblgen LLVM DESTINATION "${LLVM_TOOLS_INSTALL_DIR}"
+add_tablegen(llvm-tblgen LLVM
+  DESTINATION "${LLVM_TOOLS_INSTALL_DIR}"
+  EXPORT LLVM
   AsmMatcherEmitter.cpp
   AsmWriterEmitter.cpp
   AsmWriterInst.cpp

diff  --git a/mlir/tools/mlir-pdll/CMakeLists.txt 
b/mlir/tools/mlir-pdll/CMakeLists.txt
index 6acee39e875cd..67b65d7ad5723 100644
--- a/mlir/tools/mlir-pdll/CMakeLists.txt
+++ b/mlir/tools/mlir-pdll/CMakeLists.txt
@@ -12,7 +12,9 @@ set(LIBS
   MLIRPDLLParser
   )
 
-add_tablegen(mlir-pdll MLIR_PDLL DESTINATION "${MLIR_TOOLS_INSTALL_DIR}"
+add_tablegen(mlir-pdll MLIR_PDLL
+  DESTINATION "${MLIR_TOOLS_INSTALL_DIR}"
+  EXPORT MLIR
   mlir-pdll.cpp
 
   DEPENDS

diff  --git a/mlir/tools/mlir-tblgen/CMakeLists.txt 
b/mlir/tools/mlir-tblgen/CMakeLists.txt
index 85f9207f723b1..afc3ab1853986 100644
--- a/mlir/tools/mlir-tblgen/CMakeLists.txt
+++ b/mlir/tools/mlir-tblgen/CMakeLists.txt
@@ -4,7 +

[PATCH] D131565: [cmake] Fix tablegen exports

[Sylvestre Ledru via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8f555a52e033: [cmake] Fix tablegen exports (authored by 
nikic, committed by sylvestre.ledru).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131565

Files:
  clang/utils/TableGen/CMakeLists.txt
  llvm/cmake/modules/TableGen.cmake
  llvm/utils/TableGen/CMakeLists.txt
  mlir/tools/mlir-pdll/CMakeLists.txt
  mlir/tools/mlir-tblgen/CMakeLists.txt

Index: mlir/tools/mlir-tblgen/CMakeLists.txt
===
--- mlir/tools/mlir-tblgen/CMakeLists.txt
+++ mlir/tools/mlir-tblgen/CMakeLists.txt
@@ -4,7 +4,9 @@
   TableGen
 )
 
-add_tablegen(mlir-tblgen MLIR DESTINATION "${MLIR_TOOLS_INSTALL_DIR}"
+add_tablegen(mlir-tblgen MLIR
+  DESTINATION "${MLIR_TOOLS_INSTALL_DIR}"
+  EXPORT MLIR
   AttrOrTypeDefGen.cpp
   AttrOrTypeFormatGen.cpp
   CodeGenHelpers.cpp
Index: mlir/tools/mlir-pdll/CMakeLists.txt
===
--- mlir/tools/mlir-pdll/CMakeLists.txt
+++ mlir/tools/mlir-pdll/CMakeLists.txt
@@ -12,7 +12,9 @@
   MLIRPDLLParser
   )
 
-add_tablegen(mlir-pdll MLIR_PDLL DESTINATION "${MLIR_TOOLS_INSTALL_DIR}"
+add_tablegen(mlir-pdll MLIR_PDLL
+  DESTINATION "${MLIR_TOOLS_INSTALL_DIR}"
+  EXPORT MLIR
   mlir-pdll.cpp
 
   DEPENDS
Index: llvm/utils/TableGen/CMakeLists.txt
===
--- llvm/utils/TableGen/CMakeLists.txt
+++ llvm/utils/TableGen/CMakeLists.txt
@@ -2,7 +2,9 @@
 
 set(LLVM_LINK_COMPONENTS Support)
 
-add_tablegen(llvm-tblgen LLVM DESTINATION "${LLVM_TOOLS_INSTALL_DIR}"
+add_tablegen(llvm-tblgen LLVM
+  DESTINATION "${LLVM_TOOLS_INSTALL_DIR}"
+  EXPORT LLVM
   AsmMatcherEmitter.cpp
   AsmWriterEmitter.cpp
   AsmWriterInst.cpp
Index: llvm/cmake/modules/TableGen.cmake
===
--- llvm/cmake/modules/TableGen.cmake
+++ llvm/cmake/modules/TableGen.cmake
@@ -2,6 +2,7 @@
 # while LLVM_TARGET_DEPENDS may contain additional file dependencies.
 # Extra parameters for `tblgen' may come after `ofn' parameter.
 # Adds the name of the generated file to TABLEGEN_OUTPUT.
+include(LLVMDistributionSupport)
 
 function(tablegen project ofn)
   cmake_parse_arguments(ARG "" "" "DEPENDS;EXTRA_INCLUDES" ${ARGN})
@@ -140,7 +141,7 @@
 endfunction()
 
 macro(add_tablegen target project)
-  cmake_parse_arguments(ADD_TABLEGEN "" "DESTINATION" "" ${ARGN})
+  cmake_parse_arguments(ADD_TABLEGEN "" "DESTINATION;EXPORT" "" ${ARGN})
 
   set(${target}_OLD_LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS})
   set(LLVM_LINK_COMPONENTS ${LLVM_LINK_COMPONENTS} TableGen)
@@ -190,14 +191,12 @@
   endif()
 
   if (ADD_TABLEGEN_DESTINATION AND NOT LLVM_INSTALL_TOOLCHAIN_ONLY AND LLVM_BUILD_UTILS)
-set(export_to_llvmexports)
-if(${target} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
-NOT LLVM_DISTRIBUTION_COMPONENTS)
-  set(export_to_llvmexports EXPORT LLVMExports)
+set(export_arg)
+if(ADD_TABLEGEN_EXPORT)
+  get_target_export_arg(${target} ${ADD_TABLEGEN_EXPORT} export_arg)
 endif()
-
 install(TARGETS ${target}
-${export_to_llvmexports}
+${export_arg}
 COMPONENT ${target}
 RUNTIME DESTINATION "${ADD_TABLEGEN_DESTINATION}")
 if(NOT LLVM_ENABLE_IDE)
@@ -206,5 +205,8 @@
COMPONENT ${target})
 endif()
   endif()
-  set_property(GLOBAL APPEND PROPERTY LLVM_EXPORTS ${target})
+  if(ADD_TABLEGEN_EXPORT)
+string(TOUPPER ${ADD_TABLEGEN_EXPORT} export_upper)
+set_property(GLOBAL APPEND PROPERTY ${export_upper}_EXPORTS ${target})
+  endif()
 endmacro()
Index: clang/utils/TableGen/CMakeLists.txt
===
--- clang/utils/TableGen/CMakeLists.txt
+++ clang/utils/TableGen/CMakeLists.txt
@@ -1,6 +1,8 @@
 set(LLVM_LINK_COMPONENTS Support)
 
-add_tablegen(clang-tblgen CLANG DESTINATION "${CLANG_TOOLS_INSTALL_DIR}"
+add_tablegen(clang-tblgen CLANG
+  DESTINATION "${CLANG_TOOLS_INSTALL_DIR}"
+  EXPORT Clang
   ASTTableGen.cpp
   ClangASTNodesEmitter.cpp
   ClangASTPropertiesEmitter.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 452964.
wyt added a comment.

Add FIXME.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h

Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -71,14 +71,22 @@
   std::vector> &BlockStates;
 };
 
+// Runs dataflow analysis (specified from `MakeAnalysis`) and the `PostVisitCFG`
+// function (if provided) on the body of the function that matches
+// `TargetFuncMatcher` in code snippet `Code`. `VerifyResults` checks that the
+// results from the analysis are correct.
+//
+// Requirements:
+//
+//  `AnalysisT` contains a type `Lattice`.
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,
 ast_matchers::internal::Matcher TargetFuncMatcher,
 std::function MakeAnalysis,
-std::function
-PostVisitStmt,
+PostVisitCFG,
 std::function VerifyResults, ArrayRef Args,
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   llvm::Annotations AnnotatedCode(Code);
@@ -112,13 +120,14 @@
   Environment Env(DACtx, *F);
   auto Analysis = MakeAnalysis(Context, Env);
 
-  std::function
-  PostVisitStmtClosure = nullptr;
-  if (PostVisitStmt != nullptr) {
-PostVisitStmtClosure = [&PostVisitStmt, &Context](
-   const CFGStmt &Stmt,
-   const TypeErasedDataflowAnalysisState &State) {
-  PostVisitStmt(Context, Stmt, State);
+  std::function
+  PostVisitCFGClosure = nullptr;
+  if (PostVisitCFG) {
+PostVisitCFGClosure = [&PostVisitCFG, &Context](
+  const CFGElement &Element,
+  const TypeErasedDataflowAnalysisState &State) {
+  PostVisitCFG(Context, Element, State);
 };
   }
 
@@ -130,7 +139,7 @@
 
   llvm::Expected>>
   MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
-   PostVisitStmtClosure);
+   PostVisitCFGClosure);
   if (!MaybeBlockStates)
 return MaybeBlockStates.takeError();
   auto &BlockStates = *MaybeBlockStates;
@@ -141,6 +150,33 @@
   return llvm::Error::success();
 }
 
+template 
+llvm::Error checkDataflow(
+llvm::StringRef Code,
+ast_matchers::internal::Matcher TargetFuncMatcher,
+std::function MakeAnalysis,
+std::function
+PostVisitStmt,
+std::function VerifyResults, ArrayRef Args,
+const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+
+  std::function
+  PostVisitCFG = nullptr;
+  if (PostVisitStmt) {
+PostVisitCFG =
+[&PostVisitStmt](ASTContext &Context, const CFGElement &Element,
+ const TypeErasedDataflowAnalysisState &State) {
+  if (auto Stmt = Element.getAs()) {
+PostVisitStmt(Context, *Stmt, State);
+  }
+};
+  }
+  return checkDataflowOnCFG(Code, TargetFuncMatcher, MakeAnalysis, PostVisitCFG,
+VerifyResults, Args, VirtualMappedFiles);
+}
+
 // Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
 // code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
 template 
@@ -157,9 +193,9 @@
 const tooling::FileContentMappings &VirtualMappedFiles = {}) {
   using StateT = DataflowAnalysisState;
 
-  return checkDataflow(
+  return checkDataflowOnCFG(
   Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
-  /*PostVisitStmt=*/nullptr,
+  /*PostVisitCFG=*/nullptr,
   [&VerifyResults](AnalysisData AnalysisData) {
 if (AnalysisData.BlockStates.empty()) {
   VerifyResults({}, AnalysisData.ASTCtx);
@@ -180,9 +216,13 @@
   AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
   AnalysisData.Env, AnalysisData.Analysis,
   [&Results,
-   &Annotations](const clang::CFGStmt &Stmt,
+   &Annotations](const clang::CFGElement &Element,
  const TypeErasedDataflowAnalysisState &State) {
-auto It = Annotations.find(Stmt.getStmt());
+// FIXME: Extend testing annotations to non statement constructs
+auto Stmt = Element.getAs();
+if (!Stmt)
+  return;
+auto It = Annotations.find(Stmt->getStmt());
 if (It == Annotations.end())
   return;
   

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

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 452965.
wyt added a comment.

Propagate change from parent patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131616

Files:
  clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
@@ -5,12 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
-//
-//  This file defines a simplistic version of Constant Propagation as an example
-//  of a forward, monotonic dataflow analysis. The analysis tracks all
-//  variables in the scope, but lacks escape analysis.
-//
-//===--===//
 
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "TestingSupport.h"
Index: clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
===
--- clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_unittest(ClangAnalysisFlowSensitiveTests
+  CFGMatchSwitchTest.cpp
   ChromiumCheckModelTest.cpp
   DataflowAnalysisContextTest.cpp
   DataflowEnvironmentTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
@@ -0,0 +1,132 @@
+//===- unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace dataflow;
+using namespace ast_matchers;
+
+namespace {
+// State for tracking the number of matches on each kind of CFGElement by the
+// CFGMatchSwitch. Currently only tracks CFGStmt and CFGInitializer.
+struct CFGElementMatches {
+  unsigned int StmtMatches = 0;
+  unsigned int InitializerMatches = 0;
+};
+
+// Returns a match switch that counts the number of local variables
+// (singly-declared) and fields initialized to the integer literal 42.
+auto buildCFGMatchSwitch() {
+  return CFGMatchSwitchBuilder()
+  .CaseOfCFGStmt(
+  declStmt(hasSingleDecl(
+  varDecl(hasInitializer(integerLiteral(equals(42)),
+  [](const DeclStmt *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.StmtMatches++; })
+  .CaseOfCFGInit(
+  cxxCtorInitializer(withInitializer(integerLiteral(equals(42,
+  [](const CXXCtorInitializer *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.InitializerMatches++; })
+  .Build();
+}
+
+// Runs the match switch `MS` on the control flow graph generated from `Code`,
+// tracking information in state `S`. For simplicity, this test utility is
+// restricted to CFGs with a single control flow block (excluding entry and
+// exit blocks) - generated by `Code` with sequential flow (i.e. no branching).
+//
+// Requirements:
+//
+//  `Code` must contain a function named `f`, the body of this function will be
+//  used to generate the CFG.
+template 
+void applySwitchToCode(CFGMatchSwitch &MS, State &S,
+   llvm::StringRef Code) {
+  auto Unit = tooling::buildASTFromCodeWithArgs(Code, {"-Wno-unused-value"});
+  auto &Ctx = Unit->getASTContext();
+  const auto *F = selectFirst(
+  "f", match(functionDecl(isDefinition(), hasName("f")).bind("f"), Ctx));
+
+  CFG::BuildOptions BO;
+  BO.AddInitializers = true;
+
+  auto CFG = CFG::buildCFG(F, F->getBody

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

[weiyi via Phabricator via cfe-commits]

wyt added a reviewer: xazax.hun.
wyt added inline comments.
Herald added a subscriber: rnkovacs.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:103
+  virtual void transferCFGElement(const CFGElement *Element, Lattice &L,
+  Environment &Env) {}
+

gribozavr2 wrote:
> Instead of adding virtual function, please continue following the CRTP 
> pattern. Add the expected function declarations in the comment above the 
> class.
> Instead of adding virtual function, please continue following the CRTP 
> pattern. Add the expected function declarations in the comment above the 
> class.

As discussed, moving to a CRTP pattern causes the code to break as they have 
not been implemented by users yet. Added a FIXME to do so after users have been 
migrated to implement transferCFGElement.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

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

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

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 452966.
wyt added a comment.

Fix incorrect change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131616

Files:
  clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h
  clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h
  clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
  clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
  clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/MatchSwitchTest.cpp
@@ -5,12 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===--===//
-//
-//  This file defines a simplistic version of Constant Propagation as an example
-//  of a forward, monotonic dataflow analysis. The analysis tracks all
-//  variables in the scope, but lacks escape analysis.
-//
-//===--===//
 
 #include "clang/Analysis/FlowSensitive/MatchSwitch.h"
 #include "TestingSupport.h"
Index: clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
===
--- clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -4,6 +4,7 @@
   )
 
 add_clang_unittest(ClangAnalysisFlowSensitiveTests
+  CFGMatchSwitchTest.cpp
   ChromiumCheckModelTest.cpp
   DataflowAnalysisContextTest.cpp
   DataflowEnvironmentTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
===
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp
@@ -0,0 +1,132 @@
+//===- unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Analysis/FlowSensitive/CFGMatchSwitch.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/Expr.h"
+#include "clang/AST/Stmt.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/ADT/StringRef.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using namespace clang;
+using namespace dataflow;
+using namespace ast_matchers;
+
+namespace {
+// State for tracking the number of matches on each kind of CFGElement by the
+// CFGMatchSwitch. Currently only tracks CFGStmt and CFGInitializer.
+struct CFGElementMatches {
+  unsigned int StmtMatches = 0;
+  unsigned int InitializerMatches = 0;
+};
+
+// Returns a match switch that counts the number of local variables
+// (singly-declared) and fields initialized to the integer literal 42.
+auto buildCFGMatchSwitch() {
+  return CFGMatchSwitchBuilder()
+  .CaseOfCFGStmt(
+  declStmt(hasSingleDecl(
+  varDecl(hasInitializer(integerLiteral(equals(42)),
+  [](const DeclStmt *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.StmtMatches++; })
+  .CaseOfCFGInit(
+  cxxCtorInitializer(withInitializer(integerLiteral(equals(42,
+  [](const CXXCtorInitializer *, const MatchFinder::MatchResult &,
+ CFGElementMatches &Counter) { Counter.InitializerMatches++; })
+  .Build();
+}
+
+// Runs the match switch `MS` on the control flow graph generated from `Code`,
+// tracking information in state `S`. For simplicity, this test utility is
+// restricted to CFGs with a single control flow block (excluding entry and
+// exit blocks) - generated by `Code` with sequential flow (i.e. no branching).
+//
+// Requirements:
+//
+//  `Code` must contain a function named `f`, the body of this function will be
+//  used to generate the CFG.
+template 
+void applySwitchToCode(CFGMatchSwitch &MS, State &S,
+   llvm::StringRef Code) {
+  auto Unit = tooling::buildASTFromCodeWithArgs(Code, {"-Wno-unused-value"});
+  auto &Ctx = Unit->getASTContext();
+  const auto *F = selectFirst(
+  "f", match(functionDecl(isDefinition(), hasName("f")).bind("f"), Ctx));
+
+  CFG::BuildOptions BO;
+  BO.AddInitializers = true;
+
+  auto CFG = CFG::buildCFG(F, F->getBody(), &Ctx, BO);
+  auto CFGBlock = *CFG->getEntry().succ_begin();
+  for (auto &Elt : CFGBlock->Elements) {
+MS(Elt, Ctx, S);
+  }
+}
+
+TEST(CFGMatc

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

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added a comment.

FWIW, just some things noticed when I examined some of the new warning that 
popped up after this patch:

https://github.com/llvm/llvm-project/issues/53253 mentioned that for example 
gcc complained about this. Although, as shown here 
https://godbolt.org/z/bq34Kexac there are some other differences that clang now 
complains already with -Wall, but that is not the case with gcc (I think one 
need to enable `-Wpedantic`  in gcc to get a warning about signed overflow).

A similar warning (when assigning -1 to an unsigned bitfield) can be given by 
both gcc and clang by using ` -Wsign-conversion` but that is not part of 
`-Wall` either. But maybe that is a totally different thing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

[PATCH] D131891: [clang][dataflow] Debug string for value kinds.

[weiyi via Phabricator via cfe-commits]

wyt updated this revision to Diff 452968.
wyt added a comment.

Use llvm::StringRef for returning static strings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131891

Files:
  clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
  clang/lib/Analysis/FlowSensitive/DebugSupport.cpp


Index: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
===
--- clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -18,6 +18,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatAdapters.h"
@@ -31,7 +32,33 @@
 using llvm::fmt_pad;
 using llvm::formatv;
 
-std::string debugString(Solver::Result::Assignment Assignment) {
+llvm::StringRef debugString(Value::Kind Kind) {
+  switch (Kind) {
+  case Value::Kind::Integer:
+return "Integer";
+  case Value::Kind::Reference:
+return "Reference";
+  case Value::Kind::Pointer:
+return "Pointer";
+  case Value::Kind::Struct:
+return "Struct";
+  case Value::Kind::AtomicBool:
+return "AtomicBool";
+  case Value::Kind::Conjunction:
+return "Conjunction";
+  case Value::Kind::Disjunction:
+return "Disjunction";
+  case Value::Kind::Negation:
+return "Negation";
+  case Value::Kind::Implication:
+return "Implication";
+  case Value::Kind::Biconditional:
+return "Biconditional";
+  }
+  llvm_unreachable("Unhandled value kind");
+}
+
+llvm::StringRef debugString(Solver::Result::Assignment Assignment) {
   switch (Assignment) {
   case Solver::Result::Assignment::AssignedFalse:
 return "False";
@@ -41,7 +68,7 @@
   llvm_unreachable("Booleans can only be assigned true/false");
 }
 
-std::string debugString(Solver::Result::Status Status) {
+llvm::StringRef debugString(Solver::Result::Status Status) {
   switch (Status) {
   case Solver::Result::Status::Satisfiable:
 return "Satisfiable";
Index: clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
===
--- clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
+++ clang/include/clang/Analysis/FlowSensitive/DebugSupport.h
@@ -20,15 +20,19 @@
 #include "clang/Analysis/FlowSensitive/Solver.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/StringRef.h"
 
 namespace clang {
 namespace dataflow {
 
+/// Returns a string representation of a value kind.
+llvm::StringRef debugString(Value::Kind Kind);
+
 /// Returns a string representation of a boolean assignment to true or false.
-std::string debugString(Solver::Result::Assignment Assignment);
+llvm::StringRef debugString(Solver::Result::Assignment Assignment);
 
 /// Returns a string representation of the result status of a SAT check.
-std::string debugString(Solver::Result::Status Status);
+llvm::StringRef debugString(Solver::Result::Status Status);
 
 /// Returns a string representation for the boolean value `B`.
 ///


Index: clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
===
--- clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
+++ clang/lib/Analysis/FlowSensitive/DebugSupport.cpp
@@ -18,6 +18,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatAdapters.h"
@@ -31,7 +32,33 @@
 using llvm::fmt_pad;
 using llvm::formatv;
 
-std::string debugString(Solver::Result::Assignment Assignment) {
+llvm::StringRef debugString(Value::Kind Kind) {
+  switch (Kind) {
+  case Value::Kind::Integer:
+return "Integer";
+  case Value::Kind::Reference:
+return "Reference";
+  case Value::Kind::Pointer:
+return "Pointer";
+  case Value::Kind::Struct:
+return "Struct";
+  case Value::Kind::AtomicBool:
+return "AtomicBool";
+  case Value::Kind::Conjunction:
+return "Conjunction";
+  case Value::Kind::Disjunction:
+return "Disjunction";
+  case Value::Kind::Negation:
+return "Negation";
+  case Value::Kind::Implication:
+return "Implication";
+  case Value::Kind::Biconditional:
+return "Biconditional";
+  }
+  llvm_unreachable("Unhandled value kind");
+}
+
+llvm::StringRef debugString(Solver::Result::Assignment Assignment) {
   switch (Assignment) {
   case Solver::Result::Assignment::AssignedFalse:
 return "False";
@@ -41,7 +68,7 @@
   llvm_unreachable("Booleans can only be assigned true/false");
 }
 
-std::string debugString(Solver::Result::Status Status) {
+llvm::StringRef debugString(Solver::Result::Status Status) {
   switch (Status) {

[clang] ccbc22c - [Sema] fix false -Wcomma being emitted from void returning functions

[YingChi Long via cfe-commits]

Author: YingChi Long
Date: 2022-08-16T20:44:38+08:00
New Revision: ccbc22cd8976c285d76e9f66dd5cac2fe908d084

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

LOG: [Sema] fix false -Wcomma being emitted from void returning functions

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

Reviewed By: aaron.ballman

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaExpr.cpp
clang/test/SemaCXX/warn-comma-operator.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index ded0b39c27ea5..1da6015ad1610 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -73,7 +73,8 @@ Bug Fixes
   number of arguments cause an assertion fault.
 - Fix multi-level pack expansion of undeclared function parameters.
   This fixes `Issue 56094 
`_.
-
+- Fix `#57151 `_.
+  ``-Wcomma`` is emitted for void returning functions.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 03f5ec9848e1f..a2a2e1ed85fc3 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -13957,8 +13957,10 @@ QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, 
ExprResult &RHS,
   return getLangOpts().CPlusPlus ? LHSType : 
LHSType.getAtomicUnqualifiedType();
 }
 
-// Only ignore explicit casts to void.
-static bool IgnoreCommaOperand(const Expr *E) {
+// Scenarios to ignore if expression E is:
+// 1. an explicit cast expression into void
+// 2. a function call expression that returns void
+static bool IgnoreCommaOperand(const Expr *E, const ASTContext &Context) {
   E = E->IgnoreParens();
 
   if (const CastExpr *CE = dyn_cast(E)) {
@@ -13973,6 +13975,8 @@ static bool IgnoreCommaOperand(const Expr *E) {
 }
   }
 
+  if (const auto *CE = dyn_cast(E))
+return CE->getCallReturnType(Context)->isVoidType();
   return false;
 }
 
@@ -14014,7 +14018,7 @@ void Sema::DiagnoseCommaOperator(const Expr *LHS, 
SourceLocation Loc) {
   }
 
   // Only allow some expressions on LHS to not warn.
-  if (IgnoreCommaOperand(LHS))
+  if (IgnoreCommaOperand(LHS, Context))
 return;
 
   Diag(Loc, diag::warn_comma_operator);

diff  --git a/clang/test/SemaCXX/warn-comma-operator.cpp 
b/clang/test/SemaCXX/warn-comma-operator.cpp
index 635ce01b1c9cf..75148cb416fdb 100644
--- a/clang/test/SemaCXX/warn-comma-operator.cpp
+++ b/clang/test/SemaCXX/warn-comma-operator.cpp
@@ -140,6 +140,27 @@ void test_nested_fixits(void) {
   // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
 }
 
+
+void void_func();
+int int_func() { return 0; }
+
+void void_function_comma(){
+  void_func(), int_func(); // expected no -Wcomma because of the returning 
type `void` 
+  // Reported by https://github.com/llvm/llvm-project/issues/57151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+
+typedef void Void;
+Void typedef_func();
+
+void whatever() {
+  // We don't get confused about type aliases.
+  typedef_func(), int_func();
+  // Even function pointers don't confuse us.
+  void (*fp)() = void_func;
+  fp(), int_func();
+}
+
 #ifdef __cplusplus
 class S2 {
 public:
@@ -296,4 +317,23 @@ void test_dependent_cast() {
   (void)T{}, 0;
   static_cast(T{}), 0;
 }
+
+namespace {
+
+// issue #57151
+
+struct S {
+  void mem() {}
+};
+
+void whatever() {
+  struct S s;
+  // Member function calls also work as expected.
+  s.mem(), int_func();
+  // As do lambda calls.
+  []() { return; }(), int_func();
+}
+
+} // namespace
+
 #endif  // ifdef __cplusplus



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

[PATCH] D131892: [Sema] fix false -Wcomma being emitted from void returning functions

[YingChi Long via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGccbc22cd8976: [Sema] fix false -Wcomma being emitted from 
void returning functions (authored by inclyc).

Changed prior to commit:
  https://reviews.llvm.org/D131892?vs=452767&id=452970#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131892

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaExpr.cpp
  clang/test/SemaCXX/warn-comma-operator.cpp

Index: clang/test/SemaCXX/warn-comma-operator.cpp
===
--- clang/test/SemaCXX/warn-comma-operator.cpp
+++ clang/test/SemaCXX/warn-comma-operator.cpp
@@ -140,6 +140,27 @@
   // CHECK: fix-it:{{.*}}:{[[@LINE-8]]:46-[[@LINE-8]]:46}:")"
 }
 
+
+void void_func();
+int int_func() { return 0; }
+
+void void_function_comma(){
+  void_func(), int_func(); // expected no -Wcomma because of the returning type `void` 
+  // Reported by https://github.com/llvm/llvm-project/issues/57151
+  // Descriptions about -Wcomma: https://reviews.llvm.org/D3976
+}
+
+typedef void Void;
+Void typedef_func();
+
+void whatever() {
+  // We don't get confused about type aliases.
+  typedef_func(), int_func();
+  // Even function pointers don't confuse us.
+  void (*fp)() = void_func;
+  fp(), int_func();
+}
+
 #ifdef __cplusplus
 class S2 {
 public:
@@ -296,4 +317,23 @@
   (void)T{}, 0;
   static_cast(T{}), 0;
 }
+
+namespace {
+
+// issue #57151
+
+struct S {
+  void mem() {}
+};
+
+void whatever() {
+  struct S s;
+  // Member function calls also work as expected.
+  s.mem(), int_func();
+  // As do lambda calls.
+  []() { return; }(), int_func();
+}
+
+} // namespace
+
 #endif  // ifdef __cplusplus
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -13957,8 +13957,10 @@
   return getLangOpts().CPlusPlus ? LHSType : LHSType.getAtomicUnqualifiedType();
 }
 
-// Only ignore explicit casts to void.
-static bool IgnoreCommaOperand(const Expr *E) {
+// Scenarios to ignore if expression E is:
+// 1. an explicit cast expression into void
+// 2. a function call expression that returns void
+static bool IgnoreCommaOperand(const Expr *E, const ASTContext &Context) {
   E = E->IgnoreParens();
 
   if (const CastExpr *CE = dyn_cast(E)) {
@@ -13973,6 +13975,8 @@
 }
   }
 
+  if (const auto *CE = dyn_cast(E))
+return CE->getCallReturnType(Context)->isVoidType();
   return false;
 }
 
@@ -14014,7 +14018,7 @@
   }
 
   // Only allow some expressions on LHS to not warn.
-  if (IgnoreCommaOperand(LHS))
+  if (IgnoreCommaOperand(LHS, Context))
 return;
 
   Diag(Loc, diag::warn_comma_operator);
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -73,7 +73,8 @@
   number of arguments cause an assertion fault.
 - Fix multi-level pack expansion of undeclared function parameters.
   This fixes `Issue 56094 `_.
-
+- Fix `#57151 `_.
+  ``-Wcomma`` is emitted for void returning functions.
 
 Improvements to Clang's diagnostics
 ^^^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131274: [clang][Darwin] Re-apply "Always set the default C++ Standard Library to libc++"

[Louis Dionne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG65d83ba34378: [clang][Darwin] Re-apply "Always set the 
default C++ Standard Library to libc++" (authored by ldionne).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131274

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/Xarch.c
  clang/test/Driver/apple-kext-mkernel.c
  clang/test/Driver/arc.c
  clang/test/Driver/bindings.c
  clang/test/Driver/cc-log-diagnostics.c
  clang/test/Driver/cpp-precomp.c
  clang/test/Driver/darwin-debug-flags.c
  clang/test/Driver/darwin-dsymutil.c
  clang/test/Driver/darwin-iphone-defaults.m
  clang/test/Driver/darwin-stdlib.cpp
  clang/test/Driver/darwin-verify-debug.c
  clang/test/Driver/diagnostics.c
  clang/test/Driver/exceptions.m
  clang/test/Driver/redundant-args.c
  clang/test/Headers/float-darwin.c
  clang/test/Headers/tgmath-darwin.c
  clang/test/PCH/reloc.c
  clang/test/lit.cfg.py

Index: clang/test/lit.cfg.py
===
--- clang/test/lit.cfg.py
+++ clang/test/lit.cfg.py
@@ -134,7 +134,7 @@
 # Set available features we allow tests to conditionalize on.
 #
 if config.clang_default_cxx_stdlib != '':
-config.available_features.add('default-cxx-stdlib-set')
+config.available_features.add('default-cxx-stdlib={}'.format(config.clang_default_cxx_stdlib))
 
 # As of 2011.08, crash-recovery tests still do not pass on FreeBSD.
 if platform.system() not in ['FreeBSD']:
Index: clang/test/PCH/reloc.c
===
--- clang/test/PCH/reloc.c
+++ clang/test/PCH/reloc.c
@@ -1,8 +1,8 @@
-// RUN: %clang -target x86_64-apple-darwin10 --relocatable-pch -o %t \
+// RUN: %clang -target x86_64-apple-darwin11 --relocatable-pch -o %t \
 // RUN:   -isysroot %S/Inputs/libroot %S/Inputs/libroot/usr/include/reloc.h
-// RUN: %clang -target x86_64-apple-darwin10 -fsyntax-only \
+// RUN: %clang -target x86_64-apple-darwin11 -fsyntax-only \
 // RUN:   -include-pch %t -isysroot %S/Inputs/libroot %s -Xclang -verify
-// RUN: not %clang -target x86_64-apple-darwin10 -include-pch %t %s
+// RUN: not %clang -target x86_64-apple-darwin11 -include-pch %t %s
 // REQUIRES: x86-registered-target
 
 #include 
Index: clang/test/Headers/tgmath-darwin.c
===
--- clang/test/Headers/tgmath-darwin.c
+++ clang/test/Headers/tgmath-darwin.c
@@ -1,5 +1,5 @@
 // REQUIRES: system-darwin
-// RUN: %clang -target x86_64-apple-darwin10 -fsyntax-only -std=c11 -isysroot %S/Inputs %s
+// RUN: %clang -target x86_64-apple-darwin11 -fsyntax-only -std=c11 -isysroot %S/Inputs %s
 #include 
 
 // Test the #include_next of tgmath.h works on Darwin.
Index: clang/test/Headers/float-darwin.c
===
--- clang/test/Headers/float-darwin.c
+++ clang/test/Headers/float-darwin.c
@@ -1,5 +1,5 @@
 // REQUIRES: system-darwin
-// RUN: %clang -target x86_64-apple-darwin10 -fsyntax-only -std=c11 -isysroot %S/Inputs %s
+// RUN: %clang -target x86_64-apple-darwin11 -fsyntax-only -std=c11 -isysroot %S/Inputs %s
 #include 
 
 // Test the #include_next on float.h works on Darwin.
Index: clang/test/Driver/redundant-args.c
===
--- clang/test/Driver/redundant-args.c
+++ clang/test/Driver/redundant-args.c
@@ -1,2 +1,2 @@
-// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN: %clang -target x86_64-apple-darwin11 \
 // RUN:   -Werror -x c -x c -fsyntax-only %s
Index: clang/test/Driver/exceptions.m
===
--- clang/test/Driver/exceptions.m
+++ clang/test/Driver/exceptions.m
@@ -1,4 +1,4 @@
-// RUN: %clang -target x86_64-apple-darwin9 \
+// RUN: %clang -target x86_64-apple-darwin11 \
 // RUN:   -fsyntax-only -fno-exceptions %s
 
 void f1(void) {
Index: clang/test/Driver/diagnostics.c
===
--- clang/test/Driver/diagnostics.c
+++ clang/test/Driver/diagnostics.c
@@ -6,37 +6,37 @@
 // diagnostics when only compiling for all targets.
 
 // This is normally a non-fatal warning:
-// RUN: %clang --target=x86_64-apple-darwin10 \
+// RUN: %clang --target=x86_64-apple-darwin11 \
 // RUN:   -fsyntax-only -lfoo %s 2>&1 | FileCheck %s
 
 // Either with a specific -Werror=unused.. or a blanket -Werror, this
 // causes the command to fail.
-// RUN: not %clang --target=x86_64-apple-darwin10 \
+// RUN: not %clang --target=x86_64-apple-darwin11 \
 // RUN:   -fsyntax-only -lfoo \
 // RUN:   -Werror=unused-command-line-argument %s 2>&1 | FileCheck %s
 
-// RUN: not %clang --target=x86_64-apple-darwin10 \
+// RUN: not %clang --target=x86_64-apple-darwin11 \
 // RUN:   -fsyntax-only -lfoo -Werror %s 2>&1 | FileCheck %s
 
 // With a specific -Wno-..., no diagnostic s

[clang] 65d83ba - [clang][Darwin] Re-apply "Always set the default C++ Standard Library to libc++"

[Louis Dionne via cfe-commits]

Author: Louis Dionne
Date: 2022-08-16T09:27:18-04:00
New Revision: 65d83ba34378b8e740c5203fe46a9c50d2aeb862

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

LOG: [clang][Darwin] Re-apply "Always set the default C++ Standard Library to 
libc++"

Newer SDKs don't even provide libstdc++ headers, so it's effectively
never valid to build for libstdc++ unless the user explicitly asks
for it (in which case they will need to provide include paths and more).

This is a re-application of c5ccb78ade81 which had been reverted in
33171df9cc7f because it broke the Fuchsia CI bots. The issue was that
the test was XPASSing because it didn't fail anymore when the
CLANG_DEFAULT_CXX_LIB was set to libc++, which seems to be done for
Fuchsia. Instead, the test only fails if CLANG_DEFAULT_CXX_LIB is
set to libstdc++.

As a fly-by fix, also adjust the triple used by various tests to
something that is supported. Those tests were shown to fail on
internal bots.

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

Added: 


Modified: 
clang/lib/Driver/ToolChains/Darwin.cpp
clang/test/Driver/Xarch.c
clang/test/Driver/apple-kext-mkernel.c
clang/test/Driver/arc.c
clang/test/Driver/bindings.c
clang/test/Driver/cc-log-diagnostics.c
clang/test/Driver/cpp-precomp.c
clang/test/Driver/darwin-debug-flags.c
clang/test/Driver/darwin-dsymutil.c
clang/test/Driver/darwin-iphone-defaults.m
clang/test/Driver/darwin-stdlib.cpp
clang/test/Driver/darwin-verify-debug.c
clang/test/Driver/diagnostics.c
clang/test/Driver/exceptions.m
clang/test/Driver/redundant-args.c
clang/test/Headers/float-darwin.c
clang/test/Headers/tgmath-darwin.c
clang/test/PCH/reloc.c
clang/test/lit.cfg.py

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Darwin.cpp 
b/clang/lib/Driver/ToolChains/Darwin.cpp
index bada811daadfe..7e3fc625d8c85 100644
--- a/clang/lib/Driver/ToolChains/Darwin.cpp
+++ b/clang/lib/Driver/ToolChains/Darwin.cpp
@@ -896,12 +896,7 @@ types::ID MachO::LookupTypeForExtension(StringRef Ext) 
const {
 bool MachO::HasNativeLLVMSupport() const { return true; }
 
 ToolChain::CXXStdlibType Darwin::GetDefaultCXXStdlibType() const {
-  // Use libstdc++ on old targets (OSX < 10.9 and iOS < 7)
-  if ((isTargetMacOSBased() && isMacosxVersionLT(10, 9)) ||
-  (isTargetIOSBased() && isIPhoneOSVersionLT(7, 0)))
-return ToolChain::CST_Libstdcxx;
-
-  // On all other targets, use libc++
+  // Always use libc++ by default
   return ToolChain::CST_Libcxx;
 }
 

diff  --git a/clang/test/Driver/Xarch.c b/clang/test/Driver/Xarch.c
index ae0816f2c8f44..f7693fb689d58 100644
--- a/clang/test/Driver/Xarch.c
+++ b/clang/test/Driver/Xarch.c
@@ -1,12 +1,12 @@
-// RUN: %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -O3 %s -S -### 2>&1 
| FileCheck -check-prefix=O3ONCE %s
+// RUN: %clang -target i386-apple-darwin11 -m32 -Xarch_i386 -O3 %s -S -### 
2>&1 | FileCheck -check-prefix=O3ONCE %s
 // O3ONCE: "-O3"
 // O3ONCE-NOT: "-O3"
 
-// RUN: %clang -target i386-apple-darwin9 -m64 -Xarch_i386 -O3 %s -S -### 2>&1 
| FileCheck -check-prefix=O3NONE %s
+// RUN: %clang -target i386-apple-darwin11 -m64 -Xarch_i386 -O3 %s -S -### 
2>&1 | FileCheck -check-prefix=O3NONE %s
 // O3NONE-NOT: "-O3"
 // O3NONE: argument unused during compilation: '-Xarch_i386 -O3'
 
-// RUN: not %clang -target i386-apple-darwin9 -m32 -Xarch_i386 -o -Xarch_i386 
-S %s -S -Xarch_i386 -o 2>&1 | FileCheck -check-prefix=INVALID %s
+// RUN: not %clang -target i386-apple-darwin11 -m32 -Xarch_i386 -o -Xarch_i386 
-S %s -S -Xarch_i386 -o 2>&1 | FileCheck -check-prefix=INVALID %s
 // INVALID: error: invalid Xarch argument: '-Xarch_i386 -o'
 // INVALID: error: invalid Xarch argument: '-Xarch_i386 -S'
 // INVALID: error: invalid Xarch argument: '-Xarch_i386 -o'

diff  --git a/clang/test/Driver/apple-kext-mkernel.c 
b/clang/test/Driver/apple-kext-mkernel.c
index f056ec898d4db..ac476e9680258 100644
--- a/clang/test/Driver/apple-kext-mkernel.c
+++ b/clang/test/Driver/apple-kext-mkernel.c
@@ -1,20 +1,20 @@
-// RUN: %clang -target x86_64-apple-darwin10 -mkernel -### -fsyntax-only %s 
2>&1 | FileCheck --check-prefix=CHECK-X86 %s
-// RUN: %clang -target x86_64-apple-darwin10 -mkernel -### -fsyntax-only 
-fbuiltin -fno-builtin -fcommon -fno-common %s 2>&1 | FileCheck 
--check-prefix=CHECK-X86 %s
+// RUN: %clang -target x86_64-apple-darwin11 -mkernel -### -fsyntax-only %s 
2>&1 | FileCheck --check-prefix=CHECK-X86 %s
+// RUN: %clang -target x86_64-apple-darwin11 -mkernel -### -fsyntax-only 
-fbuiltin -fno-builtin -fcommon -fno-common %s 2>&1 | FileCheck 
--check-prefix=CHECK-X86 %s
 
 // CHECK-X86: "-disable-red-zone"
 // CHECK-X86: "-fno-builtin"
 // CHECK-X86: "-fno-rtti"
 // CHECK-X86-NOT: "-fcommon"
 
-// RUN: %clang -

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This looks correct per my reading of [basic.start.dynamic], but is this an ABI 
breaking change that we may want to use ABI versioning for in case someone is 
relying on the old order for some reason?

Also, the change should have a release note for the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127233

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

[PATCH] D130586: [cmake] Use `CMAKE_INSTALL_LIBDIR` too

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne accepted this revision.
sebastian-ne added a comment.

Looks good to me, thanks!




Comment at: 
utils/bazel/llvm-project-overlay/clang/include/clang/Config/config.h:70
 
 /* Multilib suffix for libdir. */
+#define CLANG_INSTALL_LIBDIR_BASENAME "lib"

This comment is outdated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130586

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

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:13547-13551
+  if (auto ValD = Info.EvaluatingDecl.dyn_cast()) {
+const VarDecl *VD = dyn_cast_or_null(ValD);
+if (VD && !VD->isConstexpr())
+  NotConstexprVar = true;
+  }

shafik wrote:
> shafik wrote:
> > aaron.ballman wrote:
> > > This seems to be equivalent unless I'm misunderstanding something about 
> > > the member dyn_cast.
> > I think the problem is that `PointerUnion` requires that it be one of the 
> > static types it was defined with and in this case that is `const ValueDecl 
> > *, const Expr *` but maybe I am missing something.
> It looks like the big difference is that in `doCastIfPossible(...)` we end up 
> calling `Self::isPossible(f)` which ends up in `PointerUnion::isPossible...)` 
> which uses `FirstIndexOfType` and if the template arguments don't contain the 
> type then it fail.
Whelp, TIL!

Then I guess I'd go with:

`if (const auto *VD = 
dyn_cast_or_null(Info.EvaluatingDecl.dyn_cast))`


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

https://reviews.llvm.org/D131874

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

[PATCH] D130510: Missing tautological compare warnings due to unary operators

[Muhammad Usman Shahid via Phabricator via cfe-commits]

Codesbyusman updated this revision to Diff 452987.
Codesbyusman added a comment.

updated also the test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-bitwise-compare.cpp
  clang/test/SemaCXX/warn-unreachable.cpp

Index: clang/test/SemaCXX/warn-unreachable.cpp
===
--- clang/test/SemaCXX/warn-unreachable.cpp
+++ clang/test/SemaCXX/warn-unreachable.cpp
@@ -396,16 +396,18 @@
   if (y == -1 && y != -1)  // expected-note {{silence}}
 calledFun();// expected-warning {{will never be executed}}
 
-  // TODO: Extend warning to the following code:
-  if (x < -1)
-calledFun();
-  if (x == -1)
-calledFun();
+  if (x == -1)   // expected-note {{silence}}
+calledFun(); // expected-warning {{will never be executed}}
 
-  if (x != -1)
+  if (x != -1)   // expected-note {{silence}}
 calledFun();
   else
+calledFun(); // expected-warning {{will never be executed}}
+
+  // TODO: Extend warning to the following code:
+  if (x < -1)
 calledFun();
+ 
   if (-1 > x)
 calledFun();
   else
Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -11,3 +11,15 @@
   bool b4 = !!(x | 5);
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
 }
+
+template   // silence
+void foo(int x) {
+bool b1 = (x & sizeof(T)) == 8;
+bool b2 = (x & I) == 8;
+bool b3 = (x & 4) == 8; // expected-warning {{bitwise comparison always evaluates to false}}
+}
+
+void run(int x) {
+foo<4, int>(8); // expected-note {{in instantiation of function template specialization 'foo<4, int>' requested here}}
+}
+   
\ No newline at end of file
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
 #define mydefine 2
+#define mydefine2 -2
 
 enum {
   ZERO,
@@ -11,29 +12,66 @@
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((-8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & -8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((x & 8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((2 & x) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((x & -8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-2 & x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+ 
   if ((x | 4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x | 3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+
+  if ((x | -4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x | -3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  
   if ((x & 0x15) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & 0xFFEB) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((0x23 | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((0xFFDD | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
 
   if (!!((8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if (!!((-8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   int y = ((8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((-8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
+  y = ((-3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
 
   if ((x & 8) == 8) {}
   if ((x & 8) != 8) {}
   if ((x | 4) == 4) {}
   if ((x | 4) != 4) {}
 
+  if ((-2 & x) != 4) {}
+  if ((x & -8) == -8) {}
+  if ((x & -8) != -8) {}
+  if ((x | -4) == -4) {}
+  if ((x | -4

[PATCH] D130510: Missing tautological compare warnings due to unary operators

[Muhammad Usman Shahid via Phabricator via cfe-commits]

Codesbyusman updated this revision to Diff 452988.
Codesbyusman added a comment.

updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-bitwise-compare.cpp
  clang/test/SemaCXX/warn-unreachable.cpp

Index: clang/test/SemaCXX/warn-unreachable.cpp
===
--- clang/test/SemaCXX/warn-unreachable.cpp
+++ clang/test/SemaCXX/warn-unreachable.cpp
@@ -396,16 +396,18 @@
   if (y == -1 && y != -1)  // expected-note {{silence}}
 calledFun();// expected-warning {{will never be executed}}
 
-  // TODO: Extend warning to the following code:
-  if (x < -1)
-calledFun();
-  if (x == -1)
-calledFun();
+  if (x == -1)   // expected-note {{silence}}
+calledFun(); // expected-warning {{will never be executed}}
 
-  if (x != -1)
+  if (x != -1)   // expected-note {{silence}}
 calledFun();
   else
+calledFun(); // expected-warning {{will never be executed}}
+
+  // TODO: Extend warning to the following code:
+  if (x < -1)
 calledFun();
+ 
   if (-1 > x)
 calledFun();
   else
Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -11,3 +11,15 @@
   bool b4 = !!(x | 5);
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
 }
+
+template   // silence
+void foo(int x) {
+bool b1 = (x & sizeof(T)) == 8;
+bool b2 = (x & I) == 8;
+bool b3 = (x & 4) == 8; // expected-warning {{bitwise comparison always evaluates to false}}
+}
+
+void run(int x) {
+foo<4, int>(8); // expected-note {{in instantiation of function template specialization 'foo<4, int>' requested here}}
+}
+
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
 #define mydefine 2
+#define mydefine2 -2
 
 enum {
   ZERO,
@@ -11,29 +12,66 @@
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((-8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & -8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((x & 8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((2 & x) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((x & -8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-2 & x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+ 
   if ((x | 4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x | 3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+
+  if ((x | -4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x | -3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  
   if ((x & 0x15) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & 0xFFEB) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((0x23 | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((0xFFDD | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
 
   if (!!((8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if (!!((-8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   int y = ((8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((-8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
+  y = ((-3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
 
   if ((x & 8) == 8) {}
   if ((x & 8) != 8) {}
   if ((x | 4) == 4) {}
   if ((x | 4) != 4) {}
 
+  if ((-2 & x) != 4) {}
+  if ((x & -8) == -8) {}
+  if ((x & -8) != -8) {}
+  if ((x | -4) == -4) {}
+  if ((x | -4) != -4) {}
+
   if ((x & 9) == 8) {}
   if ((x & 9

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

[Paul Scoropan via Phabricator via cfe-commits]

pscoro updated this revision to Diff 452990.
pscoro added a comment.

reverted previous fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-stackprotect.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll

Index: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
@@ -0,0 +1,206 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-LINUX-LE
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-LINUX-LE
+
+; RUN: llc -verify-machineinstrs -O0 -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX-FAST
+; RUN: llc -verify-machineinstrs -O0 -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-FAST
+
+attributes #1 = { nounwind }
+declare void @llvm.ppc.kill.canary()
+define dso_local void @test_kill_canary() #1 {
+; CHECK-AIX-LABEL: test_kill_canary:
+; CHECK-AIX:   # %bb.0: # %entry
+; CHECK-AIX-NEXT:blr
+;
+; CHECK-LABEL: test_kill_canary:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:blr
+;
+; CHECK-AIX64-LABEL: test_kill_canary:
+; CHECK-AIX64:   # %bb.0: # %entry
+; CHECK-AIX64-NEXT:blr
+;
+; CHECK-LINUX-LE-LABEL: test_kill_canary:
+; CHECK-LINUX-LE:   # %bb.0: # %entry
+; CHECK-LINUX-LE-NEXT:blr
+;
+; CHECK-AIX-FAST-LABEL: test_kill_canary:
+; CHECK-AIX-FAST:   # %bb.0: # %entry
+; CHECK-AIX-FAST-NEXT:blr
+;
+; CHECK-FAST-LABEL: test_kill_canary:
+; CHECK-FAST:   # %bb.0: # %entry
+; CHECK-FAST-NEXT:blr
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
+
+attributes #0 = { sspreq }
+; Function Attrs: sspreq
+define dso_local void @test_kill_canary_ssp() #0 #1 {
+; CHECK-AIX-LABEL: test_kill_canary_ssp:
+; CHECK-AIX:   # %bb.0: # %entry
+; CHECK-AIX-NEXT:mflr r0
+; CHECK-AIX-NEXT:stw r0, 8(r1)
+; CHECK-AIX-NEXT:stwu r1, -64(r1)
+; CHECK-AIX-NEXT:lwz r3, L..C0(r2) # @__ssp_canary_word
+; CHECK-AIX-NEXT:lwz r4, 0(r3)
+; CHECK-AIX-NEXT:stw r4, 60(r1)
+; CHECK-AIX-NEXT:lwz r4, 0(r3)
+; CHECK-AIX-NEXT:not r4, r4
+; CHECK-AIX-NEXT:stw r4, 60(r1)
+; CHECK-AIX-NEXT:lwz r3, 0(r3)
+; CHECK-AIX-NEXT:lwz r4, 60(r1)
+; CHECK-AIX-NEXT:cmplw r3, r4
+; CHECK-AIX-NEXT:bne cr0, L..BB1_2
+; CHECK-AIX-NEXT:  # %bb.1: # %entry
+; CHECK-AIX-NEXT:addi r1, r1, 64
+; CHECK-AIX-NEXT:lwz r0, 8(r1)
+; CHECK-AIX-NEXT:mtlr r0
+; CHECK-AIX-NEXT:blr
+; CHECK-AIX-NEXT:  L..BB1_2: # %entry
+; CHECK-AIX-NEXT:bl .__stack_chk_fail[PR]
+; CHECK-AIX-NEXT:nop
+;
+; CHECK-LABEL: test_kill_canary_ssp:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mflr r0
+; CHECK-NEXT:std r0, 16(r1)
+; CHECK-NEXT:stdu r1, -128(r1)
+; CHECK-NEXT:ld r3, -28688(r13)
+; CHECK-NEXT:std r3, 120(r1)
+; CHECK-NEXT:ld r3, -28688(r13)
+; CHECK-NEXT:not r3, r3
+; CHECK-NEXT:std r3, 120(r1)
+; CHECK-NEXT:ld r3, 120(r1)
+; CHECK-NEXT:ld r4, -28688(r13)
+; CHECK-NEXT:cmpld r4, r3
+; CHECK-NEXT:bne cr0, .LBB1_2
+; CHECK-NEXT:  # %bb.1: # %entry
+; CHECK-NEXT:addi r1, r1, 128
+; CHECK-NEXT:ld r0, 16(r1)
+; CHECK-NEXT:mtlr r0
+; CH

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp:236-238
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
   sum += sizeof(PMyStruct);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate

njames93 wrote:
> mizvekov wrote:
> > Is this change really desirable, or should we put a FIXME here?
> Not warning on these cases seems like a pretty big red flag, especially the 
> `MyStruct *`.
Thank you for your comment!  I'm not sure I fully agree that this is a red 
flag.  I'm inclined to think that whether or not there should be a warning on 
`MyStruct *` or `PMyStruct` is a pretty subjective.  These are both very common 
idioms, and are meaningful.  I do appreciate the perspective that `MyStruct *` 
is just one character different from `MyStruct`, and as such, it might be a 
typo to ask for sizeof the pointer, when you really wanted sizeof the struct.  
But the counter-argument (accidentally asking for sizeof the struct when you 
really wanted the pointer size) is just as valid-- and the checker obviously 
cannot warn in that case.   

I am perfectly fine with kicking the can down the road a bit by replacing the 
discarded `// CHECK-MESSAGES` directive with a `// FIXME`, as @mizvekov 
suggested.  I expect that when someone circles back to really deeply reconsider 
the semantics of this checker, there will be a number of changes (other 
existing warnings dropped, warnings for new and missed cases added, much better 
sync between C and C++, as well as intentional consideration of things like 
__typeof__ (in it's various forms) and decltype, rather than the haphazard 
coarse-grain matching that seems to be going on now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131926

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

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3140
+  return CGF.Builder.CreateFPExt(result, ConvertType(E->getType()));
+  }
+  return result;

rjmccall wrote:
> Please extract this block out as:
> 
> ```
> llvm::Value *EmitPromotedValue(llvm::Value *result, QualType PromotionType);
> ```
These changes you are proposing is when the argument of the unary __imag / 
__real is of type _Complex Float16. I would think that this new method 
EmitPromotedValue would be replacing the equivalent code in 
ComplexEmitter::EmitPromoted instead,  not in the scalar emitter, right?


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

https://reviews.llvm.org/D113107

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

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

[Reid Kleckner via Phabricator via cfe-commits]

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

Yes, looks good to me, thanks!


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

https://reviews.llvm.org/D130709

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

[PATCH] D130510: Missing tautological compare warnings due to unary operators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This is looking much closer to what I think we had in mind, so I mostly have 
some cleanup suggestions.




Comment at: clang/lib/Analysis/CFG.cpp:979
 
-const BinaryOperator *BitOp = dyn_cast(BoolExpr);
+const BinaryOperator *BitOp = 
dyn_cast(BoolExpr->IgnoreParens());
 if (BitOp && (BitOp->getOpcode() == BO_And ||

You can drop the `IgnoreParens()` here -- `BoolExpr` is either `LHSExpr` or 
`RHSExpr`, and both of those were initialized from a call to `IgnoreParens()`.



Comment at: clang/lib/Analysis/CFG.cpp:1013-1015
+  // Helper function to get the sub expressions also for the unary operators. 
If
+  // unary are encountered will solve manually. This function can return value
+  // as 12 and also -12.





Comment at: clang/lib/Analysis/CFG.cpp:1017-1019
+// The value to return;
+Optional Value = llvm::None;
+

Removing an unnecessary variable.



Comment at: clang/lib/Analysis/CFG.cpp:1020-1021
+
+const IntegerLiteral *IntLiteral = nullptr;
+const UnaryOperator *UnOp = dyn_cast(E->IgnoreParens());
+

Coding style nit -- if the type is spelled out explicitly in the 
initialization, we tend to prefer using `auto` for the type so we don't have to 
repeat the type information twice.

Also removes a declaration that's not needed.



Comment at: clang/lib/Analysis/CFG.cpp:1027
+  // Literal.
+  const Expr *SubExpr = UnOp->getSubExpr();
+  IntLiteral = dyn_cast(SubExpr->IgnoreParens());

This ensures everything past here doesn't have to care about parens.



Comment at: clang/lib/Analysis/CFG.cpp:1028-1031
+  IntLiteral = dyn_cast(SubExpr->IgnoreParens());
+
+  if (IntLiteral) {
+





Comment at: clang/lib/Analysis/CFG.cpp:1032
+
+Value = IntLiteral->getValue();
+// Getting the Operator Code to perform them manually.





Comment at: clang/lib/Analysis/CFG.cpp:1033-1034
+Value = IntLiteral->getValue();
+// Getting the Operator Code to perform them manually.
+const UnaryOperatorKind OpCode = UnOp->getOpcode();
+





Comment at: clang/lib/Analysis/CFG.cpp:1041-1045
+  return -*Value;
+case UO_Not:
+  return ~*Value;
+case UO_LNot:
+  return *Value = !*Value;





Comment at: clang/lib/Analysis/CFG.cpp:1047
+default:
+  return Value;
+}

If there's a unary expression operating on an integer literal... it has to be 
one of the listed operations above, otherwise I don't think you can get here. 
So might as well assert and bail out rather than return a value that we didn't 
properly perform the unary operation on.



Comment at: clang/lib/Analysis/CFG.cpp:1050-1058
+
+} else {
+
+  IntLiteral = dyn_cast(E->IgnoreParens());
+  if (IntLiteral)
+return Value = IntLiteral->getValue();
+}




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

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

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

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130033

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

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

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added a comment.

@dcoughlin @kubamracek  @compnerd any comments before I commit this patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131714

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

[clang] ba1c396 - MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

[Balazs Benics via cfe-commits]

Author: Fred Tingaud
Date: 2022-08-16T17:09:55+02:00
New Revision: ba1c396e09a6dc56d817df0d378f3c826bbacaaa

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

LOG: MSVC compatibility mode: fix error on unqualified templated base class 
initialization in case of partial specialization

I introduced a patch to handle unqualified templated base class
initialization in MSVC compatibility mode:
https://reviews.llvm.org/rGc894e85fc64dd8d83b460de81080fff93c5ca334
We identified a problem with this patch in the case where the base class
is partially specialized, which can lead to triggering an assertion in
the case of a mix between types and values.
The minimal test case is:

  template  class Vec {};
  template  class Index : public Vec {
Index() : Vec() {}
  };
  template class Index<0>;

The detailed problem is that I was using the
`InjectedClassNameSpecialization`, to which the class template arguments
were then applied in order. But in the process, we were losing all the
partial specializations of the base class and creating an index mismatch
between the expected and passed arguments.

Patch By: frederic-tingaud-sonarsource

Reviewed By: rnk

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

Added: 


Modified: 
clang/lib/Sema/SemaDeclCXX.cpp
clang/test/SemaTemplate/ms-unqualified-base-class.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 78c39197e47bb..9ab009488c955 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -4309,11 +4309,21 @@ Sema::BuildMemInitializer(Decl *ConstructorD,
   }
 
   if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
-auto UnqualifiedBase = R.getAsSingle();
-if (UnqualifiedBase) {
-  Diag(IdLoc, diag::ext_unqualified_base_class)
-  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
-  BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+if (auto UnqualifiedBase = R.getAsSingle()) {
+  auto *TempSpec = cast(
+  UnqualifiedBase->getInjectedClassNameSpecialization());
+  TemplateName TN = TempSpec->getTemplateName();
+  for (auto const &Base : ClassDecl->bases()) {
+auto BaseTemplate =
+Base.getType()->getAs();
+if (BaseTemplate && Context.hasSameTemplateName(
+BaseTemplate->getTemplateName(), TN)) {
+  Diag(IdLoc, diag::ext_unqualified_base_class)
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+  BaseType = Base.getType();
+  break;
+}
+  }
 }
   }
 

diff  --git a/clang/test/SemaTemplate/ms-unqualified-base-class.cpp 
b/clang/test/SemaTemplate/ms-unqualified-base-class.cpp
index 6d9a072769c63..5e926c5087f93 100644
--- a/clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ b/clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@ int main() {
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template 
is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Array;
+
+template  class Wrong : public Vec {
+  Wrong() : NonExistent() {} // expected-error {{member initializer 
'NonExistent' does not name a non-static data member or base class}}
+};
+
+template class Wrong;
+
+template  class Wrong2 : public Vec {
+  Wrong2() : Vec() {} // expected-error {{too few template arguments for 
class template 'Vec'}}
+};
+
+template class Wrong2;
+
+template  class Wrong3 : public Vec {
+  Wrong3() : Base() {} // expected-error {{member initializer 'Base' does not 
name a non-static data member or base class}}
+};
+
+template class Wrong3;



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

[PATCH] D130709: MSVC compatibility mode: fix error on unqualified templated base class initialization in case of partial specialization

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGba1c396e09a6: MSVC compatibility mode: fix error on 
unqualified templated base class… (authored by frederic-tingaud-sonarsource, 
committed by steakhal).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130709

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaTemplate/ms-unqualified-base-class.cpp


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template 
is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data 
member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class 
templates is a Microsoft extension}}
+};
+
+template class Array;
+
+template  class Wrong : public Vec {
+  Wrong() : NonExistent() {} // expected-error {{member initializer 
'NonExistent' does not name a non-static data member or base class}}
+};
+
+template class Wrong;
+
+template  class Wrong2 : public Vec {
+  Wrong2() : Vec() {} // expected-error {{too few template arguments for 
class template 'Vec'}}
+};
+
+template class Wrong2;
+
+template  class Wrong3 : public Vec {
+  Wrong3() : Base() {} // expected-error {{member initializer 'Base' does not 
name a non-static data member or base class}}
+};
+
+template class Wrong3;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4309,11 +4309,21 @@
   }
 
   if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
-auto UnqualifiedBase = R.getAsSingle();
-if (UnqualifiedBase) {
-  Diag(IdLoc, diag::ext_unqualified_base_class)
-  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
-  BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+if (auto UnqualifiedBase = R.getAsSingle()) {
+  auto *TempSpec = cast(
+  UnqualifiedBase->getInjectedClassNameSpecialization());
+  TemplateName TN = TempSpec->getTemplateName();
+  for (auto const &Base : ClassDecl->bases()) {
+auto BaseTemplate =
+Base.getType()->getAs();
+if (BaseTemplate && Context.hasSameTemplateName(
+BaseTemplate->getTemplateName(), TN)) {
+  Diag(IdLoc, diag::ext_unqualified_base_class)
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+  BaseType = Base.getType();
+  break;
+}
+  }
 }
   }
 


Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- clang/test/SemaTemplate/ms-unqualified-base-class.cpp
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -83,3 +83,37 @@
 
   return I;
 }
+
+template  class Vec {}; // expected-note {{template is declared here}}
+
+template  class Index : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Index() : Vec() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template class Index<0>;
+
+template  class Array : public Vec {
+  // after-error@+1 {{member initializer 'Vec' does not name a non-static data member or base class}}
+  Array() : Vec() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template class Array;
+
+template  class Wrong : public Vec {
+  Wrong() : NonExistent() {} // expected-error {{member initializer 'NonExistent' does not name a non-static data member or base class}}
+};
+
+template class Wrong;
+
+template  class Wrong2 : public Vec {
+  Wrong2() : Vec() {} // expected-error {{too few template arguments for class template 'Vec'}}
+};
+
+template class Wrong2;
+
+template  class Wrong3 : public Vec {
+  Wrong3() : Base() {} // expected-error {{member initializer 'Base' does not name a non-static data member or base class}}
+};
+
+template class Wrong3;
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4

[PATCH] D131685: [clang][AST] RecursiveASTVisitor should visit owned TagDecl of friend type.

[David Rector via Phabricator via cfe-commits]

davrec added a comment.

Once the FIXME is removed this looks good, but I was involved in this so better 
if @sammccall can give the thumbs up at least to the RecursiveASTVisitor code.




Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1577-1578
 DEF_TRAVERSE_DECL(FriendTemplateDecl, {
+  // FIXME: Traverse getOwnedTagDecl like at the FriendDecl case?
+  // (FriendTemplateDecl is not used)
   if (D->getFriendType())

I don't think anything is necessary here, because we should never see an 
`OwnedTagDecl` here.
In the FriendDecl case the ElaboratedType only has an OwnedTagDecl when `struct 
Fr` has not yet been declared before the friend decl.
In the documentation of FriendTemplateDecl on the other hand these examples are 
given:
```
/// template \ class A {
///   friend class MyVector; // not a friend template
///   template \ friend class B; // not a friend template
///   template \ friend class Foo::Nested; // friend template
/// };
```
So, a FriendTemplateDecl is only created when referencing a nested class 
template member.  But that *must* have been declared before the friend decl, or 
we will get an error:

```
template struct B;
template struct A { template friend struct B::Fr; }; 
//ERROR: no member named 'Fr' in B
```

So the OwnedTagDecl should always be nullptr here/the issue should never arise 
in this case.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131685

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

[PATCH] D131614: [clang][dataflow] Extend transfer functions for other `CFGElement`s

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:144
 llvm::Optional>>>
-runDataflowAnalysis(
+runDataflowAnalysisOnCFG(
 const ControlFlowContext &CFCtx, AnalysisT &Analysis,





Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:167
+
+  // Contains the CFG which the analysis is run over.
+  const ControlFlowContext &CFCtx;





Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:81
+//
+//  `AnalysisT` contains a type `Lattice`.
 template 

Please use triple slash for doc comments.



Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:83
 template 
-llvm::Error checkDataflow(
+llvm::Error checkDataflowOnCFG(
 llvm::StringRef Code,




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131614

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

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp:236-238
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
   sum += sizeof(PMyStruct);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate

chrish_ericsson_atx wrote:
> njames93 wrote:
> > mizvekov wrote:
> > > Is this change really desirable, or should we put a FIXME here?
> > Not warning on these cases seems like a pretty big red flag, especially the 
> > `MyStruct *`.
> Thank you for your comment!  I'm not sure I fully agree that this is a red 
> flag.  I'm inclined to think that whether or not there should be a warning on 
> `MyStruct *` or `PMyStruct` is a pretty subjective.  These are both very 
> common idioms, and are meaningful.  I do appreciate the perspective that 
> `MyStruct *` is just one character different from `MyStruct`, and as such, it 
> might be a typo to ask for sizeof the pointer, when you really wanted sizeof 
> the struct.  But the counter-argument (accidentally asking for sizeof the 
> struct when you really wanted the pointer size) is just as valid-- and the 
> checker obviously cannot warn in that case.   
> 
> I am perfectly fine with kicking the can down the road a bit by replacing the 
> discarded `// CHECK-MESSAGES` directive with a `// FIXME`, as @mizvekov 
> suggested.  I expect that when someone circles back to really deeply 
> reconsider the semantics of this checker, there will be a number of changes 
> (other existing warnings dropped, warnings for new and missed cases added, 
> much better sync between C and C++, as well as intentional consideration of 
> things like __typeof__ (in it's various forms) and decltype, rather than the 
> haphazard coarse-grain matching that seems to be going on now.
I agree with this patch only in so far that:

* This is effectively a partial revert of the changes made in 
https://reviews.llvm.org/rG15f3cd6bfc670ba6106184a903eb04be059e5977
* The checker was pretty much buggy to begin with.
* That change significantly increased the amount of patterns we would accept, 
but at the same time the existing tests did not pick that up and this was not 
carefully considered.
* It seems unreasonable that there is effectively no way to shut this warning 
up per site, except by a NOLINT directive.

If the amount of false positives is so high now that this check is unusable, 
then this is just a question of reverting to a less broken state temporarily.

But otherwise we can't leave it in the reverted state either. Without that 
change to use the canonical type or something similar, there is no reason to 
suppose any of these test cases would work at all as clang evolves and we 
improve the quality of implementation wrt type sugar retention.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131926

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

[clang] 9181ce6 - [Windows] Put init_seg(compiler/lib) in llvm.global_ctors

[Arthur Eubanks via cfe-commits]

Author: Arthur Eubanks
Date: 2022-08-16T08:16:18-07:00
New Revision: 9181ce623fd8189252659da7c48de1982597b79c

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

LOG: [Windows] Put init_seg(compiler/lib) in llvm.global_ctors

Currently we treat initializers with init_seg(compiler/lib) as similar
to any other init_seg, they simply have a global variable in the proper
section (".CRT$XCC" for compiler/".CRT$XCL" for lib) and are added to
llvm.used. However, this doesn't match with how LLVM sees normal (or
init_seg(user)) initializers via llvm.global_ctors. This
causes issues like incorrect init_seg(compiler) vs init_seg(user)
ordering due to GlobalOpt evaluating constructors, and the
ability to remove init_seg(compiler/lib) initializers at all.

Currently we use 'A' for priorities less than 200. Use 200 for
init_seg(compiler) (".CRT$XCC") and 400 for init_seg(lib) (".CRT$XCL"),
which do not append the priority to the section name. Priorities
between 200 and 400 use ".CRT$XCC${Priority}". This allows for
some wiggle room for people/future extensions that want to add
initializers between compiler and lib.

Fixes #56922

Reviewed By: rnk

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

Added: 


Modified: 
clang/include/clang/Basic/AttrDocs.td
clang/lib/CodeGen/CGDeclCXX.cpp
clang/test/CodeGenCXX/pragma-init_seg.cpp
llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
llvm/test/CodeGen/X86/ctor-priority-coff.ll

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index 6afd3063524c9..32084bf512bc1 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -127,6 +127,10 @@ relative ordering of values is important. For example:
 ``Obj2`` will be initialized *before* ``Obj1`` despite the usual order of
 initialization being the opposite.
 
+On Windows, ``init_seg(compiler)`` is represented with a priority of 200 and
+``init_seg(library)`` is represented with a priority of 400. ``init_seg(user)``
+uses the default 65535 priority.
+
 This attribute is only supported for C++ and Objective-C++ and is ignored in
 other language modes. Currently, this attribute is not implemented on z/OS.
   }];

diff  --git a/clang/lib/CodeGen/CGDeclCXX.cpp b/clang/lib/CodeGen/CGDeclCXX.cpp
index 420141362f0e2..8ceb4863d6ead 100644
--- a/clang/lib/CodeGen/CGDeclCXX.cpp
+++ b/clang/lib/CodeGen/CGDeclCXX.cpp
@@ -553,7 +553,18 @@ CodeGenModule::EmitCXXGlobalVarDeclInitFunc(const VarDecl 
*D,
 CXXThreadLocalInits.push_back(Fn);
 CXXThreadLocalInitVars.push_back(D);
   } else if (PerformInit && ISA) {
-EmitPointerToInitFunc(D, Addr, Fn, ISA);
+// Contract with backend that "init_seg(compiler)" corresponds to priority
+// 200 and "init_seg(lib)" corresponds to priority 400.
+int Priority = -1;
+if (ISA->getSection() == ".CRT$XCC")
+  Priority = 200;
+else if (ISA->getSection() == ".CRT$XCL")
+  Priority = 400;
+
+if (Priority != -1)
+  AddGlobalCtor(Fn, Priority, COMDATKey);
+else
+  EmitPointerToInitFunc(D, Addr, Fn, ISA);
   } else if (auto *IPA = D->getAttr()) {
 OrderGlobalInitsOrStermFinalizers Key(IPA->getPriority(),
   PrioritizedCXXGlobalInits.size());

diff  --git a/clang/test/CodeGenCXX/pragma-init_seg.cpp 
b/clang/test/CodeGenCXX/pragma-init_seg.cpp
index 187a06a0c0f0c..9ac8e33554cc4 100644
--- a/clang/test/CodeGenCXX/pragma-init_seg.cpp
+++ b/clang/test/CodeGenCXX/pragma-init_seg.cpp
@@ -10,12 +10,12 @@ namespace simple_init {
 #pragma init_seg(compiler)
 int x = f();
 // CHECK: @"?x@simple_init@@3HA" = dso_local global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr = private constant ptr 
@"??__Ex@simple_init@@YAXXZ", section ".CRT$XCC"
+// No function pointer!  This one goes on @llvm.global_ctors.
 
 #pragma init_seg(lib)
 int y = f();
 // CHECK: @"?y@simple_init@@3HA" = dso_local global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr.1 = private constant ptr 
@"??__Ey@simple_init@@YAXXZ", section ".CRT$XCL"
+// No function pointer!  This one goes on @llvm.global_ctors.
 
 #pragma init_seg(user)
 int z = f();
@@ -29,14 +29,14 @@ namespace internal_init {
 namespace {
 int x = f();
 // CHECK: @"?x@?A0x{{[^@]*}}@internal_init@@3HA" = internal global i32 0, 
align 4
-// CHECK: @__cxx_init_fn_ptr.2 = private constant ptr 
@"??__Ex@?A0x{{[^@]*}}@internal_init@@YAXXZ", section ".asdf"
+// CHECK: @__cxx_init_fn_ptr = private constant ptr 
@"??__Ex@?A0x{{[^@]*}}@internal_init@@YAXXZ", section ".asdf"
 }
 }
 
 namespace selectany_init {
 int __declspec(selectany) x = f();
 // CHECK: @"?x@selectany_init@@3HA" = weak_odr dso_local global i32 0, comdat, 
align 4
-// CHECK: @__cxx_init_fn_ptr.3 = private co

[PATCH] D131910: [Windows] Put init_seg(compiler/lib) in llvm.global_ctors

[Arthur Eubanks via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9181ce623fd8: [Windows] Put init_seg(compiler/lib) in 
llvm.global_ctors (authored by aeubanks).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131910

Files:
  clang/include/clang/Basic/AttrDocs.td
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/test/CodeGenCXX/pragma-init_seg.cpp
  llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
  llvm/test/CodeGen/X86/ctor-priority-coff.ll

Index: llvm/test/CodeGen/X86/ctor-priority-coff.ll
===
--- llvm/test/CodeGen/X86/ctor-priority-coff.ll
+++ llvm/test/CodeGen/X86/ctor-priority-coff.ll
@@ -6,6 +6,15 @@
 ; CHECK: .section.CRT$XCA00042,"dr"
 ; CHECK: .p2align3
 ; CHECK: .quad   f
+; CHECK: .section.CRT$XCC,"dr"
+; CHECK: .p2align3
+; CHECK: .quad   i
+; CHECK: .section.CRT$XCC00250,"dr"
+; CHECK: .p2align3
+; CHECK: .quad   k
+; CHECK: .section.CRT$XCL,"dr"
+; CHECK: .p2align3
+; CHECK: .quad   j
 ; CHECK: .section.CRT$XCT12345,"dr"
 ; CHECK: .p2align3
 ; CHECK: .quad   g
@@ -24,10 +33,13 @@
 @str1 = private dso_local unnamed_addr constant [6 x i8] c"first\00", align 1
 @str2 = private dso_local unnamed_addr constant [5 x i8] c"main\00", align 1
 
-@llvm.global_ctors = appending global [3 x { i32, ptr, ptr }] [
+@llvm.global_ctors = appending global [6 x { i32, ptr, ptr }] [
   { i32, ptr, ptr } { i32 12345, ptr @g, ptr null },
   { i32, ptr, ptr } { i32 42, ptr @f, ptr null },
-  { i32, ptr, ptr } { i32 23456, ptr @init_h, ptr @h }
+  { i32, ptr, ptr } { i32 23456, ptr @init_h, ptr @h },
+  { i32, ptr, ptr } { i32 200, ptr @i, ptr null },
+  { i32, ptr, ptr } { i32 400, ptr @j, ptr null },
+  { i32, ptr, ptr } { i32 250, ptr @k, ptr null }
 ]
 
 declare dso_local i32 @puts(ptr nocapture readonly) local_unnamed_addr
@@ -50,6 +62,23 @@
   ret void
 }
 
+define dso_local void @i() {
+entry:
+  store i8 43, ptr @h
+  ret void
+}
+
+define dso_local void @j() {
+entry:
+  store i8 44, ptr @h
+  ret void
+}
+
+define dso_local void @k() {
+entry:
+  store i8 45, ptr @h
+  ret void
+}
 
 ; Function Attrs: nounwind uwtable
 define dso_local i32 @main() local_unnamed_addr {
Index: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
===
--- llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
+++ llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp
@@ -1889,11 +1889,24 @@
 // string that sorts between .CRT$XCA and .CRT$XCU. In the general case, we
 // make a name like ".CRT$XCT12345", since that runs before .CRT$XCU. Really
 // low priorities need to sort before 'L', since the CRT uses that
-// internally, so we use ".CRT$XCA1" for them.
+// internally, so we use ".CRT$XCA1" for them. We have a contract with
+// the frontend that "init_seg(compiler)" corresponds to priority 200 and
+// "init_seg(lib)" corresponds to priority 400, and those respectively use
+// 'C' and 'L' without the priority suffix. Priorities between 200 and 400
+// use 'C' with the priority as a suffix.
 SmallString<24> Name;
+char LastLetter = 'T';
+bool AddPrioritySuffix = Priority != 200 && Priority != 400;
+if (Priority < 200)
+  LastLetter = 'A';
+else if (Priority < 400)
+  LastLetter = 'C';
+else if (Priority == 400)
+  LastLetter = 'L';
 raw_svector_ostream OS(Name);
-OS << ".CRT$X" << (IsCtor ? "C" : "T") <<
-(Priority < 200 ? 'A' : 'T') << format("%05u", Priority);
+OS << ".CRT$X" << (IsCtor ? "C" : "T") << LastLetter;
+if (AddPrioritySuffix)
+  OS << format("%05u", Priority);
 MCSectionCOFF *Sec = Ctx.getCOFFSection(
 Name, COFF::IMAGE_SCN_CNT_INITIALIZED_DATA | COFF::IMAGE_SCN_MEM_READ,
 SectionKind::getReadOnly());
Index: clang/test/CodeGenCXX/pragma-init_seg.cpp
===
--- clang/test/CodeGenCXX/pragma-init_seg.cpp
+++ clang/test/CodeGenCXX/pragma-init_seg.cpp
@@ -10,12 +10,12 @@
 #pragma init_seg(compiler)
 int x = f();
 // CHECK: @"?x@simple_init@@3HA" = dso_local global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr = private constant ptr @"??__Ex@simple_init@@YAXXZ", section ".CRT$XCC"
+// No function pointer!  This one goes on @llvm.global_ctors.
 
 #pragma init_seg(lib)
 int y = f();
 // CHECK: @"?y@simple_init@@3HA" = dso_local global i32 0, align 4
-// CHECK: @__cxx_init_fn_ptr.1 = private constant ptr @"??__Ey@simple_init@@YAXXZ", section ".CRT$XCL"
+// No function pointer!  This one goes on @llvm.global_ctors.
 
 #pragma init_seg(user)
 int z = f();
@@ -29,14 +29,14 @@
 namespace {
 int x = f();
 // CHECK: @"?x@?A0x{{[^@]*}}@internal_init@@3HA" = internal global i32 0, align 4
-/

[PATCH] D130096: [Clang][AMDGPU] Emit AMDGPU library control constants in clang

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 updated this revision to Diff 453021.
jhuber6 added a comment.

Adjusting, adding code generation options for the other constants and changing 
to use linkonce ODR linkage.

I attempted to follow Jon's suggestion and group it with the existing code. but 
all the existing handling for this occurs in the driver. So I don't think 
there's a convenient way to drop in this functionality without adding a new 
function as in this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130096

Files:
  clang/include/clang/Basic/CodeGenOptions.def
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/TargetInfo.cpp
  clang/lib/CodeGen/TargetInfo.h
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGen/amdgcn-control-constants.c

Index: clang/test/CodeGen/amdgcn-control-constants.c
===
--- /dev/null
+++ clang/test/CodeGen/amdgcn-control-constants.c
@@ -0,0 +1,54 @@
+// RUN: %clang_cc1 -x c -triple amdgcn-amd-amdhsa -target-cpu gfx90a -S -emit-llvm -o - %s | FileCheck %s --check-prefix=GFX90A
+// RUN: %clang_cc1 -x c -triple amdgcn-amd-amdhsa -target-cpu gfx1030 -S -emit-llvm -o - %s | FileCheck %s --check-prefix=GFX1030
+// RUN: %clang_cc1 -x c -triple amdgcn-amd-amdhsa -target-cpu gfx908 -ffast-math -S -emit-llvm -o - %s | FileCheck %s --check-prefix=FAST
+// RUN: %clang_cc1 -x c -triple amdgcn-amd-amdhsa -target-cpu gfx908 -ffinite-math-only -S -emit-llvm -o - %s | FileCheck %s --check-prefix=FINITE
+// RUN: %clang_cc1 -x c -triple amdgcn-amd-amdhsa -target-cpu gfx703 -fgpu-flush-denormals-to-zero -S -emit-llvm -o - %s | FileCheck %s --check-prefix=DAZ
+// RUN: %clang_cc1 -x c -triple amdgcn-amd-amdhsa -target-cpu gfx908 -funsafe-math-optimizations -S -emit-llvm -o - %s | FileCheck %s --check-prefix=UNSAFE-MATH
+
+// GFX90A: @__oclc_daz_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// GFX90A: @__oclc_wavefrontsize64 = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// GFX90A: @__oclc_finite_only_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// GFX90A: @__oclc_unsafe_math_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// GFX90A: @__oclc_correctly_rounded_sqrt32 = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// GFX90A: @__oclc_ISA_version = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i32 9010, align 4
+// GFX90A: @__oclc_ABI_version = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i32 400, align 4
+
+// GFX1030: @__oclc_daz_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// GFX1030: @__oclc_wavefrontsize64 = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// GFX1030: @__oclc_finite_only_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// GFX1030: @__oclc_unsafe_math_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// GFX1030: @__oclc_correctly_rounded_sqrt32 = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// GFX1030: @__oclc_ISA_version = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i32 10048, align 4
+// GFX1030: @__oclc_ABI_version = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i32 400, align 4
+
+// FAST: @__oclc_daz_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// FAST: @__oclc_wavefrontsize64 = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// FAST: @__oclc_finite_only_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// FAST: @__oclc_unsafe_math_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// FAST: @__oclc_correctly_rounded_sqrt32 = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// FAST: @__oclc_ISA_version = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i32 9008, align 4
+// FAST: @__oclc_ABI_version = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i32 400, align 4
+
+// FINITE: @__oclc_daz_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// FINITE: @__oclc_wavefrontsize64 = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// FINITE: @__oclc_finite_only_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// FINITE: @__oclc_unsafe_math_opt = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 0, align 1
+// FINITE: @__oclc_correctly_rounded_sqrt32 = linkonce_odr hidden local_unnamed_addr addrspace(4) constant i8 1, align 1
+// FINITE: @__oclc_ISA_version = linkonce_odr hidden local_unnamed_a

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx updated this revision to Diff 453022.
chrish_ericsson_atx added a comment.

Added missing newline and added // FIXME comments, per reviewer comments.


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

https://reviews.llvm.org/D131926

Files:
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c
  clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
@@ -232,10 +232,8 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
   sum += sizeof(&S);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
-  sum += sizeof(MyStruct*);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
-  sum += sizeof(PMyStruct);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(MyStruct*);  // FIXME: Warning here prior to 15f3cd6bfc6, 
consider whether to add it back.
+  sum += sizeof(PMyStruct);  // FIXME: Warning here prior to 15f3cd6bfc6, 
consider whether to add it back.
   sum += sizeof(PS);
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
   sum += sizeof(PS2);
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression-2.c
@@ -0,0 +1,62 @@
+// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t -- --
+// RUN: %check_clang_tidy %s bugprone-sizeof-expression %t -- -- -x c++
+
+#ifdef __cplusplus
+#define STRKWD
+#else
+#define STRKWD struct
+#endif
+
+int Test5() {
+  typedef int Array10[10];
+
+  struct MyStruct {
+Array10 arr;
+Array10* ptr;
+  };
+
+  typedef struct TypedefStruct {
+Array10 arr;
+Array10* ptr;
+  } TypedefStruct;
+
+  typedef const STRKWD MyStruct TMyStruct;
+  typedef const STRKWD MyStruct *PMyStruct;
+  typedef TMyStruct *PMyStruct2;
+  typedef const TypedefStruct *PTTStruct;
+
+  STRKWD MyStruct S;
+  TypedefStruct TS;
+  PMyStruct PS;
+  PMyStruct2 PS2;
+  Array10 A10;
+  PTTStruct PTTS;
+
+  int sum = 0;
+  sum += sizeof(&S);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(__typeof(&S));
+  sum += sizeof(&TS);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(__typeof(&TS));
+  sum += sizeof(STRKWD MyStruct*);
+  sum += sizeof(__typeof(STRKWD MyStruct*));
+  sum += sizeof(TypedefStruct*);
+  sum += sizeof(__typeof(TypedefStruct*));
+  sum += sizeof(PTTS);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(PMyStruct);
+  sum += sizeof(PS);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(PS2);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+  sum += sizeof(&A10);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
+
+#ifdef __cplusplus
+  MyStruct &rS = S;
+  sum += sizeof(rS); // same as sizeof(S), not a pointer.  So should not warn.
+#endif
+
+  return sum;
+}
Index: clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
@@ -148,7 +148,7 @@
   hasOperatorName("&"), hasUnaryOperand(ignoringParenImpCasts(
 hasType(hasCanonicalType(recordType());
   const auto PointerToStructType = hasUnqualifiedDesugaredType(
-  pointerType(pointee(hasCanonicalType(recordType();
+  pointerType(pointee(recordType(;
   const auto PointerToStructExpr = ignoringParenImpCasts(expr(
   hasType(hasCanonicalType(PointerToStructType)), unless(cxxThisExpr(;
 


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp
@@ -232,10 +232,8 @@
   // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 'sizeof(A*)

[PATCH] D131685: [clang][AST] RecursiveASTVisitor should visit owned TagDecl of friend type.

[David Rector via Phabricator via cfe-commits]

davrec added a comment.

In D131685#3716354 , @balazske wrote:

> I really do not know why parent of the node for the owned `TagDecl` node is 
> the `FriendDecl` and not a `TypeLoc` node, but it is working.
> The code `struct A { friend struct Fr; };` caused no problems either (no 
> duplicate visit of `struct Fr`).

This is because the TraverseDecl(OwnedTagDecl) call is performed in the 
TraverseFriendDecl call, after the TraverseTypeLoc call has already returned -- 
and I assume the ParentMap just looks uses the stack of traversal calls to 
generate the parents.
That said, this does raise the question of whether we should instead be 
changing DEF_TRAVERSE_TYPE(Elaborated) to *always* traverse its owned tag decl; 
that way the parent would be the TypeLoc node.  However in more usual cases an 
ET's owned tag decl is added to the parent DeclContext; e.g. for `struct B {int 
i} data;`,  `B` and `data` are added as separate declarations in the parent 
context (which is annoying -- so probably the Type node really *should* always 
be the parent of that decl, if we were writing the AST from scratch! -- but it 
is what it is).  So if traversed all owned tag decls, we would get some 
duplicate visitations.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131685

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

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp:236-238
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
   sum += sizeof(PMyStruct);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate

mizvekov wrote:
> chrish_ericsson_atx wrote:
> > njames93 wrote:
> > > mizvekov wrote:
> > > > Is this change really desirable, or should we put a FIXME here?
> > > Not warning on these cases seems like a pretty big red flag, especially 
> > > the `MyStruct *`.
> > Thank you for your comment!  I'm not sure I fully agree that this is a red 
> > flag.  I'm inclined to think that whether or not there should be a warning 
> > on `MyStruct *` or `PMyStruct` is a pretty subjective.  These are both very 
> > common idioms, and are meaningful.  I do appreciate the perspective that 
> > `MyStruct *` is just one character different from `MyStruct`, and as such, 
> > it might be a typo to ask for sizeof the pointer, when you really wanted 
> > sizeof the struct.  But the counter-argument (accidentally asking for 
> > sizeof the struct when you really wanted the pointer size) is just as 
> > valid-- and the checker obviously cannot warn in that case.   
> > 
> > I am perfectly fine with kicking the can down the road a bit by replacing 
> > the discarded `// CHECK-MESSAGES` directive with a `// FIXME`, as @mizvekov 
> > suggested.  I expect that when someone circles back to really deeply 
> > reconsider the semantics of this checker, there will be a number of changes 
> > (other existing warnings dropped, warnings for new and missed cases added, 
> > much better sync between C and C++, as well as intentional consideration of 
> > things like __typeof__ (in it's various forms) and decltype, rather than 
> > the haphazard coarse-grain matching that seems to be going on now.
> I agree with this patch only in so far that:
> 
> * This is effectively a partial revert of the changes made in 
> https://reviews.llvm.org/rG15f3cd6bfc670ba6106184a903eb04be059e5977
> * The checker was pretty much buggy to begin with.
> * That change significantly increased the amount of patterns we would accept, 
> but at the same time the existing tests did not pick that up and this was not 
> carefully considered.
> * It seems unreasonable that there is effectively no way to shut this warning 
> up per site, except by a NOLINT directive.
> 
> If the amount of false positives is so high now that this check is unusable, 
> then this is just a question of reverting to a less broken state temporarily.
> 
> But otherwise we can't leave it in the reverted state either. Without that 
> change to use the canonical type or something similar, there is no reason to 
> suppose any of these test cases would work at all as clang evolves and we 
> improve the quality of implementation wrt type sugar retention.
@mizvekov, I agree with your reasoning for saying "we can't leave it in the 
reverted state either".  But I'm not sure how to ensure that this gets the 
needed attention.  Should we just file a separate PR on github to track the 
needed refactoring?  I do not believe I'll have the bandwidth to look at this 
in the next few months.

In the meantime, assuming the bots are happy with patchset 2, I'll land this 
as-is later today.

Thank you very much for your feedback!


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

https://reviews.llvm.org/D131926

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

[PATCH] D131939: [clang-tidy] Add performance-expensive-flat-container-operation check

[Nicolas van Kempen via Phabricator via cfe-commits]

nicovank created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun, mgorny.
Herald added a project: All.
nicovank edited the summary of this revision.
nicovank edited the summary of this revision.
nicovank updated this revision to Diff 452886.
nicovank updated this revision to Diff 452894.
nicovank added a comment.
nicovank updated this revision to Diff 452896.
Eugene.Zelenko added reviewers: alex, aaron.ballman, njames93, 
LegalizeAdulthood.
Eugene.Zelenko added a project: clang-tools-extra.
nicovank updated this revision to Diff 453026.
nicovank marked an inline comment as done.
nicovank published this revision for review.
nicovank added subscribers: marksantaniello, ivanmurashko, 0x1eaf.
nicovank marked an inline comment as done.
Herald added a subscriber: cfe-commits.

Rename second test file


nicovank added a comment.

Add isLanguageVersionSupported.




Comment at: 
clang-tools-extra/clang-tidy/performance/ExpensiveFlatContainerOperationCheck.h:31
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:

Please add `isLanguageVersionSupported`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance/expensive-flat-container-operation.rst:6
+
+This check operates on vector-based containers such as
+``boost::container::flat_(map|set)`` and ``folly::sorted_vector_(map|set)``.

Please synchronize first statement with Release Notes.


This check has been enabled internally at Facebook for a few months now (with 
`OnlyWarnInLoops` disabled), where `folly::sorted_vector_map` is pretty widely 
used.

I saw `std::flat_(map|set)` and `std::flat_multi(map|set)` will appear in 
C++23, at which point this check can be updated to include these.

`folly::heap_vector_(map|set)` is another example of such containers, though 
still relatively new so not included here yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131939

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  
clang-tools-extra/clang-tidy/performance/ExpensiveFlatContainerOperationCheck.cpp
  
clang-tools-extra/clang-tidy/performance/ExpensiveFlatContainerOperationCheck.h
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/performance/expensive-flat-container-operation.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance/expensive-flat-container-operation-only-warn-in-loops.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance/expensive-flat-container-operation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance/expensive-flat-container-operation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance/expensive-flat-container-operation.cpp
@@ -0,0 +1,383 @@
+// RUN: %check_clang_tidy %s performance-expensive-flat-container-operation %t -- \
+// RUN:   -config="{CheckOptions: \
+// RUN: [{key: performance-expensive-flat-container-operation.OnlyWarnInLoops, \
+// RUN:   value: false}] \
+// RUN: }"
+
+namespace std {
+template 
+struct pair {
+  pair(T1, T2);
+};
+
+template
+struct initializer_list {
+  initializer_list();
+};
+
+template struct remove_reference  { typedef T type; };
+template struct remove_reference  { typedef T type; };
+template struct remove_reference { typedef T type; };
+
+template
+typename std::remove_reference::type&& move(T&&) noexcept;
+} // namespace std
+
+// Copied from boost/container/flat_map.hpp and boost/container/flat_set.hpp and simplified.
+namespace boost {
+namespace container {
+struct ordered_unique_range_t {};
+
+template 
+struct flat_map {
+  typedef Key key_type;
+  typedef T mapped_type;
+  typedef std::pair value_type;
+
+  typedef int size_type;
+  typedef struct {} iterator;
+  typedef struct {} const_iterator;
+
+  const_iterator begin() const noexcept;
+  const_iterator end() const noexcept;
+
+  template  std::pair emplace(Args&&...);
+
+  template  iterator emplace_hint(const_iterator, Args&&...);
+
+  template  std::pair try_emplace(const key_type&, Args&&...);
+  template  iterator try_emplace(const_iterator, const key_type&, Args&&...);
+  template  std::pair try_emplace(key_type&&, Args&&...);
+  template  iterator try_emplace(const_iterator, key_type&&, Args&&...);
+
+  std::pair insert(const value_type&);
+  std::pair insert(value_type&&);
+  iterator insert(const_iterator, const value_type&);
+  iterator insert(const_iterator, value_type&&);
+  template  void insert(InputIterator, InputIterator);
+  template  void insert(ordered_unique_range_t, InputIterator, InputIterator);
+  void insert(std::initializer_list);
+  void insert(ordered_unique_range_t, std::initializer_list);
+
+  iterator erase(const_iterator);
+  size_type erase(cons

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

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h:53-54
+  template 
+  CFGMatchSwitchBuilder CaseOfCFGStmt(MSMatcherT M,
+  MSActionT A) {
+StmtBuilder = StmtBuilder.template CaseOf(M, A);





Comment at: clang/include/clang/Analysis/FlowSensitive/CFGMatchSwitch.h:67-68
+  template 
+  CFGMatchSwitchBuilder CaseOfCFGInit(MSMatcherT M,
+  MSActionT A) {
+InitBuilder = InitBuilder.template CaseOf(M, A);





Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:92-93
+  template 
+  ASTMatchSwitchBuilder CaseOf(MSMatcherT M,
+   MSActionT A) {
 Matchers.push_back(std::move(M));

Can we keep doing this?



Comment at: clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h:102
 
-  MatchSwitch Build() && {
+  ASTMatchSwitch Build() {
 return [Matcher = BuildMatcher(), Actions = std::move(Actions)](

Ditto.



Comment at: clang/unittests/Analysis/FlowSensitive/CFGMatchSwitchTest.cpp:34-35
+struct CFGElementMatches {
+  unsigned int StmtMatches = 0;
+  unsigned int InitializerMatches = 0;
+};




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131616

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

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

[Matheus Izvekov via Phabricator via cfe-commits]

mizvekov added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/sizeof-expression.cpp:236-238
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate
   sum += sizeof(PMyStruct);
-  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: suspicious usage of 
'sizeof(A*)'; pointer to aggregate

chrish_ericsson_atx wrote:
> mizvekov wrote:
> > chrish_ericsson_atx wrote:
> > > njames93 wrote:
> > > > mizvekov wrote:
> > > > > Is this change really desirable, or should we put a FIXME here?
> > > > Not warning on these cases seems like a pretty big red flag, especially 
> > > > the `MyStruct *`.
> > > Thank you for your comment!  I'm not sure I fully agree that this is a 
> > > red flag.  I'm inclined to think that whether or not there should be a 
> > > warning on `MyStruct *` or `PMyStruct` is a pretty subjective.  These are 
> > > both very common idioms, and are meaningful.  I do appreciate the 
> > > perspective that `MyStruct *` is just one character different from 
> > > `MyStruct`, and as such, it might be a typo to ask for sizeof the 
> > > pointer, when you really wanted sizeof the struct.  But the 
> > > counter-argument (accidentally asking for sizeof the struct when you 
> > > really wanted the pointer size) is just as valid-- and the checker 
> > > obviously cannot warn in that case.   
> > > 
> > > I am perfectly fine with kicking the can down the road a bit by replacing 
> > > the discarded `// CHECK-MESSAGES` directive with a `// FIXME`, as 
> > > @mizvekov suggested.  I expect that when someone circles back to really 
> > > deeply reconsider the semantics of this checker, there will be a number 
> > > of changes (other existing warnings dropped, warnings for new and missed 
> > > cases added, much better sync between C and C++, as well as intentional 
> > > consideration of things like __typeof__ (in it's various forms) and 
> > > decltype, rather than the haphazard coarse-grain matching that seems to 
> > > be going on now.
> > I agree with this patch only in so far that:
> > 
> > * This is effectively a partial revert of the changes made in 
> > https://reviews.llvm.org/rG15f3cd6bfc670ba6106184a903eb04be059e5977
> > * The checker was pretty much buggy to begin with.
> > * That change significantly increased the amount of patterns we would 
> > accept, but at the same time the existing tests did not pick that up and 
> > this was not carefully considered.
> > * It seems unreasonable that there is effectively no way to shut this 
> > warning up per site, except by a NOLINT directive.
> > 
> > If the amount of false positives is so high now that this check is 
> > unusable, then this is just a question of reverting to a less broken state 
> > temporarily.
> > 
> > But otherwise we can't leave it in the reverted state either. Without that 
> > change to use the canonical type or something similar, there is no reason 
> > to suppose any of these test cases would work at all as clang evolves and 
> > we improve the quality of implementation wrt type sugar retention.
> @mizvekov, I agree with your reasoning for saying "we can't leave it in the 
> reverted state either".  But I'm not sure how to ensure that this gets the 
> needed attention.  Should we just file a separate PR on github to track the 
> needed refactoring?  I do not believe I'll have the bandwidth to look at this 
> in the next few months.
> 
> In the meantime, assuming the bots are happy with patchset 2, I'll land this 
> as-is later today.
> 
> Thank you very much for your feedback!
Oh please wait for others to review, I don't think landing today is reasonable!

I am not really a stakeholder for this checker except for that original change.

I would advise for you to wait for another more responsible reviewer to accept 
as well before merging, unless this gets stale for a long time and no one else 
seems to be interested.


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

https://reviews.llvm.org/D131926

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

[PATCH] D131872: [Intrinsics] Add initial support for NonNull attribute

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks accepted this revision.
aeubanks added a comment.

lgtm

Commit message nit:

Saying

  Test plan:
  ninja check-all

isn't super useful, just mentioning the desired changes is good enough (e.g. 
your code snippet)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131872

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

[PATCH] D131975: [clang][dataflow] Use llvm::is_contained()

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
gribozavr 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/D131975

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


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -20,6 +20,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include 
@@ -207,9 +208,7 @@
 
 bool Environment::canDescend(unsigned MaxDepth,
  const DeclContext *Callee) const {
-  return CallStack.size() <= MaxDepth &&
- std::find(CallStack.begin(), CallStack.end(), Callee) ==
- CallStack.end();
+  return CallStack.size() <= MaxDepth && !llvm::is_contained(CallStack, 
Callee);
 }
 
 Environment Environment::pushCall(const CallExpr *Call) const {


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -20,6 +20,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include 
@@ -207,9 +208,7 @@
 
 bool Environment::canDescend(unsigned MaxDepth,
  const DeclContext *Callee) const {
-  return CallStack.size() <= MaxDepth &&
- std::find(CallStack.begin(), CallStack.end(), Callee) ==
- CallStack.end();
+  return CallStack.size() <= MaxDepth && !llvm::is_contained(CallStack, Callee);
 }
 
 Environment Environment::pushCall(const CallExpr *Call) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113107: Support of expression granularity for _Float16.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

I think I introduced a bug when replacing the VisitUnaryMinus/Plus/Imag/Real 
with VisitMinus/Plus/Imag/Real. Now this simple test case is failing in the 
non-promotion path (with the +avx512fp16).

_Float16 _Complex MinusOp_c_c(_Float16 c) {

  return -c;

}

error: cannot compile this scalar expression yet

  return -c;
 ^~

1 error generated.

The way I implemented it is that I have added the HANDLE_UNOP macro but I think 
that's wrong. I think I need to keep the VisitUnary* methods and fork out of 
them in the promotion path!  Same for both Scalar and Complex Exprs. Your 
thoughts? 
Thanks.


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

https://reviews.llvm.org/D113107

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

[PATCH] D131808: [clang,flang] Add missing options fsyntax-only in help

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski accepted this revision.
awarzynski added a comment.

LGTM, thanks for the fix!

You may want to update to title to better reflect the contents (e.g. “Add help 
text for -fsyntax-only”). While the fact that it fixes 
https://github.com/llvm/llvm-project/issues/57033 motivated this change, the 
actual implementation does a bit more than just fixing that.

I can land this for you once I can access stable internet. Sorry for the delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131808

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

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Erich Keane via Phabricator via cfe-commits]

erichkeane marked 8 inline comments as done.
erichkeane added inline comments.



Comment at: clang/include/clang/Sema/Sema.h:3642
+  // template, for the purposes of [temp.friend] p9.
+  bool ConstraintExpressionDependsOnEnclosingTemplate(unsigned TemplateDepth,
+  const Expr *Constraint);

ChuanqiXu wrote:
> The meaning of `TemplateDepth` is unclear. Do it mean top-down or start from 
> the constraint expression?
It is top-down, as the constraint is uninstantiated.  I've added a comment to 
clarify.



Comment at: clang/include/clang/Sema/Template.h:507-518
+struct ConstraintEvalRAII {
+  TemplateDeclInstantiator &TI;
+  bool OldValue;
+
+  ConstraintEvalRAII(TemplateDeclInstantiator &TI)
+  : TI(TI), OldValue(TI.EvaluateConstraints) {
+TI.EvaluateConstraints = false;

ChuanqiXu wrote:
> Could we remove the duplicates? For example, is it possible to make 
> ConstraintEvalRAII a subclass of Sema?
It ends up having to be a template, and for a 'getter' to exist, but done.



Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:2786-2799
+template 
+static bool DeducedArgsNeedReplacement(TemplateDeclT *Template) {
+  return false;
+}
+template <>
+bool DeducedArgsNeedReplacement(
+VarTemplatePartialSpecializationDecl *Spec) {

ChuanqiXu wrote:
> nit:
These are specializations, so they cant have a storage class.  They inherit 
them from the primary template.



Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:1233
 /* DeclContext *Owner */ Owner, TemplateArgs);
+  DeclInstantiator.setEvaluateConstraints(EvaluateConstraints);
+  return DeclInstantiator.SubstTemplateParams(OrigTPL);

ChuanqiXu wrote:
> Do we have any method to detect if the template parameter list lives in a 
> require clause or not? The current duplicating looks a little bit bad.
> 
> If there is no such methods, I guess it may be better by using enums:
> 
> ```
> TemplateParameterList *TransformTemplateParameterList(
>   TemplateParameterList *OrigTPL, enum 
> IsInRequire = Normal) {
>...
>if (IsInRequire == ...)
>   DeclInstantiator.setEvaluateConstraints(EvaluateConstraints);
>...
> }
> ```
Turns out I don't think this was required anyway, so I just removed it.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2435
+  CXXRecordDecl *Record = cast(DC);
+  Expr *TrailingRequiresClause = D->getTrailingRequiresClause();
+

ChuanqiXu wrote:
> Is this used?
Yes, see line 2453, 2459, 2470, and 2476.


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

https://reviews.llvm.org/D126907

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

[PATCH] D113107: Support of expression granularity for _Float16.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D113107#3726496 , @zahiraam wrote:

> I think I introduced a bug when replacing the VisitUnaryMinus/Plus/Imag/Real 
> with VisitMinus/Plus/Imag/Real. Now this simple test case is failing in the 
> non-promotion path (with the +avx512fp16).
>
> _Float16 _Complex MinusOp_c_c(_Float16 c) {
>
>   return -c;
>
> }
>
> error: cannot compile this scalar expression yet
>
>   return -c;
>  ^~
>
> 1 error generated.
>
> The way I implemented it is that I have added the HANDLE_UNOP macro but I 
> think that's wrong. I think I need to keep the VisitUnary* methods and fork 
> out of them in the promotion path!  Same for both Scalar and Complex Exprs. 
> Your thoughts?

Right, you still need to declare `VisitUnary*` methods that the CRTP visitor 
will call.  But you can avoid the boilerplate of having separate methods by 
just adding the `PromotionType` argument to `VisitUnary*` with a default 
argument that's appropriate for the normal path:

  llvm::Value *VisitUnaryMinus(const UnaryOperator *E, QualType PromotionType = 
QualType());




Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3140
+  return CGF.Builder.CreateFPExt(result, ConvertType(E->getType()));
+  }
+  return result;

zahiraam wrote:
> rjmccall wrote:
> > Please extract this block out as:
> > 
> > ```
> > llvm::Value *EmitPromotedValue(llvm::Value *result, QualType PromotionType);
> > ```
> These changes you are proposing is when the argument of the unary __imag / 
> __real is of type _Complex Float16. I would think that this new method 
> EmitPromotedValue would be replacing the equivalent code in 
> ComplexEmitter::EmitPromoted instead,  not in the scalar emitter, right?
I probably mixed up which emitter I commented on.  The upshot is that I would 
like there to be `EmitPromotedValue` and `EmitUnpromotedValue` helper functions 
on both emitters (which of course would take/return an `llvm::Value*` on the 
scalar emitter and a `CGComplexPair` on the complex emitter), just so that we 
have all the value promotion/unpromotion logic for each emitter in one place.


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

https://reviews.llvm.org/D113107

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

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

[Erich Keane via Phabricator via cfe-commits]

erichkeane updated this revision to Diff 453055.
erichkeane marked 5 inline comments as done.
erichkeane added a comment.

Respond/fix all of the things @ChuanqiXu mentioned.  Intend to commit early 
tomorrow based on latest feedback unless others have concerns.


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

https://reviews.llvm.org/D126907

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Sema/Template.h
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/CXX/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
  
clang/test/CXX/temp/temp.constr/temp.constr.order/class-template-partial-specializations.cpp
  
clang/test/CXX/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp
  clang/test/SemaTemplate/concepts-friends.cpp
  clang/test/SemaTemplate/concepts.cpp
  clang/test/SemaTemplate/deferred-concept-inst.cpp
  clang/test/SemaTemplate/instantiate-requires-clause.cpp
  clang/test/SemaTemplate/trailing-return-short-circuit.cpp

Index: clang/test/SemaTemplate/trailing-return-short-circuit.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/trailing-return-short-circuit.cpp
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+template 
+  requires(sizeof(T) > 2) || T::value // #FOO_REQ
+void Foo(T){};// #FOO
+
+template 
+void TrailingReturn(T)   // #TRAILING
+  requires(sizeof(T) > 2) || // #TRAILING_REQ
+  T::value   // #TRAILING_REQ_VAL
+{};
+template 
+struct HasValue {
+  static constexpr bool value = B;
+};
+static_assert(sizeof(HasValue) <= 2);
+
+template 
+struct HasValueLarge {
+  static constexpr bool value = B;
+  int I;
+};
+static_assert(sizeof(HasValueLarge) > 2);
+
+void usage() {
+  // Passes the 1st check, short-circuit so the 2nd ::value is not evaluated.
+  Foo(1.0);
+  TrailingReturn(1.0);
+
+  // Fails the 1st check, but has a ::value, so the check happens correctly.
+  Foo(HasValue{});
+  TrailingReturn(HasValue{});
+
+  // Passes the 1st check, but would have passed the 2nd one.
+  Foo(HasValueLarge{});
+  TrailingReturn(HasValueLarge{});
+
+  // Fails the 1st check, fails 2nd because there is no ::value.
+  Foo(true);
+  // expected-error@-1{{no matching function for call to 'Foo'}}
+  // expected-note@#FOO{{candidate template ignored: constraints not satisfied [with T = bool]}}
+  // expected-note@#FOO_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#FOO_REQ{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
+
+  TrailingReturn(true);
+  // expected-error@-1{{no matching function for call to 'TrailingReturn'}}
+  // expected-note@#TRAILING{{candidate template ignored: constraints not satisfied [with T = bool]}}
+  // expected-note@#TRAILING_REQ{{because 'sizeof(_Bool) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#TRAILING_REQ_VAL{{because substituted constraint expression is ill-formed: type 'bool' cannot be used prior to '::' because it has no members}}
+
+  // Fails the 1st check, fails 2nd because ::value is false.
+  Foo(HasValue{});
+  // expected-error@-1 {{no matching function for call to 'Foo'}}
+  // expected-note@#FOO{{candidate template ignored: constraints not satisfied [with T = HasValue]}}
+  // expected-note@#FOO_REQ{{because 'sizeof(HasValue) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#FOO_REQ{{and 'HasValue::value' evaluated to false}}
+  TrailingReturn(HasValue{});
+  // expected-error@-1 {{no matching function for call to 'TrailingReturn'}}
+  // expected-note@#TRAILING{{candidate template ignored: constraints not satisfied [with T = HasValue]}}
+  // expected-note@#TRAILING_REQ{{because 'sizeof(HasValue) > 2' (1 > 2) evaluated to false}}
+  // expected-note@#TRAILING_REQ_VAL{{and 'HasValue::value' evaluated to false}}
+}
Index: clang/test/SemaTemplate/instantiate-requires-clause.cpp
===
--- clang/test/SemaTemplate/instantiate-requires-clause.cpp
+++ clang/test/SemaTemplate/instantiate-requires-clause.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -x c++ %s -verify
+// RUN: %clang_cc1 -std=c++2a -x c++ %s -Wno-unused-value -verify
 
 template  requires ((sizeof(Args) == 1), ...)
 // expected-note@-1 {{because '(sizeof(int) == 1) , (sizeof(char) == 1) , (sizeof(int) == 1)' evaluated to false}}
@@ -40,6 +40,20 @@
 
 static_assert(S::f(1));
 
+// Similar to the 'S' test, but tries to use 'U' in the requires clause.
+template 
+struct S1 {
+  // expected-note@+3 {{candidate template ignored: constraints not

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:544
   }
+  llvm::stable_sort(GlobalCtors, [](const Structor &L, const Structor &R) {
+return L.LexOrder < R.LexOrder;

rnk wrote:
> Please move this sorting into EmitCtorList and apply it to destructors. I 
> believe they are currently emitted in source order, and the loader executes 
> them in reverse order, so we get the desired reverse source order cleanup 
> behavior.
I looked into this. They are indeed "currently emitted in source order". 
However, if a dtor ever uses an entry in `llvm.global_dtors`, it must have 
bailed out of deferred emission here 
(https://github.com/llvm/llvm-project/blob/69c09d11f877a35655e285cda96ec0699e385fc9/clang/lib/CodeGen/CodeGenModule.cpp#L3256-L3263),
 which is to say, the corresponding ctor is also in lexing order. So the 
situation is either the ctor/dtor pair both use lexing order, or there is no 
dtor (like `inline int` with an initializer) and the ctor is deferred/reordered 
where this patch kicks in.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127233

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

[PATCH] D131978: [clang-format] Concepts: allow identifiers after negation

[Emilia Dreamer via Phabricator via cfe-commits]

rymiel created this revision.
rymiel added reviewers: MyDeveloperDay, curdeius, HazardyKnusperkeks, owenpan.
Herald added a project: All.
rymiel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Previously, the formatter would refuse to treat identifiers within a
compound `concept` definition as actually part of the definition, if
they were after the negation operator `!`. It is now made consistent
with the likes of `&&` and `||`.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131978

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


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24109,6 +24109,15 @@
"concept DelayedCheck = false || requires(T t) { t.bar(); } && "
"sizeof(T) <= 8;");
 
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !DerivedUnit;");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !(DerivedUnit);");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !!DerivedUnit;");
+
   verifyFormat("template \n"
"concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
"&& sizeof(T) <= 8;");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3537,7 +3537,8 @@
   switch (FormatTok->Previous->Tok.getKind()) {
   case tok::coloncolon:  // Nested identifier.
   case tok::ampamp:  // Start of a function or variable for the
-  case tok::pipepipe:// constraint expression.
+  case tok::pipepipe:// constraint expression. (binary)
+  case tok::exclaim: // The same as above, but unary.
   case tok::kw_requires: // Initial identifier of a requires clause.
   case tok::equal:   // Initial identifier of a concept declaration.
 break;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -24109,6 +24109,15 @@
"concept DelayedCheck = false || requires(T t) { t.bar(); } && "
"sizeof(T) <= 8;");
 
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !DerivedUnit;");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !(DerivedUnit);");
+
+  verifyFormat("template \n"
+   "concept DelayedCheck = Unit && !!DerivedUnit;");
+
   verifyFormat("template \n"
"concept DelayedCheck = !!false || requires(T t) { t.bar(); } "
"&& sizeof(T) <= 8;");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -3537,7 +3537,8 @@
   switch (FormatTok->Previous->Tok.getKind()) {
   case tok::coloncolon:  // Nested identifier.
   case tok::ampamp:  // Start of a function or variable for the
-  case tok::pipepipe:// constraint expression.
+  case tok::pipepipe:// constraint expression. (binary)
+  case tok::exclaim: // The same as above, but unary.
   case tok::kw_requires: // Initial identifier of a requires clause.
   case tok::equal:   // Initial identifier of a concept declaration.
 break;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130510: Missing tautological compare warnings due to unary operators

[Muhammad Usman Shahid via Phabricator via cfe-commits]

Codesbyusman updated this revision to Diff 453058.
Codesbyusman marked 10 inline comments as done.
Codesbyusman added a comment.

updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-bitwise-compare.cpp
  clang/test/SemaCXX/warn-unreachable.cpp

Index: clang/test/SemaCXX/warn-unreachable.cpp
===
--- clang/test/SemaCXX/warn-unreachable.cpp
+++ clang/test/SemaCXX/warn-unreachable.cpp
@@ -396,16 +396,18 @@
   if (y == -1 && y != -1)  // expected-note {{silence}}
 calledFun();// expected-warning {{will never be executed}}
 
-  // TODO: Extend warning to the following code:
-  if (x < -1)
-calledFun();
-  if (x == -1)
-calledFun();
+  if (x == -1)   // expected-note {{silence}}
+calledFun(); // expected-warning {{will never be executed}}
 
-  if (x != -1)
+  if (x != -1)   // expected-note {{silence}}
 calledFun();
   else
+calledFun(); // expected-warning {{will never be executed}}
+
+  // TODO: Extend warning to the following code:
+  if (x < -1)
 calledFun();
+ 
   if (-1 > x)
 calledFun();
   else
Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -11,3 +11,15 @@
   bool b4 = !!(x | 5);
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
 }
+
+template   // silence
+void foo(int x) {
+bool b1 = (x & sizeof(T)) == 8;
+bool b2 = (x & I) == 8;
+bool b3 = (x & 4) == 8; // expected-warning {{bitwise comparison always evaluates to false}}
+}
+
+void run(int x) {
+foo<4, int>(8); // expected-note {{in instantiation of function template specialization 'foo<4, int>' requested here}}
+}
+
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
 #define mydefine 2
+#define mydefine2 -2
 
 enum {
   ZERO,
@@ -11,29 +12,66 @@
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((-8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & -8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((x & 8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((2 & x) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((x & -8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-2 & x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+ 
   if ((x | 4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x | 3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+
+  if ((x | -4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x | -3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  
   if ((x & 0x15) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & 0xFFEB) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((0x23 | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((0xFFDD | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
 
   if (!!((8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if (!!((-8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   int y = ((8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((-8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
+  y = ((-3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
 
   if ((x & 8) == 8) {}
   if ((x & 8) != 8) {}
   if ((x | 4) == 4) {}
   if ((x | 4) != 4) {}
 
+  if ((-2 & x) != 4) {}
+  if ((x & -8) == -8) {}
+  if ((x & -8) != -8) {}
+  if ((x | -4) == -4) {}
+  if ((x | -4) !

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

There's no compiler interoperation problem here; it's just a semantic concern 
that someone could've been relying on the old behavior.  The new behavior is 
(AIUI) clearly required by the language standard, and I don't think we want to 
get into the business of providing "do the old buggy thing" flags for 
everything we fix like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127233

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

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D127233#3726613 , @rjmccall wrote:

> There's no compiler interoperation problem here; it's just a semantic concern 
> that someone could've been relying on the old behavior.  The new behavior is 
> (AIUI) clearly required by the language standard, and I don't think we want 
> to get into the business of providing "do the old buggy thing" flags for 
> everything we fix like this.

Thanks for verifying! (I agree that we don't want to get into the business of 
providing flags for old buggy behavior, too.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127233

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

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> is this an ABI breaking change

It only affects the order of initialization within a file, so it doesn't really 
have any implications for the binary ABI.  It might break code, of course, but 
that's a different issue.

If we want a flag to maintain the old behavior, we need to define what the 
"old" order is.  Not sure that's worthwhile.




Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:580
+I = DelayedCXXInitPosition.find(D);
+unsigned LexOrder = I == DelayedCXXInitPosition.end() ? ~0U : I->second;
+AddGlobalCtor(Fn, 65535, LexOrder, COMDATKey);

This ensures delayed initialization calls are ordered relative to each other... 
but are they ordered correctly relative to non-delayed initialization calls?  
I'm skeptical that using a LexOrder of "~0U" is really correct.  (For example, 
if you change the variable "b" in your testcase to a struct with a destructor.)



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:544
   }
+  llvm::stable_sort(GlobalCtors, [](const Structor &L, const Structor &R) {
+return L.LexOrder < R.LexOrder;

ychen wrote:
> rnk wrote:
> > Please move this sorting into EmitCtorList and apply it to destructors. I 
> > believe they are currently emitted in source order, and the loader executes 
> > them in reverse order, so we get the desired reverse source order cleanup 
> > behavior.
> I looked into this. They are indeed "currently emitted in source order". 
> However, if a dtor ever uses an entry in `llvm.global_dtors`, it must have 
> bailed out of deferred emission here 
> (https://github.com/llvm/llvm-project/blob/69c09d11f877a35655e285cda96ec0699e385fc9/clang/lib/CodeGen/CodeGenModule.cpp#L3256-L3263),
>  which is to say, the corresponding ctor is also in lexing order. So the 
> situation is either the ctor/dtor pair both use lexing order, or there is no 
> dtor (like `inline int` with an initializer) and the ctor is 
> deferred/reordered where this patch kicks in.
> 
Is MayBeEmittedEagerly always true for variables with destructors?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127233

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

Was it intended that the warning generated here isn't silenced by `-w`, only by 
an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and that 
`-Wno-error` doesn't downgrade the error? See https://godbolt.org/z/s9qPveTWG 
for an example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

I agree that the change in behaviour is reasonable and have no objections to 
it. The code should not rely on particular output of `__PRETTY_FUNCTION__`.
I just wanted to point out that we still don't match GCC in other cases, not 
that is was a worthwhile goal to chase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

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

[PATCH] D130510: Missing tautological compare warnings due to unary operators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Nice! Just a few more small nits to fix that I can see.




Comment at: clang/lib/Analysis/CFG.cpp:1016-1021
+  Optional getIntegerLiteralSubexpressionValue(const Expr *E) {
+
+const auto *UnOp = dyn_cast(E->IgnoreParens());
+
+// If unary.
+if (UnOp) {

Oops, I realized we could simplify this further and keep `UnOp` scoped more 
tightly to where it's used.



Comment at: clang/lib/Analysis/CFG.cpp:1026-1032
+  if (const auto *IntLiteral = dyn_cast(SubExpr)) {
+
+llvm::APInt Value = IntLiteral->getValue();
+UnaryOperatorKind OpCode = UnOp->getOpcode();
+
+// Perform the operation manually.
+switch (OpCode) {

Another simplification to scope things more tightly.



Comment at: clang/lib/Analysis/CFG.cpp:1045-1047
+  }
+
+} else if (const auto *IntLiteral =




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131307#3726631 , @smeenai wrote:

> Was it intended that the warning generated here isn't silenced by `-w`, only 
> by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and 
> that `-Wno-error` doesn't downgrade the error? See 
> https://godbolt.org/z/s9qPveTWG for an example.

Yes, we've discussed that on this thread before: Clang's behavior for 
warnings-as-default-error require explicit suppression of the warning, and 
isn't effected by global warning/error settings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131307#3726631 , @smeenai wrote:

> Was it intended that the warning generated here isn't silenced by `-w`, only 
> by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and 
> that `-Wno-error` doesn't downgrade the error? See 
> https://godbolt.org/z/s9qPveTWG for an example.

Yes. That is the behavior of warnings which default to an error. The idea is: 
these aren't really *warnings*, they're errors that we let users downgrade for 
. So `-w` shouldn't blanket disable them or users will be very 
surprised when that warning turns into a hard error in a future version of the 
compiler. So you have to explicitly disable warnings that default to an error. 
The same is true for `-Wno-error` behavior.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 453062.
shafik marked an inline comment as done.
shafik added a comment.

- Addressing comments on casting of `EvaluatingDecl`


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

https://reviews.llvm.org/D131874

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


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2419,6 +2419,13 @@
 
 enum EMaxInt {emaxint1=-1, emaxint2=__INT_MAX__};
 
+enum NumberType {};
+
+E2 testDefaultArgForParam(E2 e2Param = (E2)-1) { // ok, not a constant 
expression context
+  E2 e2LocalInit = e2Param; // ok, not a constant expression context
+  return e2LocalInit;
+}
+
 void testValueInRangeOfEnumerationValues() {
   constexpr E1 x1 = static_cast(-8);
   constexpr E1 x2 = static_cast(8);
@@ -2454,6 +2461,8 @@
   constexpr EMaxInt x19 = static_cast(__INT_MAX__-1);
   constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1);
   // expected-error@-1 {{integer value 2147483648 is outside the valid range 
of values [-2147483648, 2147483647] for this enumeration type}}
+
+  const NumberType neg_one = (NumberType) ((NumberType) 0 - (NumberType) 1); 
// ok, not a constant expression context
 }
 
 enum SortOrder {
@@ -2470,3 +2479,8 @@
 return;
 }
 }
+
+GH50055::E2 GlobalInitNotCE1 = (GH50055::E2)-1; // ok, not a constant 
expression context
+GH50055::E2 GlobalInitNotCE2 = GH50055::testDefaultArgForParam(); // ok, not a 
constant expression context
+constexpr GH50055::E2 GlobalInitCE = (GH50055::E2)-1;
+// expected-error@-1 {{integer value -1 is outside the valid range of values 
[0, 7] for this enumeration type}}
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -13536,6 +13536,19 @@
 if (Info.Ctx.getLangOpts().CPlusPlus && Info.InConstantContext &&
 Info.EvalMode == EvalInfo::EM_ConstantExpression &&
 DestType->isEnumeralType()) {
+
+  bool ConstexprVar = true;
+
+  // We know if we are here that we are in a context that we might require
+  // a constant expression or a context that requires a constant
+  // value. But if we are initializing a value we don't know if it is a
+  // constexpr variable or not. We can check the EvaluatingDecl to 
determine
+  // if it constexpr or not. If not then we don't want to emit a 
diagnostic.
+  if (const auto *VD = 
dyn_cast_or_null(Info.EvaluatingDecl.dyn_cast())) {
+if (VD && !VD->isConstexpr())
+  ConstexprVar = false;
+  }
+
   const EnumType *ET = dyn_cast(DestType.getCanonicalType());
   const EnumDecl *ED = ET->getDecl();
   // Check that the value is within the range of the enumeration values.
@@ -13555,13 +13568,14 @@
 ED->getValueRange(Max, Min);
 --Max;
 
-if (ED->getNumNegativeBits() &&
+if (ED->getNumNegativeBits() && ConstexprVar &&
 (Max.slt(Result.getInt().getSExtValue()) ||
  Min.sgt(Result.getInt().getSExtValue(
-  Info.Ctx.getDiagnostics().Report(E->getExprLoc(),
-   
diag::warn_constexpr_unscoped_enum_out_of_range)
-  << llvm::toString(Result.getInt(),10) << Min.getSExtValue() << 
Max.getSExtValue();
-else if (!ED->getNumNegativeBits() &&
+  Info.Ctx.getDiagnostics().Report(
+  E->getExprLoc(), diag::warn_constexpr_unscoped_enum_out_of_range)
+  << llvm::toString(Result.getInt(), 10) << Min.getSExtValue()
+  << Max.getSExtValue();
+else if (!ED->getNumNegativeBits() && ConstexprVar &&
  Max.ult(Result.getInt().getZExtValue()))
   Info.Ctx.getDiagnostics().Report(E->getExprLoc(),

diag::warn_constexpr_unscoped_enum_out_of_range)


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2419,6 +2419,13 @@
 
 enum EMaxInt {emaxint1=-1, emaxint2=__INT_MAX__};
 
+enum NumberType {};
+
+E2 testDefaultArgForParam(E2 e2Param = (E2)-1) { // ok, not a constant expression context
+  E2 e2LocalInit = e2Param; // ok, not a constant expression context
+  return e2LocalInit;
+}
+
 void testValueInRangeOfEnumerationValues() {
   constexpr E1 x1 = static_cast(-8);
   constexpr E1 x2 = static_cast(8);
@@ -2454,6 +2461,8 @@
   constexpr EMaxInt x19 = static_cast(__INT_MAX__-1);
   constexpr EMaxInt x20 = static_cast((long)__INT_MAX__+1);
   // expected-error@-1 {{integer value 2147483648 is outside the valid range of values [-2147483648, 2147483647] for thi

[PATCH] D131934: [clang][deps] Compute command-lines for dependencies immediately

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM overall with some minor nitpicks.




Comment at: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h:95
+  const llvm::StringSet<> &AlreadySeen,
+  llvm::function_ref
+  LookupModuleOutput,

Creating a type alias might make this more readable & easier to refactor later.



Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:102
+  if (!CI.getDiagnosticOpts().DiagnosticSerializationFile.empty())
+CI.getDiagnosticOpts().DiagnosticSerializationFile = "-";
+  if (!CI.getDependencyOutputOpts().OutputFile.empty())

Can you point out in a comment that this value is just temporary for context 
hash computation and will be replaced by real path later on?



Comment at: clang/tools/clang-scan-deps/ClangScanDeps.cpp:403
+
+static std::string getModuleCachePath(ArrayRef Args) {
+  for (StringRef Arg : llvm::reverse(Args)) {

Can you split this into separate patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131934

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

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Wang Yihan via Phabricator via cfe-commits]

yihanaa created this revision.
yihanaa added reviewers: rjmccall, aaron.ballman, erichkeane, lebedev.ri.
yihanaa added a project: clang.
Herald added a project: All.
yihanaa requested review of this revision.
Herald added a subscriber: cfe-commits.

Clang will crash when __builtin_assume_aligned's 1st arg is array type(or 
string literal).
Open issue: https://github.com/llvm/llvm-project/issues/57169


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131979

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/builtin-assume-aligned.c
  clang/test/CodeGen/catch-alignment-assumption-ignorelist.c


Index: clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
===
--- clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
+++ clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
@@ -26,3 +26,9 @@
 void *ignore_volatiles(volatile void * x) {
   return __builtin_assume_aligned(x, 1);
 }
+
+// CHECK-LABEL: ignore_volatiles
+void ignore_volatiles_array() {
+  volatile char arr[] = "a";
+  (void)__builtin_assume_aligned(arr, 1);
+}
Index: clang/test/CodeGen/builtin-assume-aligned.c
===
--- clang/test/CodeGen/builtin-assume-aligned.c
+++ clang/test/CodeGen/builtin-assume-aligned.c
@@ -1,6 +1,8 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-unknown-unknown 
-emit-llvm -o - %s | FileCheck %s
 
+// CHECK: [[TEST7_STR:@.*]] = private unnamed_addr constant [2 x i8] c"a\00", 
align 1
+
 // CHECK-LABEL: @test1(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[A_ADDR:%.*]] = alloca i32*, align 8
@@ -124,3 +126,10 @@
   a = __builtin_assume_aligned(a, 4294967296);
 return a[0];
 }
+
+// CHECK-LABEL: @test7(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:call void @llvm.assume(i1 true) [ "align"(i8* getelementptr 
inbounds ([2 x i8], [2 x i8]* [[TEST7_STR]], i64 0, i64 0), i64 1) ]
+void test7(void) {
+  (void) __builtin_assume_aligned("a", 1);
+}
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2711,8 +2711,14 @@
 
   // Don't check pointers to volatile data. The behavior here is 
implementation-
   // defined.
-  if (Ty->getPointeeType().isVolatileQualified())
-return;
+  if (Ty->isPointerType()) {
+if (Ty->getPointeeType().isVolatileQualified())
+  return;
+  } else {
+// Ty maybe an array type
+if (Ty.isVolatileQualified())
+  return;
+  }
 
   // We need to temorairly remove the assumption so we can insert the
   // sanitizer check before it, else the check will be dropped by 
optimizations.


Index: clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
===
--- clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
+++ clang/test/CodeGen/catch-alignment-assumption-ignorelist.c
@@ -26,3 +26,9 @@
 void *ignore_volatiles(volatile void * x) {
   return __builtin_assume_aligned(x, 1);
 }
+
+// CHECK-LABEL: ignore_volatiles
+void ignore_volatiles_array() {
+  volatile char arr[] = "a";
+  (void)__builtin_assume_aligned(arr, 1);
+}
Index: clang/test/CodeGen/builtin-assume-aligned.c
===
--- clang/test/CodeGen/builtin-assume-aligned.c
+++ clang/test/CodeGen/builtin-assume-aligned.c
@@ -1,6 +1,8 @@
 // NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-unknown-unknown -emit-llvm -o - %s | FileCheck %s
 
+// CHECK: [[TEST7_STR:@.*]] = private unnamed_addr constant [2 x i8] c"a\00", align 1
+
 // CHECK-LABEL: @test1(
 // CHECK-NEXT:  entry:
 // CHECK-NEXT:[[A_ADDR:%.*]] = alloca i32*, align 8
@@ -124,3 +126,10 @@
   a = __builtin_assume_aligned(a, 4294967296);
 return a[0];
 }
+
+// CHECK-LABEL: @test7(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:call void @llvm.assume(i1 true) [ "align"(i8* getelementptr inbounds ([2 x i8], [2 x i8]* [[TEST7_STR]], i64 0, i64 0), i64 1) ]
+void test7(void) {
+  (void) __builtin_assume_aligned("a", 1);
+}
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -2711,8 +2711,14 @@
 
   // Don't check pointers to volatile data. The behavior here is implementation-
   // defined.
-  if (Ty->getPointeeType().isVolatileQualified())
-return;
+  if (Ty->isPointerType()) {
+if (Ty->getPointeeType().isVolatileQualified())
+  return;
+  } else {
+// Ty maybe an array type
+if (Ty.isVolatileQualified())
+  return;
+  }
 
   // We need to temorairly remove the assumption so we can insert the
   // sanitizer c

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
Herald added subscribers: kosarev, pmatos, asb, ormris, wenlei, kerbowa, 
arphaman, steven_wu, hiraditya, sbc100, jvesely.
Herald added a project: All.
aeubanks requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, aheejin.
Herald added projects: clang, LLVM.

Following up from D130374 .

-O1 explicitly tries to not run passes that hurt debuggability too much.
tail-call-elim hurts debuggability, so don't run it under -O1.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131980

Files:
  clang/test/CodeGen/aarch64-ls64-inline-asm.c
  clang/test/CodeGen/aarch64-neon-vcmla.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acge.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acgt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acle.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_aclt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_add.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adda.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_addv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrh.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrw.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_and.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_andv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asr.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfdot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmlalb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmlalt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bic.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brka.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkn.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkpa.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkpb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cadd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clasta-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clasta.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cls.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clz.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpeq.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpge.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpgt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmple.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmplt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpne.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpuo.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnt-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnth.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntp.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntw.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_compact.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvt-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvtnt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_div.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_divr.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dup-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dup.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c
  clang/test/CodeGen/aarch64-sve-in

[PATCH] D130510: Missing tautological compare warnings due to unary operators

[Muhammad Usman Shahid via Phabricator via cfe-commits]

Codesbyusman updated this revision to Diff 453064.
Codesbyusman marked 3 inline comments as not done.
Codesbyusman added a comment.

updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130510

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Analysis/CFG.cpp
  clang/test/Sema/warn-bitwise-compare.c
  clang/test/SemaCXX/warn-bitwise-compare.cpp
  clang/test/SemaCXX/warn-unreachable.cpp

Index: clang/test/SemaCXX/warn-unreachable.cpp
===
--- clang/test/SemaCXX/warn-unreachable.cpp
+++ clang/test/SemaCXX/warn-unreachable.cpp
@@ -396,16 +396,18 @@
   if (y == -1 && y != -1)  // expected-note {{silence}}
 calledFun();// expected-warning {{will never be executed}}
 
-  // TODO: Extend warning to the following code:
-  if (x < -1)
-calledFun();
-  if (x == -1)
-calledFun();
+  if (x == -1)   // expected-note {{silence}}
+calledFun(); // expected-warning {{will never be executed}}
 
-  if (x != -1)
+  if (x != -1)   // expected-note {{silence}}
 calledFun();
   else
+calledFun(); // expected-warning {{will never be executed}}
+
+  // TODO: Extend warning to the following code:
+  if (x < -1)
 calledFun();
+ 
   if (-1 > x)
 calledFun();
   else
Index: clang/test/SemaCXX/warn-bitwise-compare.cpp
===
--- clang/test/SemaCXX/warn-bitwise-compare.cpp
+++ clang/test/SemaCXX/warn-bitwise-compare.cpp
@@ -11,3 +11,15 @@
   bool b4 = !!(x | 5);
   // expected-warning@-1 {{bitwise or with non-zero value always evaluates to true}}
 }
+
+template   // silence
+void foo(int x) {
+bool b1 = (x & sizeof(T)) == 8;
+bool b2 = (x & I) == 8;
+bool b3 = (x & 4) == 8; // expected-warning {{bitwise comparison always evaluates to false}}
+}
+
+void run(int x) {
+foo<4, int>(8); // expected-note {{in instantiation of function template specialization 'foo<4, int>' requested here}}
+}
+
Index: clang/test/Sema/warn-bitwise-compare.c
===
--- clang/test/Sema/warn-bitwise-compare.c
+++ clang/test/Sema/warn-bitwise-compare.c
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -Wall -Wno-unused %s
 
 #define mydefine 2
+#define mydefine2 -2
 
 enum {
   ZERO,
@@ -11,29 +12,66 @@
 void f(int x) {
   if ((8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x & 8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((-8 & x) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & -8) == 4) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((x & 8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((2 & x) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((x & -8) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-2 & x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+ 
   if ((x | 4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
   if ((x | 3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
   if ((5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+
+  if ((x | -4) == 3) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x | -3) != 4) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  if ((-5 | x) != 3) {}  // expected-warning {{bitwise comparison always evaluates to true}}
+  
   if ((x & 0x15) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((x & 0xFFEB) == 0x13) {} // expected-warning {{bitwise comparison always evaluates to false}}
+
   if ((0x23 | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
+  if ((0xFFDD | x) == 0x155){} // expected-warning {{bitwise comparison always evaluates to false}}
 
   if (!!((8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+  if (!!((-8 & x) == 3)) {}  // expected-warning {{bitwise comparison always evaluates to false}}
+
   int y = ((8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((-8 & x) == 3) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to false}}
+  y = ((3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
+  y = ((-3 | x) != 5) ? 1 : 2;  // expected-warning {{bitwise comparison always evaluates to true}}
 
   if ((x & 8) == 8) {}
   if ((x & 8) != 8) {}
   if ((x | 4) == 4) {}
   if ((x | 4) != 4) {}
 
+  if ((-2 & x) != 4) {}
+  if ((x & -8) == -8) {}
+  if ((x & -8) != -8) {}
+  if ((x | -4) == -4) {}
+  if ((x | -4

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I think the correct answer here is instead to prohibit using this on 
array/string literal types in Sema instead of just ignoring the call.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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

[PATCH] D131009: [analyzer] Fixing a bug raising false positives of stack block object leaking under ARC

[Ziqing Luo via Phabricator via cfe-commits]

ziqingluo-90 added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StackAddrEscapeChecker.cpp:313-315
+  // Under ARC, blocks are retained and released automatically:
+  if (isArcManagedBlock(Referred, Ctx))
+return false;

NoQ wrote:
> Aha ok, it sounds like we can no longer be sure that the block is on the 
> stack at this point, did I get it right?
> 
> In this case I think it's more productive to have the block's memory space be 
> `UnknownSpaceRegion` from the start, so that it fell through the memory space 
> check, both here and at other call sites of `isArcManagedBlock()` (so it can 
> be removed), and in any other code that relies on memory spaces (so this 
> mistake is never made again).
//" ..., did I get it right?"//  Yes.

This suggestion makes sense to me.  To my understanding, I need to modify the 
symbolic execution engine to address it.  So shall I do it in a new patch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131009

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

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

In D131979#3726686 , @erichkeane 
wrote:

> I think the correct answer here is instead to prohibit using this on 
> array/string literal types in Sema instead of just ignoring the call.

Thanks for you replay @erichkeane , in 
clang/lib/CodeGen/CodeGenFunction.cpp:2442, Since E = CE->getSubExprAsWritten() 
is used, implicit cast is no longer visible in  
clang::CodeGenCodeGenFunction::emitAlignmentAssumptionCheck

  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai added a comment.

In D131307#3726643 , @erichkeane 
wrote:

> In D131307#3726631 , @smeenai wrote:
>
>> Was it intended that the warning generated here isn't silenced by `-w`, only 
>> by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and 
>> that `-Wno-error` doesn't downgrade the error? See 
>> https://godbolt.org/z/s9qPveTWG for an example.
>
> Yes, we've discussed that on this thread before: Clang's behavior for 
> warnings-as-default-error require explicit suppression of the warning, and 
> isn't effected by global warning/error settings.



In D131307#3726644 , @aaron.ballman 
wrote:

> In D131307#3726631 , @smeenai wrote:
>
>> Was it intended that the warning generated here isn't silenced by `-w`, only 
>> by an explicit `-Wno-enum-constexpr-conversion` (or `-Wno-everythning`), and 
>> that `-Wno-error` doesn't downgrade the error? See 
>> https://godbolt.org/z/s9qPveTWG for an example.
>
> Yes. That is the behavior of warnings which default to an error. The idea is: 
> these aren't really *warnings*, they're errors that we let users downgrade 
> for . So `-w` shouldn't blanket disable them or users will be very 
> surprised when that warning turns into a hard error in a future version of 
> the compiler. So you have to explicitly disable warnings that default to an 
> error. The same is true for `-Wno-error` behavior.

Yup, all of that makes sense; I just missed it earlier. Thank you both :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

[Sanjay Patel via Phabricator via cfe-commits]

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

LGTM
These clang tests are just awful, but I don't have the patience to fix them...




Comment at: llvm/lib/Passes/PassBuilderPipelines.cpp:245
 
 // TODO: Investigate the cost/benefit of tail call elimination on debugging.
 FunctionPassManager

Update this comment? IIUC, we are decisively saying that tail call elim is not 
worth the impact to debuggability for -O1. That way, someone -- possibly me 
again :) -- won't try to unknowingly change this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131980

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

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Are you concerned about tail-call marking, or the recursive call->loop 
transform?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131980

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

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

`getSubExprAsWritten`, as the name suggests, is a query for exploring the 
source code as written; it should generally not be used in CodeGen, which 
should be respecting the semantics of the AST.  If Sema is applying implicit 
conversions that aren't desirable, we should fix that in Sema, if necessary by 
using custom type-checking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

[Arthur Eubanks via Phabricator via cfe-commits]

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

delete comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131980

Files:
  clang/test/CodeGen/aarch64-ls64-inline-asm.c
  clang/test/CodeGen/aarch64-neon-vcmla.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.c
  clang/test/CodeGen/aarch64-sve-acle-__ARM_FEATURE_SVE_VECTOR_OPERATORS.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_abs.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acge.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acgt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_acle.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_aclt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_add.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adda.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_addv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrh.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_adrw.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_and.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_andv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asr.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfdot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmlalb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmlalt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bfmmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_bic.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brka.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkn.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkpa.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_brkpb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cadd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clasta-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clasta.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clastb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cls.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_clz.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmla.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpeq.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpge.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpgt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmple.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmplt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpne.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cmpuo.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnt-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntb.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cnth.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntp.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cntw.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_compact.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create2.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create3.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_create4.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvt-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_cvtnt.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_div.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_divr.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dot.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dup-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dup.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_dupq.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_eor.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_eorv.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_expa.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext-bfloat.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_extb.c
  clang/test/CodeGen/aarch64-sve-intrinsi

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

In D131980#3726750 , @efriedma wrote:

> Are you concerned about tail-call marking, or the recursive call->loop 
> transform?

specifically tail call marking

we have symbolizers that stopped displaying some frames with this change that 
we expect to work under -O1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131980

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

[clang] 941959d - [clang][dataflow] Use llvm::is_contained()

[Dmitri Gribenko via cfe-commits]

Author: Dmitri Gribenko
Date: 2022-08-16T19:59:21+02:00
New Revision: 941959d69de76342fbeebcebd9f0ebdf2f73c77d

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

LOG: [clang][dataflow] Use llvm::is_contained()

Reviewed By: samestep, xazax.hun

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index f64ade34bcb82..9acd993eb25da 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -20,6 +20,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include 
@@ -207,9 +208,7 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
 
 bool Environment::canDescend(unsigned MaxDepth,
  const DeclContext *Callee) const {
-  return CallStack.size() <= MaxDepth &&
- std::find(CallStack.begin(), CallStack.end(), Callee) ==
- CallStack.end();
+  return CallStack.size() <= MaxDepth && !llvm::is_contained(CallStack, 
Callee);
 }
 
 Environment Environment::pushCall(const CallExpr *Call) const {



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

[PATCH] D131975: [clang][dataflow] Use llvm::is_contained()

[Dmitri Gribenko via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG941959d69de7: [clang][dataflow] Use llvm::is_contained() 
(authored by gribozavr).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131975

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


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -20,6 +20,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include 
@@ -207,9 +208,7 @@
 
 bool Environment::canDescend(unsigned MaxDepth,
  const DeclContext *Callee) const {
-  return CallStack.size() <= MaxDepth &&
- std::find(CallStack.begin(), CallStack.end(), Callee) ==
- CallStack.end();
+  return CallStack.size() <= MaxDepth && !llvm::is_contained(CallStack, 
Callee);
 }
 
 Environment Environment::pushCall(const CallExpr *Call) const {


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -20,6 +20,7 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
 #include 
@@ -207,9 +208,7 @@
 
 bool Environment::canDescend(unsigned MaxDepth,
  const DeclContext *Callee) const {
-  return CallStack.size() <= MaxDepth &&
- std::find(CallStack.begin(), CallStack.end(), Callee) ==
- CallStack.end();
+  return CallStack.size() <= MaxDepth && !llvm::is_contained(CallStack, Callee);
 }
 
 Environment Environment::pushCall(const CallExpr *Call) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131979: [clang][UBSan] Fix __builtin_assume_aligned crash

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

Thanks for your suggestion @erichkeane @rjmccall , I will try to do that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131979

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

[PATCH] D85599: [PowerPC] Remove isTerminator for TRAP instruction

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added a comment.
Herald added a project: All.

I think we should handle this similarly to `SITargetLowering::splitKillBlock()`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D85599

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

[PATCH] D129855: [clang][PowerPC] Set lld as clang's default linker for PowerPC Linux

[Quinn Pham via Phabricator via cfe-commits]

quinnp added a comment.

Hi @MaskRay, could you please take a look at @nemanjai's suggestion?

> ...
> So I would prefer that we handle this in the CMake files if @MaskRay doesn't 
> object.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129855

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

[PATCH] D131980: [Passes] Don't run tail-call-elim in -O1

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

If you're specifically concerned about sibcall (call->jmp) optimization in the 
backend, it might be better to adjust the backend to avoid sibcalls at -O1, as 
opposed to messing with optimization passes.  (i.e. make 
-fno-optimize-sibling-calls the default at -O1.)  "tail" markings are useful 
for other purposes, like alias analysis, and I don't really want every 
optimization pass/frontend that might add "tail" markings to worry about the 
optimization level.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131980

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

[PATCH] D131926: [clang-tidy] Fix for bugprone-sizeof-expression PR57167

[Chris Hamilton via Phabricator via cfe-commits]

chrish_ericsson_atx added a comment.

Sounds fair.  I had taken your acceptance of the change as a green light.  :)  
TBH, the acceptance came much faster than I'd expected-- even though this is a 
trivial and low-risk change, I expected it to sit for at least several days.  
I'll plan to wait a few more days before landing it.


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

https://reviews.llvm.org/D131926

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

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131464#3716905 , @MaskRay wrote:

> Sorry, my previous main comment had been written before I introduced 
> `LIT_CLANG_STD_GROUP` in `llvm/utils/lit/lit/llvm/config.py`. The multiple 
> `%clang_cc1` approach actually looks like the following.
> Note the use of `%stdcxx_17-` to make the test future-proof.
> (It is non-trivial to run one `RUN` line multiples times with different 
> `LIT_CLANG_STD_GROUP`. For now I just test locally with different 
> `LIT_CLANG_STD_GROUP`.)
>
>   // RUN: %clang_cc1 %s -fsyntax-only -verify=expected,precxx17 %stdcxx_11-14 
> -fdata-sections -fcolor-diagnostics
>   // RUN: %clang_cc1 %s -fsyntax-only -verify %stdcxx_17- -fdata-sections 
> -fcolor-diagnostics
>   
>   ...
> TypedefAligned4 TA8c = TA8a + TA8b;  // expected-warning {{passing 4-byte 
> aligned argument to 8-byte aligned parameter 'this' of 'operator+' may result 
> in an unaligned pointer access}} \
>  // expected-warning {{passing 4-byte 
> aligned argument to 8-byte aligned parameter 1 of 'operator+' may result in 
> an unaligned pointer access}} \
>  // precxx17-warning {{passing 4-byte 
> aligned argument to 8-byte aligned parameter 'this' of 'StructAligned8' may 
> result in an unaligned pointer access}}

Personally, I like this style. I tend to be a heavy user of `-verify` prefixes 
though, so I might be biased.

> If this is changed to use `#if __cplusplus >= 201703L`, there will be 
> multiple lines with relative line numbers (e.g. `@-2` `@-4`)
>
>   register int ro; // expected-error {{illegal storage class on file-scoped 
> variable}}
>   #if __cplusplus >= 201703L
>   // expected-error@-2 {{ISO C++17 does not allow 'register' storage class 
> specifier}}
>   #elif __cplusplus >= 201103L
>   // expected-warning@-4 {{'register' storage class specifier is deprecated}}
>   #endif

FWIW, you don't have to use relative markers, but that uses an even less common 
idiom of bookmarks. e.g.,

  register int ro; // expected-error {{illegal storage class on file-scoped 
variable}} \
  #bookmark
  #if __cplusplus >= 201703L
  // expected-error@#bookmark {{ISO C++17 does not allow 'register' storage 
class specifier}}
  #elif __cplusplus >= 201103L
  // expected-warning@#bookmark {{'register' storage class specifier is 
deprecated}}
  #endif



> Personally I prefer multiple `%clang_cc1` over `#if`. The first few lines 
> give users a first impression. The dispatch makes it clear the test has 
> different behaviors with the `%stdcxx_*` described dialects.

I tend to have the same opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131464

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

[PATCH] D131985: clang-tidy: strip useless parens from `return` statements

[Oleg Smolsky via Phabricator via cfe-commits]

oleg.smolsky created this revision.
oleg.smolsky added a reviewer: aaron.ballman.
Herald added subscribers: carlosgalvezp, mgorny.
Herald added a project: All.
oleg.smolsky requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

- this adds a new check: `readability-return-with-redundant-parens`

We can find these with the AST matcher. We exclude macros.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131985

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/ReturnWithRedundantParensCheck.cpp
  clang-tools-extra/clang-tidy/readability/ReturnWithRedundantParensCheck.h
  clang-tools-extra/test/clang-tidy/readability-return-with-redundant-parens.cpp

Index: clang-tools-extra/test/clang-tidy/readability-return-with-redundant-parens.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/readability-return-with-redundant-parens.cpp
@@ -0,0 +1,24 @@
+// RUN: %check_clang_tidy %s readability-return-with-redundant-parens %t
+
+int good() {
+  return 1;
+}
+
+int simple1() {
+  return (1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Redundant parens in the 'return' statement [readability-return-with-redundant-parens]
+  // CHECK-FIXES: {{^  }}return 1;{{$}}
+}
+
+int complex1() {
+  int a = 0;
+  return (a + a * (a + a));
+  // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: Redundant parens in the 'return' statement [readability-return-with-redundant-parens]
+  // CHECK-FIXES: {{^  }}return a + a * (a + a);{{$}}
+}
+
+int no_space() {
+  return(1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: Redundant parens in the 'return' statement [readability-return-with-redundant-parens]
+  // CHECK-FIXES: {{^  }}return 1;{{$}}
+}
Index: clang-tools-extra/clang-tidy/readability/ReturnWithRedundantParensCheck.h
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/ReturnWithRedundantParensCheck.h
@@ -0,0 +1,38 @@
+//===--- ReturnWithRedundantParensCheck.h - clang-tidy --*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNBRACKETSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNBRACKETSCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+/// Find and remove redundant parenthesis surrounding returned expression.
+///
+/// Examples:
+/// \code
+///   void f() { return (1); } ==> void f() { return 1; }
+/// \endcode
+class ReturnWithRedundantParensCheck : public ClangTidyCheck {
+public:
+  ReturnWithRedundantParensCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace readability
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_RETURNBRACKETSCHECK_H
Index: clang-tools-extra/clang-tidy/readability/ReturnWithRedundantParensCheck.cpp
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/readability/ReturnWithRedundantParensCheck.cpp
@@ -0,0 +1,69 @@
+//===--- ReturnWithRedundantParensCheck.cpp - clang-tidy *- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "ReturnWithRedundantParensCheck.h"
+#include "clang/Lex/Lexer.h"
+#include 
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace readability {
+
+namespace {
+bool isLocationInMacroExpansion(const SourceManager &SM, SourceLocation Loc) {
+  return SM.isMacroBodyExpansion(Loc) || SM.isMacroArgExpansion(Loc);
+}
+} // namespace
+
+void ReturnWithRedundantParensCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(
+  functionDecl(
+  isDefinition(), returns(unless(voidType())),
+  hasBody(compoundStmt(hasAnySubstatement(returnStmt(has(expr()
+  .bind("return"))),
+  this);
+}
+
+void ReturnWithRedundantParensCheck::check(
+const ast_matchers::MatchFinder::MatchResult &Result) {
+  const auto *Block = Result.Nodes.getNodeAs("return");
+  CompoundStmt::const_reverse_body_iterator Last = Block->body_rbegin

[PATCH] D127233: [CodeGen] Sort llvm.global_ctors by lexical order before emission

[Yuanfang Chen via Phabricator via cfe-commits]

ychen added inline comments.



Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:580
+I = DelayedCXXInitPosition.find(D);
+unsigned LexOrder = I == DelayedCXXInitPosition.end() ? ~0U : I->second;
+AddGlobalCtor(Fn, 65535, LexOrder, COMDATKey);

efriedma wrote:
> This ensures delayed initialization calls are ordered relative to each 
> other... but are they ordered correctly relative to non-delayed 
> initialization calls?  I'm skeptical that using a LexOrder of "~0U" is really 
> correct.  (For example, if you change the variable "b" in your testcase to a 
> struct with a destructor.)
Hmm, That's a great point. I didn't think of that. I'll take a look.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:544
   }
+  llvm::stable_sort(GlobalCtors, [](const Structor &L, const Structor &R) {
+return L.LexOrder < R.LexOrder;

efriedma wrote:
> ychen wrote:
> > rnk wrote:
> > > Please move this sorting into EmitCtorList and apply it to destructors. I 
> > > believe they are currently emitted in source order, and the loader 
> > > executes them in reverse order, so we get the desired reverse source 
> > > order cleanup behavior.
> > I looked into this. They are indeed "currently emitted in source order". 
> > However, if a dtor ever uses an entry in `llvm.global_dtors`, it must have 
> > bailed out of deferred emission here 
> > (https://github.com/llvm/llvm-project/blob/69c09d11f877a35655e285cda96ec0699e385fc9/clang/lib/CodeGen/CodeGenModule.cpp#L3256-L3263),
> >  which is to say, the corresponding ctor is also in lexing order. So the 
> > situation is either the ctor/dtor pair both use lexing order, or there is 
> > no dtor (like `inline int` with an initializer) and the ctor is 
> > deferred/reordered where this patch kicks in.
> > 
> Is MayBeEmittedEagerly always true for variables with destructors?
Yeah, for this and `CodeGenModule::MustBeEmitted`, as long as the destructors 
are not constexpr where init order is not an issue.

https://github.com/llvm/llvm-project/blob/aacf1a9742f714dd432117d82d19a007289c3dee/clang/lib/AST/ASTContext.cpp#L11646-L11648
 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127233

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