date:20220720

[PATCH] D129640: [clang][ASTImporter] Improved handling of functions with auto return type.

2022-07-20 Thread Balázs Kéri via Phabricator via cfe-commits

balazske updated this revision to Diff 446057.
balazske marked 3 inline comments as done.
balazske added a comment.

Small code NFC improvements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129640

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -6320,6 +6320,24 @@
 
 struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {};
 
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithIntegerArgDeclaredInside) {
+  Decl *FromTU = getTuDecl(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+constexpr int X = 1;
+return Tmpl();
+  }
+  )",
+  Lang_CXX14, "input0.cc");
+  FunctionDecl *From = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+
+  FunctionDecl *To = Import(From, Lang_CXX14);
+  EXPECT_TRUE(To);
+  EXPECT_TRUE(isa(To->getReturnType()));
+}
+
 TEST_P(ImportAutoFunctions, ReturnWithTemplateWithStructDeclaredInside1) {
   Decl *FromTU = getTuDecl(
   R"(
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -12,9 +12,9 @@
 //===--===//
 
 #include "clang/AST/ASTImporter.h"
-#include "clang/AST/ASTImporterSharedState.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTDiagnostic.h"
+#include "clang/AST/ASTImporterSharedState.h"
 #include "clang/AST/ASTStructuralEquivalence.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Decl.h"
@@ -34,6 +34,7 @@
 #include "clang/AST/LambdaCapture.h"
 #include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/OperationKinds.h"
+#include "clang/AST/ParentMapContext.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/StmtCXX.h"
 #include "clang/AST/StmtObjC.h"
@@ -58,8 +59,8 @@
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
-#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/ErrorHandling.h"
@@ -3214,9 +3215,12 @@
 }
 
 // Returns true if the given D has a DeclContext up to the TranslationUnitDecl
-// which is equal to the given DC.
+// which is equal to the given DC, or D is equal to DC.
 static bool isAncestorDeclContextOf(const DeclContext *DC, const Decl *D) {
-  const DeclContext *DCi = D->getDeclContext();
+  const DeclContext *DCi = dyn_cast(D);
+  if (!DCi)
+DCi = D->getDeclContext();
+  assert(DCi && "Declaration should have a context");
   while (DCi != D->getTranslationUnitDecl()) {
 if (DCi == DC)
   return true;
@@ -3225,9 +3229,36 @@
   return false;
 }
 
+// Returns true if the statement S has a parent declaration that has a
+// DeclContext that is inside (or equal to) DC. In a specific use case if DC is
+// a FunctionDecl, check if statement S resides in the body of the function.
+static bool isAncestorDeclContextOf(const DeclContext *DC, const Stmt *S) {
+  ParentMapContext &ParentC = DC->getParentASTContext().getParentMapContext();
+  DynTypedNodeList Parents = ParentC.getParents(*S);
+  while (!Parents.empty()) {
+if (const Decl *PD = Parents.begin()->get())
+  return isAncestorDeclContextOf(DC, PD);
+Parents = ParentC.getParents(*Parents.begin());
+  }
+  return false;
+}
+
 static bool hasTypeDeclaredInsideFunction(QualType T, const FunctionDecl *FD) {
   if (T.isNull())
 return false;
+
+  auto CheckTemplateArgument = [FD](const TemplateArgument &Arg) {
+switch (Arg.getKind()) {
+case TemplateArgument::Type:
+  return hasTypeDeclaredInsideFunction(Arg.getAsType(), FD);
+case TemplateArgument::Expression:
+  return isAncestorDeclContextOf(FD, Arg.getAsExpr());
+default:
+  // FIXME: Handle other argument kinds.
+  return false;
+}
+  };
+
   if (const auto *RecordT = T->getAs()) {
 const RecordDecl *RD = RecordT->getDecl();
 assert(RD);
@@ -3236,12 +3267,15 @@
   return true;
 }
 if (const auto *RDTempl = dyn_cast(RD))
-  return llvm::count_if(RDTempl->getTemplateArgs().asArray(),
-[FD](const TemplateArgument &Arg) {
-  return hasTypeDeclaredInsideFunction(
-  Arg.getAsType(), FD);
-});
+  if (llvm::count_if(RDTempl->getTemplateArgs().asArray(),
+ CheckTemplateArgument))
+return true;
+// Note: It is possible that T can be get as both a RecordType and a
+// TemplateSpecializationType.
   }
+  if (const auto *TST = T->getAs(

[PATCH] D129640: [clang][ASTImporter] Improved handling of functions with auto return type.

2022-07-20 Thread Balázs Kéri via Phabricator via cfe-commits

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3269-3273
+// Note: It is possible that T can be get as both a RecordType and a
+// TemplateSpecializationType.
+  }
+  if (const auto *TST = T->getAs()) {
+return llvm::count_if(TST->template_arguments(), CheckTemplateArgument);

martong wrote:
> Is it possible that `T` is both a `RecordType` and a 
> `TemplateSpecializationType` at the same time? From the [[ 
> https://clang.llvm.org/doxygen/classclang_1_1TemplateSpecializationType.html 
> | hierarchy ]] this seems impossible (?)
The type dump shows that a `RecordType` is contained within the 
`TemplateSpecializationType` and it looks like that the get function handles 
this case. When the `else` is added the check of `TemplateSpecializationType` 
is skipped and the template argument expression `X` is not found (only integer 
constant `1`).
```
TemplateSpecializationType 0x23fd8c0 'Tmpl' sugar Tmpl
|-TemplateArgument expr
| `-ConstantExpr 0x23fd798 'int'
|   |-value: Int 1
|   `-ImplicitCastExpr 0x23fd780 'int' 
| `-DeclRefExpr 0x23fd760 'const int' lvalue Var 0x23fd648 'X' 'const int' 
non_odr_use_constant
`-RecordType 0x23fd8a0 'struct Tmpl<1>'
  `-ClassTemplateSpecialization 0x23fd7b8 'Tmpl'
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129640

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

[PATCH] D128465: [llvm] add zstd to `llvm::compression` namespace

2022-07-20 Thread Petr Hosek via Phabricator via cfe-commits

phosek added inline comments.



Comment at: llvm/cmake/config-ix.cmake:149
+  elseif(NOT LLVM_USE_SANITIZER MATCHES "Memory.*")
+find_package(zstd)
+  endif()

Since there's no `Findzstd.cmake` module shipped with CMake, and we don't 
provide in LLVM, we should use the config mode.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128465

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

[PATCH] D130150: [pseudo] Eliminate multiple-specified-types ambiguities in the grammar.

2022-07-20 Thread Sam McCall via Phabricator via cfe-commits

sammccall created this revision.
sammccall added a reviewer: hokein.
Herald added a project: All.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, alextsao1999.
Herald added a project: clang-tools-extra.

Motivating case: `foo bar;` is not a declaration of nothing with `foo` and `bar`
both types.

This is a common and critical ambiguity, and I expect eliminating it in the
grammar is more efficient than using guards (without caching).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130150

Files:
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/decl-specifier-seq.cpp
  clang-tools-extra/pseudo/test/glr.cpp

Index: clang-tools-extra/pseudo/test/glr.cpp
===
--- clang-tools-extra/pseudo/test/glr.cpp
+++ clang-tools-extra/pseudo/test/glr.cpp
@@ -11,12 +11,12 @@
 // CHECK-NEXT: â â   ââunqualified-id~IDENTIFIER := tok[7]
 // CHECK-NEXT: â ââ; := tok[8]
 // CHECK-NEXT: ââstatement~simple-declaration := decl-specifier-seq init-declarator-list ;
-// CHECK-NEXT:   ââdecl-specifier-seq~simple-type-specifier := 
-// CHECK-NEXT:   â ââsimple-type-specifier~type-name := 
+// CHECK-NEXT:   ââdecl-specifier-seq~exclusive-simple-type-specifier := 
+// CHECK-NEXT:   â ââexclusive-simple-type-specifier~type-name := 
 // CHECK-NEXT:   â â ââtype-name~IDENTIFIER := tok[5]
 // CHECK-NEXT:   â â ââtype-name~IDENTIFIER := tok[5]
 // CHECK-NEXT:   â â ââtype-name~IDENTIFIER := tok[5]
-// CHECK-NEXT:   â ââsimple-type-specifier~IDENTIFIER := tok[5]
+// CHECK-NEXT:   â ââexclusive-simple-type-specifier~IDENTIFIER := tok[5]
 // CHECK-NEXT:   ââinit-declarator-list~ptr-declarator := ptr-operator ptr-declarator
 // CHECK-NEXT:   â ââptr-operator~* := tok[6]
 // CHECK-NEXT:   â ââptr-declarator~id-expression =#1
@@ -24,7 +24,7 @@
 }
 
 // CHECK:  3 Ambiguous nodes:
-// CHECK-NEXT: 1 simple-type-specifier
+// CHECK-NEXT: 1 exclusive-simple-type-specifier
 // CHECK-NEXT: 1 statement
 // CHECK-NEXT: 1 type-name
 // CHECK-EMPTY:
Index: clang-tools-extra/pseudo/test/cxx/decl-specifier-seq.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/decl-specifier-seq.cpp
@@ -0,0 +1,34 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --print-forest | FileCheck %s
+// No ambiguity: 'bar' is definitely a variable name.
+const foo volatile bar;
+// CHECK:  simple-declaration
+// CHECK-NEXT: ââdecl-specifier-seq
+// CHECK-NEXT: â ââcombining-decl-specifier-seq~CONST
+// CHECK-NEXT: â ââexclusive-defining-type-specifier~exclusive-simple-type-specifier := 
+// CHECK-NEXT: â â ââexclusive-simple-type-specifier~type-name := 
+// CHECK-NEXT: â â â ââtype-name~IDENTIFIER
+// CHECK-NEXT: â â â ââtype-name~IDENTIFIER
+// CHECK-NEXT: â â â ââtype-name~IDENTIFIER
+// CHECK-NEXT: â â ââexclusive-simple-type-specifier~IDENTIFIER
+// CHECK-NEXT: â ââcombining-decl-specifier-seq~VOLATILE
+// CHECK-NEXT: ââinit-declarator-list~IDENTIFIER
+// CHECK-NEXT: ââ; := tok[4]
+// Ambiguity: though unlikely, we may be declaring nothing.
+unsigned baz;
+// CHECK-NEXT: declaration~simple-declaration := 
+// CHECK-NEXT: ââsimple-declaration := decl-specifier-seq ;
+// CHECK-NEXT: ââdecl-specifier-seq
+// CHECK-NEXT: â â ââcombining-decl-specifier-seq := combining-decl-specifier #1
+// CHECK-NEXT: â â â ââcombining-decl-specifier~UNSIGNED
+// CHECK-NEXT: â â ââexclusive-defining-type-specifier~exclusive-simple-type-specifier
+// CHECK-NEXT: â â   ââexclusive-simple-type-specifier~type-name
+// CHECK-NEXT: â â   â ââtype-name~IDENTIFIER
+// CHECK-NEXT: â â   â ââtype-name~IDENTIFIER
+// CHECK-NEXT: â â   â ââtype-name~IDENTIFIER
+// CHECK-NEXT: â â   ââexclusive-simple-type-specifier~IDENTIFIER
+// CHECK-NEXT: â ââ;
+// CHECK-NEXT: ââsimple-declaration := decl-specifier-seq init-declarator-list ;
+// CHECK-NEXT:   ââdecl-specifier-seq~combining-decl-specifier-seq =#1
+// CHECK-NEXT:   ââinit-declarator-list~IDENTIFIER
+// CHECK-NEXT:   ââ;
+
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -340,17 +340,28 @@
 static_assert-declaration := STATIC_ASSERT ( constant-expression , string-literal ) ;
 empty-declaration := ;
 # dcl.spec
-decl-specifier := storage-class-specifier
-decl-specifier := defining-type-specifier
-decl-specifier := function-specifier
-decl-specifier := FRIEND
-decl-specifier := TYPEDEF
-decl-specifier := CONSTEXPR
-decl-specifier := CONSTEVAL
-decl-specifier := CONSTINIT
-decl-specifier := INLINE
-decl-specifier-seq := decl-

[PATCH] D130150: [pseudo] Eliminate multiple-specified-types ambiguities in the grammar.

2022-07-20 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

I'm not sure that eliminating this in the grammar is the right technique vs 
guards, appreciate your thoughts.

On clangd/AST.cpp, this reduces ambiguities from 1748->1400 (20%).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130150

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

[PATCH] D130041: [clangd] Add decl/def support to SymbolDetails

2022-07-20 Thread Sam McCall via Phabricator via cfe-commits

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/XRefs.cpp:1528
   }
-
+  auto MainFilePath =
+  getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);

kadircet wrote:
> let's just pass the TUPath from ClangdServer into the request. (we should do 
> the same for `locateSymbolAt`, but on a separate change)
driveby: @kadircet any reason not to store the TUPath on ParsedAST? We already 
pass it to ParsedAST::build()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130041

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

[PATCH] D67025: Add .inl as valid C++ header type

2022-07-20 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added a reviewer: sammccall.
nridge added a subscriber: sammccall.
nridge added a comment.

Adding @sammccall as reviewer

This is a simple change that would benefit clangd users (it just came up again 
recently, on Stack Overflow 
)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D130066: [pseudo] Key guards by RuleID, add guards to literals (and 0).

2022-07-20 Thread Sam McCall via Phabricator via cfe-commits

sammccall updated this revision to Diff 446061.
sammccall added a comment.

rebase on function-declarator guards


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130066

Files:
  clang-tools-extra/pseudo/gen/Main.cpp
  clang-tools-extra/pseudo/include/clang-pseudo/Language.h
  clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/lib/grammar/Grammar.cpp
  clang-tools-extra/pseudo/lib/grammar/GrammarBNF.cpp
  clang-tools-extra/pseudo/test/cxx/literals.cpp
  clang-tools-extra/pseudo/test/cxx/mixed-designator.cpp
  clang-tools-extra/pseudo/tool/ClangPseudo.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp
  clang-tools-extra/pseudo/unittests/GrammarTest.cpp

Index: clang-tools-extra/pseudo/unittests/GrammarTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GrammarTest.cpp
+++ clang-tools-extra/pseudo/unittests/GrammarTest.cpp
@@ -102,16 +102,11 @@
 TEST_F(GrammarTest, Annotation) {
   build(R"bnf(
 _ := x
-
-x := y [guard=value]
-y := IDENTIFIER [guard=final]
-
+x := IDENTIFIER [guard]
   )bnf");
-  ASSERT_TRUE(Diags.empty());
-  EXPECT_EQ(G.lookupRule(ruleFor("_")).Guard, 0);
-  EXPECT_GT(G.lookupRule(ruleFor("x")).Guard, 0);
-  EXPECT_GT(G.lookupRule(ruleFor("y")).Guard, 0);
-  EXPECT_NE(G.lookupRule(ruleFor("x")).Guard, G.lookupRule(ruleFor("y")).Guard);
+  ASSERT_THAT(Diags, IsEmpty());
+  EXPECT_FALSE(G.lookupRule(ruleFor("_")).Guarded);
+  EXPECT_TRUE(G.lookupRule(ruleFor("x")).Guarded);
 }
 
 TEST_F(GrammarTest, MangleName) {
Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -631,10 +631,10 @@
   build(R"bnf(
 _ := start
 
-start := IDENTIFIER [guard=TestOnly]
+start := IDENTIFIER [guard]
   )bnf");
   TestLang.Guards.try_emplace(
-  extensionID("TestOnly"),
+  ruleFor("start"),
   [&](llvm::ArrayRef RHS, const TokenStream &Tokens) {
 assert(RHS.size() == 1 &&
RHS.front()->symbol() == tokenSymbol(clang::tok::identifier));
@@ -647,7 +647,7 @@
   const TokenStream &Succeeded = cook(lex(Input, LOptions), LOptions);
   EXPECT_EQ(glrParse({Succeeded, Arena, GSStack}, id("start"), TestLang)
 .dumpRecursive(TestLang.G),
-"[  0, end) start := IDENTIFIER [guard=TestOnly]\n"
+"[  0, end) start := IDENTIFIER [guard]\n"
 "[  0, end) ââIDENTIFIER := tok[0]\n");
 
   Input = "notest";
Index: clang-tools-extra/pseudo/tool/ClangPseudo.cpp
===
--- clang-tools-extra/pseudo/tool/ClangPseudo.cpp
+++ clang-tools-extra/pseudo/tool/ClangPseudo.cpp
@@ -45,6 +45,8 @@
 desc("Strip directives and select conditional sections"));
 static opt PrintStatistics("print-statistics", desc("Print GLR parser statistics"));
 static opt PrintForest("print-forest", desc("Print parse forest"));
+static opt ForestAbbrev("forest-abbrev", desc("Abbreviate parse forest"),
+  init(true));
 static opt HTMLForest("html-forest",
desc("output file for HTML forest"));
 static opt StartSymbol("start-symbol",
@@ -153,7 +155,7 @@
 glrParse(clang::pseudo::ParseParams{*ParseableStream, Arena, GSS},
  *StartSymID, Lang);
 if (PrintForest)
-  llvm::outs() << Root.dumpRecursive(Lang.G, /*Abbreviated=*/true);
+  llvm::outs() << Root.dumpRecursive(Lang.G, /*Abbreviated=*/ForestAbbrev);
 
 if (HTMLForest.getNumOccurrences()) {
   std::error_code EC;
Index: clang-tools-extra/pseudo/test/cxx/mixed-designator.cpp
===
--- clang-tools-extra/pseudo/test/cxx/mixed-designator.cpp
+++ clang-tools-extra/pseudo/test/cxx/mixed-designator.cpp
@@ -5,16 +5,16 @@
 // CHECK-NEXT: ââ{ := tok[3]
 // CHECK-NEXT: ââinitializer-list
 // CHECK-NEXT: â ââinitializer-list
-// CHECK-NEXT: â â ââinitializer-list~literal
-// CHECK:  â â ââ, := tok[5]
+// CHECK-NEXT: â â ââinitializer-list~NUMERIC_CONSTANT
+// CHECK-NEXT: â â ââ, := tok[5]
 // CHECK-NEXT: â â ââinitializer-list-item
 // CHECK-NEXT: â â   ââdesignator
 // CHECK-NEXT: â â   â ââ. := tok[6]
 // CHECK-NEXT: â â   â ââIDENTIFIER := tok[7]
 // CHECK-NEXT: â â   ââbrace-or-equal-initializer
 // CHECK-NEXT: â â ââ= := tok[8]
-// CHECK-NEXT: â â ââinitializer-clause~literal
-// CHECK:  â ââ, := tok[10]
+// CHECK-NEXT: â â ââinitializer-clause~NUM

[PATCH] D129573: [clang] add a diagnostic note 'while loop outside functions' at global scope

2022-07-20 Thread YingChi Long via Phabricator via cfe-commits

inclyc added a comment.

Is the build error related to this patch?


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

https://reviews.llvm.org/D129573

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

[PATCH] D130138: [modules] Replace `-Wauto-import` with `-Rmodule-include-translation`.

2022-07-20 Thread Iain Sandoe via Phabricator via cfe-commits

iains added a subscriber: MaskRay.
iains added a comment.

makes sense to me...

I guess the name looks long at first, but it's specific (I find that 
easy-to-remember flag names are more important than short-to-type ones, but 
maybe that's just me) - I wonder if @MaskRay has any comments on the flag name,

For the record:
GCC uses : -flang-info-include-translate (-flang for flag-language, rather than 
'flang', of course).
(which is a note, i.e. similar idea to the remark)

BTW.. (not for this patch, of course)
GCC also has 
info-include-translate-not
Note #include directives not translated to import declarations, and not known 
to be textual.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130138

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

[PATCH] D130150: [pseudo] Eliminate multiple-specified-types ambiguities in the grammar.

2022-07-20 Thread Sam McCall via Phabricator via cfe-commits

sammccall planned changes to this revision.
sammccall added a comment.

As discussed offline:

We agree on the goal of disallowing duplicate types in decl-specifier-seq, but 
the grammar changes are too intrusive if a guard-based approach can work 
instead.
The concern with guards on `seq` nodes is duplicate work (a tree walk at every 
level).
Since ForestNodes are immutable and reasonable in number we can introduce 
fairly cheap caching, e.g. of whether a decl-specifier[-seq] contains an 
exclusive type definition. This is a reusable building block for guards etc, so 
we should build it into the contract.

The other idea I was playing with of guarding `simple-declaration := 
decl-specifier-seq /*no declarators*/ ;` by requiring the decl-specifier-seq to 
declare a type (disallow "declaration declares nothing") is inferior as a) the 
code is technically legal, b) it doesn't address `std::string x;` misparsing as 
`Type{std} Type{::string} Declarator{x};`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130150

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

[PATCH] D129573: [clang] add a diagnostic note 'while loop outside functions' at global scope

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added a comment.

In D129573#3664905 , @inclyc wrote:

> Is the build error related to this patch?

I don't think so, it's not even close to what you are changing.


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

https://reviews.llvm.org/D129573

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

[PATCH] D129640: [clang][ASTImporter] Improved handling of functions with auto return type.

2022-07-20 Thread Gabor Marton via Phabricator via cfe-commits

martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:3269-3273
+// Note: It is possible that T can be get as both a RecordType and a
+// TemplateSpecializationType.
+  }
+  if (const auto *TST = T->getAs()) {
+return llvm::count_if(TST->template_arguments(), CheckTemplateArgument);

balazske wrote:
> martong wrote:
> > Is it possible that `T` is both a `RecordType` and a 
> > `TemplateSpecializationType` at the same time? From the [[ 
> > https://clang.llvm.org/doxygen/classclang_1_1TemplateSpecializationType.html
> >  | hierarchy ]] this seems impossible (?)
> The type dump shows that a `RecordType` is contained within the 
> `TemplateSpecializationType` and it looks like that the get function handles 
> this case. When the `else` is added the check of `TemplateSpecializationType` 
> is skipped and the template argument expression `X` is not found (only 
> integer constant `1`).
> ```
> TemplateSpecializationType 0x23fd8c0 'Tmpl' sugar Tmpl
> |-TemplateArgument expr
> | `-ConstantExpr 0x23fd798 'int'
> |   |-value: Int 1
> |   `-ImplicitCastExpr 0x23fd780 'int' 
> | `-DeclRefExpr 0x23fd760 'const int' lvalue Var 0x23fd648 'X' 'const 
> int' non_odr_use_constant
> `-RecordType 0x23fd8a0 'struct Tmpl<1>'
>   `-ClassTemplateSpecialization 0x23fd7b8 'Tmpl'
> ```
Okay. I've dug deeper to understand how that is working. So, in the 
`TemplateSpecializationType` 'Tmpl' is actually a "sugar" to the 
`RecordType` 'struct Tmpl<1>'. 
And `getAs` will return with the desugared type:
```
  // If this is a typedef for the type, strip the typedef off without
  // losing all typedef information.
  return cast(getUnqualifiedDesugaredType());
``` 
Thus, to answer my own question,  it is possible that T is both a RecordType 
and a TemplateSpecializationType at the same time!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129640

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

[clang] 5a4033c - update-test-checks: safely handle tests with #if's

2022-07-20 Thread Nicolai Hähnle via cfe-commits


Author: Nicolai Hähnle
Date: 2022-07-20T11:23:49+02:00
New Revision: 5a4033c36716de0cee75eb28b95cce44ae239cd9

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

LOG: update-test-checks: safely handle tests with #if's

There is at least one Clang test (clang/test/CodeGen/arm_acle.c) which
has functions guarded by #if's that cause those functions to be compiled
only for a subset of RUN lines.

This results in a case where one RUN line has a body for the function
and another doesn't. Treat this case as a conflict for any prefixes that
the two RUN lines have in common.

This change exposed a bug where functions with '$' in the name weren't
properly recognized in ARM assembly (despite there being a test case
that was supposed to catch the problem!). This bug is fixed as well.

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

Added: 
clang/test/utils/update_cc_test_checks/Inputs/ifdef.c
clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
clang/test/utils/update_cc_test_checks/ifdef.test

Modified: 

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll

llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected
llvm/utils/UpdateTestChecks/asm.py
llvm/utils/UpdateTestChecks/common.py
llvm/utils/update_analyze_test_checks.py
llvm/utils/update_cc_test_checks.py
llvm/utils/update_llc_test_checks.py
llvm/utils/update_test_checks.py

Removed: 




diff  --git a/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c 
b/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c
new file mode 100644
index 0..79d73a2de4863
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -DFOO | 
FileCheck -check-prefixes=CHECK,FOO %s
+
+#ifdef FOO
+int foo() {
+  return 1;
+}
+#endif
+
+int bar() {
+  return 2;
+}

diff  --git a/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected 
b/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
new file mode 100644
index 0..c1fb72db9339e
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
@@ -0,0 +1,21 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK %s
+// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -DFOO | 
FileCheck -check-prefixes=CHECK,FOO %s
+
+#ifdef FOO
+// FOO-LABEL: @foo(
+// FOO-NEXT:  entry:
+// FOO-NEXT:ret i32 1
+//
+int foo() {
+  return 1;
+}
+#endif
+
+// CHECK-LABEL: @bar(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:ret i32 2
+//
+int bar() {
+  return 2;
+}

diff  --git a/clang/test/utils/update_cc_test_checks/ifdef.test 
b/clang/test/utils/update_cc_test_checks/ifdef.test
new file mode 100644
index 0..6e4c6319a31c9
--- /dev/null
+++ b/clang/test/utils/update_cc_test_checks/ifdef.test
@@ -0,0 +1,8 @@
+## Test that functions that are only compiled in a subset of RUN lines are
+## handled correctly
+
+# RUN: cp %S/Inputs/ifdef.c %t.c && %update_cc_test_checks %t.c
+# RUN: 
diff  -u %S/Inputs/ifdef.c.expected %t.c
+## Check that re-running update_cc_test_checks doesn't change the output
+# RUN: %update_cc_test_checks %t.c
+# RUN: 
diff  -u %S/Inputs/ifdef.c.expected %t.c

diff  --git 
a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll
 
b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll
index fa03d8cc51cd9..5f3edb6edf478 100644
--- 
a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll
+++ 
b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll
@@ -1,9 +1,8 @@
 ; Check that we accept functions with '$' in the name.
-; TODO: This is not handled correcly on 32bit ARM and needs to be fixed.
 
-; RUN: llc -mtriple=armv7-unknown-linux < %s | FileCheck --prefix=LINUX %s
-; RUN: llc -mtriple=armv7-apple-darwin < %s | FileCheck --prefix=DARWIN %s
-; RUN: llc -mtriple=armv7-apple-ios < %s | FileCheck --prefix=IOS %s
+; RUN: llc -mtriple=armv7-unknown-linux < %s | FileCheck %s
+; RUN: llc -mtriple=armv7-apple-darwin < %s | FileCheck %s
+; RUN: llc -mtriple=armv7-apple-ios < %s | FileCheck %s
 
 define hidden i32 @"_Z54bar$ompvariant$bar"() {
 entry:

diff  --git 
a/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected
 
b/llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected
i

[PATCH] D130089: update-test-checks: safely handle tests with #if's

2022-07-20 Thread Nicolai Hähnle via Phabricator via cfe-commits

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
nhaehnle marked an inline comment as done.
Closed by commit rG5a4033c36716: update-test-checks: safely handle tests with 
#if's (authored by nhaehnle).

Changed prior to commit:
  https://reviews.llvm.org/D130089?vs=445817&id=446085#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130089

Files:
  clang/test/utils/update_cc_test_checks/Inputs/ifdef.c
  clang/test/utils/update_cc_test_checks/Inputs/ifdef.c.expected
  clang/test/utils/update_cc_test_checks/ifdef.test
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll
  
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll.expected
  llvm/utils/UpdateTestChecks/asm.py
  llvm/utils/UpdateTestChecks/common.py
  llvm/utils/update_analyze_test_checks.py
  llvm/utils/update_cc_test_checks.py
  llvm/utils/update_llc_test_checks.py
  llvm/utils/update_test_checks.py

Index: llvm/utils/update_test_checks.py
===
--- llvm/utils/update_test_checks.py
+++ llvm/utils/update_test_checks.py
@@ -120,6 +120,7 @@
verbose=ti.args.verbose)
   builder.process_run_line(common.OPT_FUNCTION_RE, common.scrub_body,
   raw_tool_output, prefixes, False)
+  builder.processed_prefixes(prefixes)
 
 func_dict = builder.finish_and_get_func_dict()
 is_in_function = False
Index: llvm/utils/update_llc_test_checks.py
===
--- llvm/utils/update_llc_test_checks.py
+++ llvm/utils/update_llc_test_checks.py
@@ -137,6 +137,7 @@
 
   scrubber, function_re = output_type.get_run_handler(triple)
   builder.process_run_line(function_re, scrubber, raw_tool_output, prefixes, True)
+  builder.processed_prefixes(prefixes)
 
 func_dict = builder.finish_and_get_func_dict()
 global_vars_seen_dict = {}
Index: llvm/utils/update_cc_test_checks.py
===
--- llvm/utils/update_cc_test_checks.py
+++ llvm/utils/update_cc_test_checks.py
@@ -214,6 +214,7 @@
 builder.process_run_line(
 common.OPT_FUNCTION_RE, common.scrub_body, raw_tool_output,
 prefixes, False)
+builder.processed_prefixes(prefixes)
   else:
 print('The clang command line should include -emit-llvm as asm tests '
   'are discouraged in Clang testsuite.', file=sys.stderr)
Index: llvm/utils/update_analyze_test_checks.py
===
--- llvm/utils/update_analyze_test_checks.py
+++ llvm/utils/update_analyze_test_checks.py
@@ -124,6 +124,8 @@
 common.warn('Don\'t know how to deal with this output')
 continue
 
+  builder.processed_prefixes(prefixes)
+
 func_dict = builder.finish_and_get_func_dict()
 is_in_function = False
 is_in_function_start = False
Index: llvm/utils/UpdateTestChecks/common.py
===
--- llvm/utils/UpdateTestChecks/common.py
+++ llvm/utils/UpdateTestChecks/common.py
@@ -496,6 +496,7 @@
 self._func_dict = {}
 self._func_order = {}
 self._global_var_dict = {}
+self._processed_prefixes = set()
 for tuple in run_list:
   for prefix in tuple[0]:
 self._func_dict.update({prefix:dict()})
@@ -584,30 +585,43 @@
scrubbed_body)
 
 if func in self._func_dict[prefix]:
-  if (self._func_dict[prefix][func] is None or
-  str(self._func_dict[prefix][func]) != scrubbed_body or
-  self._func_dict[prefix][func].args_and_sig != args_and_sig or
-  self._func_dict[prefix][func].attrs != attrs):
-if (self._func_dict[prefix][func] is not None and
-self._func_dict[prefix][func].is_same_except_arg_names(
+  if (self._func_dict[prefix][func] is not None and
+  (str(self._func_dict[prefix][func]) != scrubbed_body or
+   self._func_dict[prefix][func].args_and_sig != args_and_sig or
+   self._func_dict[prefix][func].attrs != attrs)):
+if self._func_dict[prefix][func].is_same_except_arg_names(
 scrubbed_extra,
 args_and_sig,
 attrs,
-is_backend)):
+is_backend):
   self._func_dict[prefix][func].scrub = scrubbed_extra
   self._func_dict[prefix][func].args_and_sig = args_and_sig
-  continue
 else:
   # This means a previous RUN line produced a body for this function
   # that is different from the one produced by this current RUN line,
   # so the body can't be common a

[PATCH] D130089: update-test-checks: safely handle tests with #if's

2022-07-20 Thread Nicolai Hähnle via Phabricator via cfe-commits

nhaehnle added inline comments.



Comment at: 
llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll:2
 ; Check that we accept functions with '$' in the name.
 ; TODO: This is not handled correcly on 32bit ARM and needs to be fixed.
 

arichardson wrote:
> Remove this TODO?
Ok, will do before I commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130089

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

[PATCH] D130091: [clang][analyzer] Added partial wide character support to CStringChecker

2022-07-20 Thread Gabor Marton via Phabricator via cfe-commits

martong added a comment.

Nice improvement and the tests are meaningful!

> clang/test/Analysis/cstring.c

Hadn't we have already a test file for this checker? What about `string.c` and 
`bstring.c`? You might have added redundant test cases in the new test file.




Comment at: clang/test/Analysis/cstring.c:36
+  char buffer[32];
+  // FIXME: This should work with 'str' instead of 'str1'
+  const char str1[] = "Hello world";

Could you please elaborate why this does not work with `str`?



Comment at: clang/test/Analysis/cstring.c:68-69
+  char buffer[32];
+  const char str1[] = "Hello\0world";
+  clang_analyzer_eval(strlen(str1) == 11); // expected-warning {{TRUE}}
+}

I think, this would deserve a FIXME comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130091

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

[PATCH] D129706: [NFCI][clang-tidy] Reimplement GlobList without relying on Regex.

2022-07-20 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 446097.
njames93 added a comment.

Improve handling of consecutive wild cards.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129706

Files:
  clang-tools-extra/clang-tidy/GlobList.cpp
  clang-tools-extra/clang-tidy/GlobList.h
  clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
===
--- clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
+++ clang-tools-extra/unittests/clang-tidy/GlobListTest.cpp
@@ -16,6 +16,13 @@
   EXPECT_FALSE(Filter.contains("aaa"));
 }
 
+TYPED_TEST(GlobListTest, NotEmpty) {
+  TypeParam Filter("*,-");
+
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_TRUE(Filter.contains("aaa"));
+}
+
 TYPED_TEST(GlobListTest, Nothing) {
   TypeParam Filter("-*");
 
@@ -57,6 +64,51 @@
   EXPECT_FALSE(Filter.contains(""));
 }
 
+TYPED_TEST(GlobListTest, PreAndPostFixPattern) {
+  TypeParam Filter("a*b");
+
+  EXPECT_TRUE(Filter.contains("ab"));
+  EXPECT_TRUE(Filter.contains("aMiddleb"));
+  EXPECT_FALSE(Filter.contains("aba"));
+  EXPECT_FALSE(Filter.contains("bab"));
+}
+
+TYPED_TEST(GlobListTest, DoubleWildcard) {
+  {
+TypeParam Filter("a**b");
+
+EXPECT_TRUE(Filter.isEquivalentTo("a*b"));
+EXPECT_TRUE(Filter.isEquivalentTo("a***b"));
+
+EXPECT_TRUE(Filter.contains("ab"));
+EXPECT_TRUE(Filter.contains("aMiddleb"));
+EXPECT_FALSE(Filter.contains("aba"));
+EXPECT_FALSE(Filter.contains("bab"));
+  }
+  {
+TypeParam Filter("a**");
+
+EXPECT_TRUE(Filter.isEquivalentTo("a*"));
+EXPECT_TRUE(Filter.isEquivalentTo("a***"));
+
+EXPECT_TRUE(Filter.contains("ab"));
+EXPECT_TRUE(Filter.contains("aMiddleb"));
+EXPECT_TRUE(Filter.contains("aba"));
+EXPECT_FALSE(Filter.contains("bab"));
+  }
+  {
+TypeParam Filter("**b");
+
+EXPECT_TRUE(Filter.isEquivalentTo("*b"));
+EXPECT_TRUE(Filter.isEquivalentTo("***b"));
+
+EXPECT_TRUE(Filter.contains("ab"));
+EXPECT_TRUE(Filter.contains("aMiddleb"));
+EXPECT_FALSE(Filter.contains("aba"));
+EXPECT_TRUE(Filter.contains("bab"));
+  }
+}
+
 TYPED_TEST(GlobListTest, PatternPriority) {
   // The last glob that matches the string decides whether that string is
   // included or excluded.
@@ -107,6 +159,9 @@
 TYPED_TEST(GlobListTest, NewlineCharactersAsSeparators) {
   TypeParam Filter("a*  \n b,\n-c*,dd");
 
+  // FIXME: Define this behaviour, There are 2 seperator characters next to each
+  // other (,\n), Technically that should be represented as containing an empty
+  // item.
   EXPECT_FALSE(Filter.contains(""));
   EXPECT_TRUE(Filter.contains("aaa"));
   EXPECT_TRUE(Filter.contains("b"));
@@ -117,5 +172,63 @@
   EXPECT_FALSE(Filter.contains("ddd"));
 }
 
+TYPED_TEST(GlobListTest, IgnoreNegativeMatches) {
+  TypeParam Filter("a,-a,b*,-b*,-*", false);
+
+  EXPECT_FALSE(Filter.contains(""));
+  EXPECT_TRUE(Filter.contains("a"));
+  EXPECT_TRUE(Filter.contains("b"));
+  EXPECT_TRUE(Filter.contains("bar"));
+}
+
+TYPED_TEST(GlobListTest, EscapeNegatives) {
+  TypeParam Filter("\\-a,-a");
+
+  std::string Canonicalized;
+  llvm::raw_string_ostream Str(Canonicalized);
+  Filter.print(Str);
+  Str.flush();
+
+  // Ensure negatives are printed correctly.
+  EXPECT_EQ("\\-a,-a", Canonicalized);
+  EXPECT_TRUE(Filter.isEquivalentTo("\\-a,-a"));
+
+  EXPECT_TRUE(Filter.contains("-a"));
+  EXPECT_FALSE(Filter.contains("a"));
+}
+
+void checkCanonicalize(StringRef Glob, StringRef Expected) {
+  GlobList Orig(Glob);
+  std::string Canonicalized;
+  llvm::raw_string_ostream Str(Canonicalized);
+  Orig.print(Str);
+  Str.flush();
+  EXPECT_EQ(Canonicalized, Expected)
+  << "While canonicalizing glob: \"" << Glob << "\"";
+  EXPECT_TRUE(Orig.isEquivalentTo(Expected))
+  << "While canonicalizing glob: \"" << Glob << "\"";
+  GlobList Canonical(Canonicalized);
+  EXPECT_TRUE(Orig.isEquivalentTo(Canonical))
+  << "While canonicalizing glob: \"" << Glob << "\"";
+  EXPECT_TRUE(Canonical.isEquivalentTo(Glob))
+  << "While canonicalizing glob: \"" << Glob << "\"";
+}
+
+TEST(GlobListTest, Printing) {
+  // A positive wildcard is always printed with any previous items removed.
+  // checkCanonicalize(" *", "*");
+  // checkCanonicalize(" * ,   -AAA ", "*,-AAA");
+  checkCanonicalize("Unused , *, -A*", "*,-A*");
+  // A negative wildcard is removed if there is nothing else after it.
+  checkCanonicalize("-* ", "-*");
+  checkCanonicalize("-*, AAA  ", "AAA");
+  // The canonical representation uses commas as the seperators instead of
+  // newlines.
+  checkCanonicalize("Unused\n-*\nRest\n*Others", "Rest,*Others");
+  // Consecutive wildcards are treated as being one wildcard.
+  checkCanonicalize("Unused,**", "*");
+  checkCanonicalize("Unused,-**,Used", "Used");
+}
+
 } // namespace tidy
 } // namespace clang
Index: clang-tools-extra/clan

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

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added a comment.

@JDevlieghere @teemperor ping

Can you please verify that everything works with LLDB, we just changed how 
those types are printed?


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] D112374: [clang] Implement ElaboratedType sugaring for types written bare

2022-07-20 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

In D112374#3657640 , @mizvekov wrote:

> In general, I would not expect external tools to care about the shape of the 
> AST. I would expect the type API would be used in a way where we ignore a 
> type sugar node we have no reason to acknowledge.
> Ie you query if some type is a (possible sugar to) X, and you would either 
> get X or nothing. The type sugar over it would just be skipped over and you 
> would have no reason to know what was in there or what shape it had.

I'm afraid your expectations are wrong, and not by a little bit :-)

I totally agree this is the best way to the use the AST: understanding what you 
want to depend on and what groups of differences (e.g. sugar) to ignore, and 
writing code that expresses that intent.

However this is empirically not how lots of downstream (and plenty of in-tree) 
code is written, because:

- it requires a deep understanding of the "philosophy" of the AST to understand 
where extensions are possible in future
- many people write AST traversals using matchers, which constrains and 
obscures exactly what's being matched
- many people write AST trawling code based on AST dumps of examples, rather 
than a first-principles approach
- it is difficult and tedious to test
- people are often only as careful as they're incentivized to be

I've seen plenty of (useful) out-of-tree tidy checks written by people fuzzy on 
the difference between a Type and a TypeLoc, or what sugar is. Clang makes it 
(almost) easy to write tools but hard to write robust tools.

All of this is to say I like this change & appreciate how willing you are to 
help out-of-tree tools (which is best-effort), but I expect a lot of churn 
downstream. (And LLVM has a clear policy that that's OK).

(BTW, last time I landed such a change, investigating the LLDB tests was indeed 
the most difficult part, and I'm not even on windows. Running a linux VM of 
some sort might be your best bet, unfortunately)

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] D129864: [Flang] Generate documentation for compiler flags

2022-07-20 Thread Andrzej Warzynski via Phabricator via cfe-commits

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

The change in ClangOptionDocEmitter.cpp is required for Flang as it heavily 
relies on these "include" flags defined in Options.td 
.
 This won't affect Clang as it does not use `IncludeFlags` in 
ClangOptionsDocs.td 
.
 If Clang devs were to use them in "ClangOptionsDocs.td", the required hooks in 
ClangOptionDocEmitter.cpp will already be there :)

LGTM, thanks for working on this! Give it another day before merging - in case 
Clang folks would like to chime in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129864

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

[PATCH] D109977: LLVM Driver Multicall tool

2022-07-20 Thread Michał Górny via Phabricator via cfe-commits

mgorny added a comment.

Ok, I've made D130158  as an absolutely 
minimal change that seems to fix it for me. Basically, I'm skipping all the 
driver-related logic that's not strictly necessary when 
`LLVM_TOOL_LLVM_DRIVER_BUILD` is off. The driver remains broken when dylib is 
used but at least it shouldn't affect people who don't enable it now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109977

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

[PATCH] D67025: Add .inl as valid C++ header type

2022-07-20 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

> This seems to be a common practice.

FWIW, I did a search over github repos and this file extension is used, but I 
would hardly call it a common practice (compared to other extensions we don't 
support, like `.inc`). It does seem to be used as a header-like file, 
predominately, but I'm not really certain whether there is sufficient evidence 
that we should treat this extension as always being a header file given that 
it's not a super common extension (it doesn't help that I've never seen that 
extension used in the wild before; today was the first time I learned of it). 
My understanding is that we're usually pretty conservative with this list, and 
this is why we have the `-x` option so that users can specify a specific 
language mode to use. Is there a reason that option does not suffice for clangd?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D67025: Add .inl as valid C++ header type

2022-07-20 Thread Wasim Abbas via Phabricator via cfe-commits

abbaswasim added a comment.

> FWIW, I did a search over github repos and this file extension is used, but I 
> would hardly call it a common practice

Fair enough. what would be sufficient evidence?

I think in general whether its `.inl` or any other extension thats used to 
separate template implementations out of a header needs a solution. I have 
started using `.hh` as a workaround but it not being a complete translation 
unit is still a problem. And clangd also complains about recursive includes in 
the .hh file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D67025: Add .inl as valid C++ header type

2022-07-20 Thread Wasim Abbas via Phabricator via cfe-commits

abbaswasim added a comment.

Also I couldn't make the `-x` option to work with `lsp-mode` but that's not 
`clangd`s problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D130066: [pseudo] Key guards by RuleID, add guards to literals (and 0).

2022-07-20 Thread Haojian Wu via Phabricator via cfe-commits

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

it looks good.




Comment at: clang-tools-extra/pseudo/gen/Main.cpp:102
   llvm::StringRef Name = G.table().AttributeValues[EID];
-  assert(!Name.empty());
   Out.os() << llvm::formatv("EXTENSION({0}, {1})\n", Name, EID);

I think this invariant is still true even in this patch, ExtensionID for empty 
attribute value is 0, and we start from 1 in the loop.



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/grammar/Grammar.h:31
 //
 //contextual-override := IDENTIFIER [guard=Override]
 //

nit: update the stale comment.



Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:27
 
-bool guardOverride(llvm::ArrayRef RHS,
-   const TokenStream &Tokens) {
-  assert(RHS.size() == 1 &&
- RHS.front()->symbol() == tokenSymbol(clang::tok::identifier));
-  return Tokens.tokens()[RHS.front()->startTokenIndex()].text() == "override";
-}
-bool guardFinal(llvm::ArrayRef RHS,
-const TokenStream &Tokens) {
-  assert(RHS.size() == 1 &&
- RHS.front()->symbol() == tokenSymbol(clang::tok::identifier));
-  return Tokens.tokens()[RHS.front()->startTokenIndex()].text() == "final";
-}
-bool guardModule(llvm::ArrayRef RHS,
- const TokenStream &Tokens) {
-  return Tokens.tokens()[RHS.front()->startTokenIndex()].text() == "module";
+bool isStringUD(const Token &Tok) { return !Tok.text().endswith("\""); }
+bool isCharUD(const Token &Tok) { return !Tok.text().endswith("'"); }

nit: not sure the abbreviation of UD is a clearer, took me a while to 
understand it is for UserDefined, add a comment?



Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:156
 
 llvm::DenseMap buildGuards() {
+#define TOKEN_GUARD(kind, cond)
\

we might probably to consider to split it out from the CXX.cpp at some point in 
the future. I think currently it is fine.



Comment at: clang-tools-extra/pseudo/lib/cxx/CXX.cpp:171
+  {(RuleID)Rule::non_function_declarator_0declarator,
+   SYMBOL_GUARD(declarator, !isFunctionDeclarator(&N))},
+  {(RuleID)Rule::contextual_override_0identifier,

nit: add a blank line, I think function-declarator guard is different than the 
contextual-sensitive guard.



Comment at: clang-tools-extra/pseudo/tool/ClangPseudo.cpp:48
 static opt PrintForest("print-forest", desc("Print parse forest"));
+static opt ForestAbbrev("forest-abbrev", desc("Abbreviate parse forest"),
+  init(true));

I like this flag -- I have been wanted for several times.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130066

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

[PATCH] D67025: Add .inl as valid C++ header type

2022-07-20 Thread Aaron Ballman via Phabricator via cfe-commits

aaron.ballman added a comment.

In D67025#3665301 , @abbaswasim wrote:

>> FWIW, I did a search over github repos and this file extension is used, but 
>> I would hardly call it a common practice
>
> Fair enough. what would be sufficient evidence?

I'm not certain we have any sort of agreed-upon measure for it. Personally, I'd 
want to see evidence that several other programming tools also treat the 
extension the same way. I notice that MSVC's file open dialog considers .inl to 
be a "Visual C++ File" and its save dialog considers .inl to be a  "C++ Header 
File", so that's a good sign. I also notice that TextPad also considers .inl to 
be a C/C++ file (doesn't distinguish between language or what kind of file 
though).  Does XCode have something similar? Notepad++ or other text editors? 
If so, I think that qualifies as sufficient evidence.

> I think in general whether its `.inl` or any other extension thats used to 
> separate template implementations out of a header needs a solution. I have 
> started using `.hh` as a workaround but it not being a complete translation 
> unit is still a problem. And clangd also complains about recursive includes 
> in the .hh file.

I've seen: .inc, .def, .hh,, and .H all within the past few years but in each 
case, it meant roughly the same thing "source that gets included but isn't 
intended as a stand-alone header file" (though I've certainly seen .H and .hh 
used to mean "C++ header file" as opposed to "C header file").

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D67025: Add .inl as valid C++ header type

2022-07-20 Thread Wasim Abbas via Phabricator via cfe-commits

abbaswasim added a comment.

AFAICT Xcode understands and reads it as c++ header file.  It highlights it 
fine and icon is "h+".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D67025: Add .inl as valid C++ header type

2022-07-20 Thread Sam McCall via Phabricator via cfe-commits

sammccall added a comment.

(sorry i thought i sent these comments earlier)

This is a change to clang rather than clangd, so it affects the behavior of the 
compiler and other clang-based tools.
While it might do something useful in clangd, I'm skeptical of its general 
applicability because with this extension we do not expect that such files will 
be parseable.

I think the evidence you've found is that editors (e.g. clangd!) should handle 
these files, but not necessarily compilers.

> one has to add #include "header.hpp" at the top of .inl files to make it work 
> but thats not a big deal.

It may not be a big burden to add it to code that you own, but making the 
compiler misinterpret other people's code where this #include isn't present can 
be a problem.

I think ideally we'd express this behavior at the clangd level instead. 
Something like "we suspect this file contains $LANGUAGE, so if the compile 
flags/extension don't imply anything particular, treat it that way".
As well as being less intrusive, this is a more general, useful feature:

- LSP allows the editor to specify the language, typically based in editor UI, 
so this would give the user control over whether such files were treated as 
C/C++/ObjC/ObjC++ etc rather than hard-coding C++. (We can still have a 
default).
- this needn't be specific to suffix ".inl". LLVM uses ".inc", I'm sure other 
codebases use other extensions.

(And the reason i never sent the comment is i couldn't come to a conclusion on 
the best way to do this inside clangd. Needs some more thought...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D129573: [clang] add a diagnostic note 'while loop outside functions' at global scope

2022-07-20 Thread YingChi Long via Phabricator via cfe-commits

inclyc added a comment.

Could you please review the latest patch ? I have removed some redundant test 
cases mentioned above


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

https://reviews.llvm.org/D129573

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

[PATCH] D129635: [OpenMP] Update the default version of OpenMP to 5.1

2022-07-20 Thread Alexey Bataev via Phabricator via cfe-commits

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

LG


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129635

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

[PATCH] D130160: [pseudo] Eliminate the dangling-else syntax ambiguity.

2022-07-20 Thread Haojian Wu via Phabricator via cfe-commits

hokein created this revision.
hokein added a reviewer: usaxena95.
Herald added a project: All.
hokein requested review of this revision.
Herald added a subscriber: alextsao1999.
Herald added a project: clang-tools-extra.

- the grammar ambiguity is eliminate by a guard;
- modify the guard function signatures, now all parameters are folded in to a 
single object, avoid a long parameter list (as we will add more parameters in 
the near future);


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130160

Files:
  clang-tools-extra/pseudo/include/clang-pseudo/Language.h
  clang-tools-extra/pseudo/lib/GLR.cpp
  clang-tools-extra/pseudo/lib/cxx/CXX.cpp
  clang-tools-extra/pseudo/lib/cxx/cxx.bnf
  clang-tools-extra/pseudo/test/cxx/dangling-else.cpp
  clang-tools-extra/pseudo/unittests/GLRTest.cpp

Index: clang-tools-extra/pseudo/unittests/GLRTest.cpp
===
--- clang-tools-extra/pseudo/unittests/GLRTest.cpp
+++ clang-tools-extra/pseudo/unittests/GLRTest.cpp
@@ -634,11 +634,12 @@
 start := IDENTIFIER [guard=TestOnly]
   )bnf");
   TestLang.Guards.try_emplace(
-  extensionID("TestOnly"),
-  [&](llvm::ArrayRef RHS, const TokenStream &Tokens) {
-assert(RHS.size() == 1 &&
-   RHS.front()->symbol() == tokenSymbol(clang::tok::identifier));
-return Tokens.tokens()[RHS.front()->startTokenIndex()].text() == "test";
+  extensionID("TestOnly"), [&](const GuardParams &Params) {
+assert(Params.RHS.size() == 1 &&
+   Params.RHS.front()->symbol() ==
+   tokenSymbol(clang::tok::identifier));
+return Params.Tokens.tokens()[Params.RHS.front()->startTokenIndex()]
+   .text() == "test";
   });
   clang::LangOptions LOptions;
   TestLang.Table = LRTable::buildSLR(TestLang.G);
Index: clang-tools-extra/pseudo/test/cxx/dangling-else.cpp
===
--- /dev/null
+++ clang-tools-extra/pseudo/test/cxx/dangling-else.cpp
@@ -0,0 +1,27 @@
+// RUN: clang-pseudo -grammar=cxx -source=%s --start-symbol=statement-seq --print-forest | FileCheck %s
+
+if (true)
+ if (true) {
+
+ }
+ else { // should belong to the nested if statement
+
+ }
+
+// CHECK:  statement-seq~selection-statement := IF ( condition ) statement
+// CHECK-NEXT: ââIF
+// CHECK-NEXT: ââ(
+// CHECK-NEXT: ââcondition~TRUE
+// CHECK-NEXT: ââ)
+// CHECK-NEXT: ââstatement~selection-statement
+// CHECK-NEXT:   ââIF
+// CHECK-NEXT:   ââ(
+// CHECK-NEXT:   ââcondition~TRUE
+// CHECK-NEXT:   ââ)
+// CHECK-NEXT:   ââstatement~compound-statement := { }
+// CHECK-NEXT:   â ââ{
+// CHECK-NEXT:   â ââ}
+// CHECK-NEXT:   ââELSE 
+// CHECK-NEXT:   ââstatement~compound-statement := { }
+// CHECK-NEXT: ââ{ 
+// CHECK-NEXT: ââ}
Index: clang-tools-extra/pseudo/lib/cxx/cxx.bnf
===
--- clang-tools-extra/pseudo/lib/cxx/cxx.bnf
+++ clang-tools-extra/pseudo/lib/cxx/cxx.bnf
@@ -290,7 +290,7 @@
 compound-statement := { statement-seq_opt [recover=Brackets] }
 statement-seq := statement
 statement-seq := statement-seq statement
-selection-statement := IF CONSTEXPR_opt ( init-statement_opt condition ) statement
+selection-statement := IF CONSTEXPR_opt ( init-statement_opt condition ) statement [guard=NextTokenNotElse]
 selection-statement := IF CONSTEXPR_opt ( init-statement_opt condition ) statement ELSE statement
 selection-statement := SWITCH ( init-statement_opt condition ) statement
 iteration-statement := WHILE ( condition ) statement
Index: clang-tools-extra/pseudo/lib/cxx/CXX.cpp
===
--- clang-tools-extra/pseudo/lib/cxx/CXX.cpp
+++ clang-tools-extra/pseudo/lib/cxx/CXX.cpp
@@ -11,6 +11,7 @@
 #include "clang-pseudo/Language.h"
 #include "clang-pseudo/grammar/Grammar.h"
 #include "clang-pseudo/grammar/LRTable.h"
+#include "clang/Basic/TokenKinds.h"
 #include 
 
 namespace clang {
@@ -21,29 +22,29 @@
 #include "CXXBNF.inc"
 ;
 
-bool guardOverride(llvm::ArrayRef RHS,
-   const TokenStream &Tokens) {
-  assert(RHS.size() == 1 &&
- RHS.front()->symbol() == tokenSymbol(clang::tok::identifier));
-  return Tokens.tokens()[RHS.front()->startTokenIndex()].text() == "override";
+bool guardOverride(const GuardParams &Params) {
+  assert(Params.RHS.size() == 1 &&
+ Params.RHS.front()->symbol() == tokenSymbol(clang::tok::identifier));
+  return Params.Tokens.tokens()[Params.RHS.front()->startTokenIndex()].text() ==
+ "override";
 }
-bool guardFinal(llvm::ArrayRef RHS,
-const TokenStream &Tokens) {
-  assert(RHS.size() == 1 &&
- RHS.front()->symbol() == tokenSymbol(clang::tok::identifier));
-  return Tokens.tokens()[RHS.front()->startTokenIndex()].text() == "final";
+bool guardFinal(const GuardParams &Params) {
+

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread Jacques Légaré via Phabricator via cfe-commits

jlegare created this revision.
Herald added a project: All.
jlegare requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Two options have been added: -finstrument-functions-exclude-function-list and 
-finstrument-functions-exclude-file-list.
The first can be used to exclude functions by name, and the second can be used 
to exclude a function by the
file name where its definition occurs. Both options take a comma-separated list 
of values as their argument. By-name
exclusion is done on the demangled name.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130161

Files:
  clang/docs/ClangCommandLineReference.rst
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Driver/ToolChains/Clang.cpp

Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5628,6 +5628,8 @@
   Args.AddLastArg(CmdArgs, options::OPT_finstrument_functions,
   options::OPT_finstrument_functions_after_inlining,
   options::OPT_finstrument_function_entry_bare);
+  Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions_exclude_function_list);
+  Args.AddAllArgs(CmdArgs, options::OPT_finstrument_functions_exclude_file_list);
 
   // NVPTX/AMDGCN doesn't support PGO or coverage. There's no runtime support
   // for sampling, overhead of call arc collection is way too high and there's
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2338,7 +2338,7 @@
 
   /// ShouldInstrumentFunction - Return true if the current function should be
   /// instrumented with __cyg_profile_func_* calls
-  bool ShouldInstrumentFunction();
+  bool ShouldInstrumentFunction(llvm::Function *F);
 
   /// ShouldSkipSanitizerInstrumentation - Return true if the current function
   /// should not be instrumented with sanitizers.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -375,7 +375,7 @@
   // Emit function epilog (to return).
   llvm::DebugLoc Loc = EmitReturnBlock();
 
-  if (ShouldInstrumentFunction()) {
+  if (ShouldInstrumentFunction(CurFn)) {
 if (CGM.getCodeGenOpts().InstrumentFunctions)
   CurFn->addFnAttr("instrument-function-exit", "__cyg_profile_func_exit");
 if (CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining)
@@ -521,13 +521,40 @@
 
 /// ShouldInstrumentFunction - Return true if the current function should be
 /// instrumented with __cyg_profile_func_* calls
-bool CodeGenFunction::ShouldInstrumentFunction() {
+bool CodeGenFunction::ShouldInstrumentFunction(llvm::Function *F) {
   if (!CGM.getCodeGenOpts().InstrumentFunctions &&
   !CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining &&
   !CGM.getCodeGenOpts().InstrumentFunctionEntryBare)
 return false;
   if (!CurFuncDecl || CurFuncDecl->hasAttr())
 return false;
+
+  // Downcast to a function declaration and check to see if the current function
+  // is inlined.  If it is inlined but not externally-visible, then don't
+  // generate tracing for it: at link-time, the function will be gone and the
+  // first argument to the __cyg_profile_* function will be unresolvable.
+  const FunctionDecl *FDecl = dyn_cast(CurFuncDecl);
+  if (FDecl &&
+  FDecl->isInlined() &&
+  !FDecl->isInlineDefinitionExternallyVisible())
+return false;
+
+  typedef std::vector::const_iterator ExcludedFunctionIterator;
+
+  // The list of excluded function name substrings specified on the command-line
+  // is searched to see if any of them is a substring of our (current) function
+  // name.
+  const std::vector &ExcludedFunctions =
+CGM.getCodeGenOpts().InstrumentFunctionsExcludedFunctions;
+
+  std::string FName(F->getName());
+  for (ExcludedFunctionIterator ExcludedFunction = ExcludedFunctions.begin(),
+ Sentinel = ExcludedFunctions.end();
+   ExcludedFunction != Sentinel; ++ExcludedFunction) {
+if (FName.find(*ExcludedFunction) != std::string::npos)
+  return false;
+  }
+
   return true;
 }
 
@@ -1018,7 +1045,7 @@
   CurFuncIsThunk);
   }
 
-  if (ShouldInstrumentFunction()) {
+  if (ShouldInstrumentFunction(CurFn)) {
 if (CGM.getCodeGenOpts().InstrumentFunctions)
   CurFn->addFnAttr("instrument-function-entry", "__cyg_profile_func_enter");
 if (CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining)
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.t

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17870
+///   parameter type and the same cv-qualifiers and ref-qualifier, if any.
+bool AreSpecialMemberFunctionsSameKind(CXXMethodDecl *M1, CXXMethodDecl *M2,
+ Sema::CXXSpecialMember CSM) {





Comment at: clang/lib/Sema/SemaDecl.cpp:17888
+///   [CWG2595], if any, are satisfied is more constrained.
+void SetEligibleMethods(Sema &S, CXXRecordDecl* Record,
+ArrayRef Methods,





Comment at: clang/lib/Sema/SemaDecl.cpp:17960-17961
+}
+if (AnotherMethodIsMoreConstrained)
+  break;
+  }

royjacobson wrote:
> cor3ntin wrote:
> > Is that codepath ever taken?
> > I wonder if there is a way to do that that is not n^2. Maybe not.
> It's not, I didn't mean to leave it there. Thanks for the catch :)
> 
> 
> Generally finding a maximal set in a partial ordering is O(n^2) in the worst 
> case which is when no two objects are comparable.
> 
> We could optimize this a bit for some cases: We can group the functions by 
> their kind, and we can skip comparing functions if we already know that they 
> are not maximal. But since the number of SMFs expected in a class is very 
> (1-3) small, I didn't think those possible improvements are worth the extra 
> complexity.
> 
> Another possible optimization is we could short-circuit this evaluation if we 
> know that a type is not trivially [copyable], since that's the only outcome 
> we really care about, but then the AST is left in a very awkward state.
> 
Thanks, I guess this is fine, I cannot really think of a better way either


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread James Henderson via Phabricator via cfe-commits

jhenderson resigned from this revision.
jhenderson added a comment.

I'm not sure why I've been added as a reviewer, as I don't develop or review 
things within clang...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130161

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

[PATCH] D130058: [Clang] Diagnose ill-formed constant expression when setting a non fixed enum to a value outside the range of the enumeration values

2022-07-20 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/lib/AST/Decl.cpp:4609
+
+  if (NumNegativeBits) {
+unsigned NumBits = std::max(NumNegativeBits, NumPositiveBits + 1);

How does this work if both NumNegativeBits and NumPositiveBits are 0?  Based on 
my reading of the standard, an empty enum should have the valid values 0 and 1, 
so a 'bitwidth' of 1.



Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:2409
+// Empty but as-if it had a single enumerator with value 0
+enum EEmpty {};
+

Would also like to see `enum E4 {e41=0};`, which should have the values 0 and 1.



Comment at: clang/test/SemaCXX/constant-expression-cxx11.cpp:2429
+  constexpr EEmpty x7 = static_cast(1); // expected-error {{must be 
initialized by a constant expression}}
+  // expected-note@-1 {{store of value outside of the range of unscoped enum, 
valid values 0 to 0}}
+

Ok, so I think '1' should be valid here based on my reading.  

An empty should be the exact same case as the proposed `E4` above, and I 
believe we can never allow 'zero' bits.


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

https://reviews.llvm.org/D130058

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

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread Jacques Légaré via Phabricator via cfe-commits

jlegare added a subscriber: jhenderson.
jlegare added a comment.

@jhenderson Apologies, I'm new to the process here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130161

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

[clang] 7169659 - [clang] Small adjustments for -fexperimental-library

2022-07-20 Thread Louis Dionne via cfe-commits


Author: Louis Dionne
Date: 2022-07-20T09:14:55-04:00
New Revision: 7169659752557aa107e3efe470ba988155ae3bbd

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

LOG: [clang] Small adjustments for -fexperimental-library

Move -lc++experimental before -lc++abi (that was forgotten in the
original patch), and mark a test as UNSUPPORTED on AIX. I contacted
the owners of the AIX bot that failed because I was unable to reproduce
the issue locally.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Hexagon.cpp
clang/test/Driver/experimental-library-flag.cpp

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Hexagon.cpp 
b/clang/lib/Driver/ToolChains/Hexagon.cpp
index 0e47704ab636a..93b987c07f290 100644
--- a/clang/lib/Driver/ToolChains/Hexagon.cpp
+++ b/clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -614,10 +614,10 @@ void HexagonToolChain::AddCXXStdlibLibArgs(const ArgList 
&Args,
   switch (Type) {
   case ToolChain::CST_Libcxx:
 CmdArgs.push_back("-lc++");
-CmdArgs.push_back("-lc++abi");
-CmdArgs.push_back("-lunwind");
 if (Args.hasArg(options::OPT_fexperimental_library))
   CmdArgs.push_back("-lc++experimental");
+CmdArgs.push_back("-lc++abi");
+CmdArgs.push_back("-lunwind");
 break;
 
   case ToolChain::CST_Libstdcxx:

diff  --git a/clang/test/Driver/experimental-library-flag.cpp 
b/clang/test/Driver/experimental-library-flag.cpp
index bd6fd87c258ef..db661c026d06f 100644
--- a/clang/test/Driver/experimental-library-flag.cpp
+++ b/clang/test/Driver/experimental-library-flag.cpp
@@ -1,7 +1,10 @@
-// On Windows, -stdlib=libc++ is currently ignored, so -lc++experimental is 
not added.
-// Once -stdlib=libc++ works on Windows, this XFAIL can be removed.
+// On some platforms, -stdlib=libc++ is currently ignored, so 
-lc++experimental is not added.
+// Once -stdlib=libc++ works on those, this XFAIL can be removed.
 // XFAIL: windows, x86_64-scei-ps4, x86_64-sie-ps5
 
+// For some reason, this fails with a core dump on AIX. This needs to be 
investigated.
+// UNSUPPORTED: powerpc-ibm-aix
+
 // RUN: %clangxx -fexperimental-library -stdlib=libc++ -### %s 2>&1 | 
FileCheck --check-prefixes=CHECK,CHECK-LIBCXX %s
 // RUN: %clangxx -fexperimental-library -stdlib=libstdc++ -### %s 2>&1 | 
FileCheck --check-prefixes=CHECK,CHECK-LIBSTDCXX %s
 // RUN: %clangxx -fexperimental-library -stdlib=libc++ -nostdlib++ -### %s 
2>&1 | FileCheck --check-prefixes=CHECK,CHECK-NOSTDLIB %s



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

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread James Henderson via Phabricator via cfe-commits

jhenderson added a comment.

In D130161#3665527 , @jlegare wrote:

> @jhenderson Apologies, I'm new to the process here.

Usually, you should look at who has recently modified the touched files, or 
reviewed those same files, and add them as reviewers. Often, it's a good idea 
to add several people as reviewers at once, since not everybody has the time to 
review everything. You should also look to see if there's a code owner for the 
impacted code. There is some documentation on finding reviewers here: 
https://llvm.org/docs/Phabricator.html#finding-potential-reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130161

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

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

Can you share the motivation for this? There's already an attribute to exclude 
functions from this instrumentation.




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:524
 /// instrumented with __cyg_profile_func_* calls
-bool CodeGenFunction::ShouldInstrumentFunction() {
+bool CodeGenFunction::ShouldInstrumentFunction(llvm::Function *F) {
   if (!CGM.getCodeGenOpts().InstrumentFunctions &&

Why do we need the new F parameter? Doesn't CurFuncDecl work?



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:554
+   ExcludedFunction != Sentinel; ++ExcludedFunction) {
+if (FName.find(*ExcludedFunction) != std::string::npos)
+  return false;

This seems a rather imprecise match. What is ExcludedFunction is "foo" and we 
have a function "football", should it be excluded?

Also the description says the demangled name is used (which seemed odd) but I 
don't see any demangling taking place?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130161

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

[PATCH] D130130: [clang-tidy] Add an IgnoreMacros option to readability-avoid-const-params-in-decls

2022-07-20 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added a comment.

Please mention changes in Release Notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130130

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

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread Jacques Légaré via Phabricator via cfe-commits

jlegare added a comment.

@jhenderson Thanks for the information, I appreciate it.

@Hans Thank you for the comments. I'll be looking to address them later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130161

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

[PATCH] D130168: [CUDA] remove duplicate condition

2022-07-20 Thread VitalyR via Phabricator via cfe-commits

VitalyR created this revision.
Herald added subscribers: mattd, yaxunl.
Herald added a project: All.
VitalyR 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/D130168

Files:
  clang/lib/Sema/SemaCUDA.cpp


Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -714,7 +714,7 @@
   // Do not promote dependent variables since the cotr/dtor/initializer are
   // not determined. Do it after instantiation.
   if (getLangOpts().CUDAIsDevice && !VD->hasAttr() &&
-  !VD->hasAttr() && !VD->hasAttr() &&
+  !VD->hasAttr() &&
   (VD->isFileVarDecl() || VD->isStaticDataMember()) &&
   !IsDependentVar(VD) &&
   ((VD->isConstexpr() || VD->getType().isConstQualified()) &&


Index: clang/lib/Sema/SemaCUDA.cpp
===
--- clang/lib/Sema/SemaCUDA.cpp
+++ clang/lib/Sema/SemaCUDA.cpp
@@ -714,7 +714,7 @@
   // Do not promote dependent variables since the cotr/dtor/initializer are
   // not determined. Do it after instantiation.
   if (getLangOpts().CUDAIsDevice && !VD->hasAttr() &&
-  !VD->hasAttr() && !VD->hasAttr() &&
+  !VD->hasAttr() &&
   (VD->isFileVarDecl() || VD->isStaticDataMember()) &&
   !IsDependentVar(VD) &&
   ((VD->isConstexpr() || VD->getType().isConstQualified()) &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread Jacques Légaré via Phabricator via cfe-commits

jlegare added a comment.

@Hans However, to answer your question: the user may not be in a position to 
change the source where the troublesome functions are located, in order to add 
the attribute.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130161

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

[PATCH] D129009: [LTO] Fix LTO for aliased IFuncs

2022-07-20 Thread Schrodinger ZHU Yifan via Phabricator via cfe-commits

SchrodingerZhu added a comment.

Hi, is there anything else I should do for this patch？


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129009

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

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO.

2022-07-20 Thread Quinn Pham via Phabricator via cfe-commits

quinnp updated this revision to Diff 446147.
quinnp marked 6 inline comments as done.
quinnp added a comment.

Addressing review comments. Fixing the forwarding for -fno-function-sectons and 
removing the ObjectFormatType check.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129401

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/forwarding-sections-liblto.c
  clang/test/Driver/gold-lto-sections.c
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/test/LTO/PowerPC/data-sections-aix.ll
  llvm/test/LTO/PowerPC/data-sections-linux.ll

Index: llvm/test/LTO/PowerPC/data-sections-linux.ll
===
--- /dev/null
+++ llvm/test/LTO/PowerPC/data-sections-linux.ll
@@ -0,0 +1,26 @@
+; RUN: rm -rf %t
+; RUN: mkdir %t
+; RUN: llvm-as %s -o %t/bc.bc
+; RUN: llvm-lto -exported-symbol var -O0 %t/bc.bc -o %t/default.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=1 %t/bc.bc -o \
+; RUN:   %t/data-sections.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=0 %t/bc.bc -o \
+; RUN:   %t/no-data-sections.o
+; RUN: obj2yaml %t/default.o | FileCheck --match-full-lines %s
+; RUN: obj2yaml %t/data-sections.o | FileCheck --match-full-lines %s
+; RUN: obj2yaml %t/no-data-sections.o | FileCheck --match-full-lines \
+; RUN:   --check-prefix CHECK-NO-DATA-SECTIONS %s
+
+target triple = "powerpc64le-unknown-linux-gnu"
+
+@var = global i32 0
+
+; CHECK:   Symbols:
+; CHECK: - Name:var
+; CHECK-NEXT:  Type:STT_OBJECT
+; CHECK-NEXT:  Section: .bss.var
+
+; CHECK-NO-DATA-SECTIONS:  Symbols:
+; CHECK-NO-DATA-SECTIONS:- Name:var
+; CHECK-NO-DATA-SECTIONS-NEXT: Type:STT_OBJECT
+; CHECK-NO-DATA-SECTIONS-NEXT: Section: .bss
Index: llvm/test/LTO/PowerPC/data-sections-aix.ll
===
--- /dev/null
+++ llvm/test/LTO/PowerPC/data-sections-aix.ll
@@ -0,0 +1,22 @@
+; RUN: rm -rf %t
+; RUN: mkdir %t
+; RUN: llvm-as %s -o %t/bc.bc
+; RUN: llvm-lto -exported-symbol var -O0 %t/bc.bc -o %t/default.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=1 %t/bc.bc -o \
+; RUN:   %t/data-sections.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=0 %t/bc.bc -o \
+; RUN:   %t/no-data-sections.o
+; RUN: obj2yaml %t/default.o | FileCheck --match-full-lines %s
+; RUN: obj2yaml %t/data-sections.o | FileCheck --match-full-lines %s
+; RUN: obj2yaml %t/no-data-sections.o | FileCheck --match-full-lines \
+; RUN:   --check-prefix CHECK-NO-DATA-SECTIONS %s
+
+target triple = "powerpc-ibm-aix7.2.0.0"
+
+@var = global i32 0
+
+; CHECK:  Symbols:
+; CHECK-NOT:- Name: .data
+
+; CHECK-NO-DATA-SECTIONS: Symbols:
+; CHECK-NO-DATA-SECTIONS:   - Name: .data
Index: llvm/lib/LTO/LTOCodeGenerator.cpp
===
--- llvm/lib/LTO/LTOCodeGenerator.cpp
+++ llvm/lib/LTO/LTOCodeGenerator.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
+#include "llvm/CodeGen/CommandFlags.h"
 #include "llvm/CodeGen/ParallelCG.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/Config/config.h"
@@ -344,6 +345,11 @@
   Config.CPU = "cyclone";
   }
 
+  // If data-sections is not explicitly set or unset, set data-sections by
+  // default to match the behaviour of lld and gold plugin.
+  if (!codegen::getExplicitDataSections())
+Config.Options.DataSections = true;
+
   TargetMach = createTargetMachine();
   assert(TargetMach && "Unable to create target machine");
 
Index: clang/test/Driver/gold-lto-sections.c
===
--- clang/test/Driver/gold-lto-sections.c
+++ clang/test/Driver/gold-lto-sections.c
@@ -4,5 +4,5 @@
 // RUN: -Wl,-plugin-opt=foo -O3 \
 // RUN: -ffunction-sections -fdata-sections \
 // RUN: | FileCheck %s
-// CHECK: "-plugin-opt=-function-sections"
-// CHECK: "-plugin-opt=-data-sections"
+// CHECK: "-plugin-opt=-function-sections=1"
+// CHECK: "-plugin-opt=-data-sections=1"
Index: clang/test/Driver/forwarding-sections-liblto.c
===
--- /dev/null
+++ clang/test/Driver/forwarding-sections-liblto.c
@@ -0,0 +1,15 @@
+// RUN: touch %t.o
+// RUN: %clang %t.o -### -flto 2>&1 | FileCheck %s
+// RUN: %clang %t.o -### -flto 2>&1 -ffunction-sections -fdata-sections | \
+// RUN:   FileCheck %s --check-prefix=CHECK-SECTIONS
+// RUN: %clang %t.o -### -flto 2>&1 -fno-function-sections -fno-data-sections \
+// RUN:   | FileCheck %s --check-prefix=CHECK-NO-SECTIONS
+
+// CHECK-NOT: "-plugin-opt=-function-sections=1"
+// CHECK-NOT: "-plugin-opt=-function-sections=0"
+// CHECK-NOT: "-plugin-opt=-

[clang] 7373497 - UNSUPPORT test on 64-bit AIX too

2022-07-20 Thread Jake Egan via cfe-commits


Author: Jake Egan
Date: 2022-07-20T10:05:03-04:00
New Revision: 7373497a4b282e30b07d72fa601265a631f4c5c5

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

LOG: UNSUPPORT test on 64-bit AIX too

The test failure affects both bitmodes.

Added: 


Modified: 
clang/test/Driver/experimental-library-flag.cpp

Removed: 




diff  --git a/clang/test/Driver/experimental-library-flag.cpp 
b/clang/test/Driver/experimental-library-flag.cpp
index db661c026d06..2738df8473d8 100644
--- a/clang/test/Driver/experimental-library-flag.cpp
+++ b/clang/test/Driver/experimental-library-flag.cpp
@@ -3,7 +3,7 @@
 // XFAIL: windows, x86_64-scei-ps4, x86_64-sie-ps5
 
 // For some reason, this fails with a core dump on AIX. This needs to be 
investigated.
-// UNSUPPORTED: powerpc-ibm-aix
+// UNSUPPORTED: powerpc{{.*}}-ibm-aix
 
 // RUN: %clangxx -fexperimental-library -stdlib=libc++ -### %s 2>&1 | 
FileCheck --check-prefixes=CHECK,CHECK-LIBCXX %s
 // RUN: %clangxx -fexperimental-library -stdlib=libstdc++ -### %s 2>&1 | 
FileCheck --check-prefixes=CHECK,CHECK-LIBSTDCXX %s



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

[clang] bd519b9 - redo UNSUPPORT test on 64-bit AIX too

2022-07-20 Thread Jake Egan via cfe-commits


Author: Jake Egan
Date: 2022-07-20T10:18:28-04:00
New Revision: bd519b9335fe507bc2d2ebbbd3321ba046f038a3

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

LOG: redo UNSUPPORT test on 64-bit AIX too

The test failure affects both bitmodes.

Added: 


Modified: 
clang/test/Driver/experimental-library-flag.cpp

Removed: 




diff  --git a/clang/test/Driver/experimental-library-flag.cpp 
b/clang/test/Driver/experimental-library-flag.cpp
index 2738df8473d8..529888b867c1 100644
--- a/clang/test/Driver/experimental-library-flag.cpp
+++ b/clang/test/Driver/experimental-library-flag.cpp
@@ -3,7 +3,7 @@
 // XFAIL: windows, x86_64-scei-ps4, x86_64-sie-ps5
 
 // For some reason, this fails with a core dump on AIX. This needs to be 
investigated.
-// UNSUPPORTED: powerpc{{.*}}-ibm-aix
+// UNSUPPORTED: aix
 
 // RUN: %clangxx -fexperimental-library -stdlib=libc++ -### %s 2>&1 | 
FileCheck --check-prefixes=CHECK,CHECK-LIBCXX %s
 // RUN: %clangxx -fexperimental-library -stdlib=libstdc++ -### %s 2>&1 | 
FileCheck --check-prefixes=CHECK,CHECK-LIBSTDCXX %s



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

[PATCH] D128927: [libc++] Always build c++experimental.a

2022-07-20 Thread Hans Wennborg via Phabricator via cfe-commits

hans added a comment.

In D128927#3662659 , @ldionne wrote:

> The weird part here is that you're configuring libc++, but you are building 
> neither the static nor the shared library. I don't understand why you do 
> that, and that may hide some other more fundamental issue in your setup.

Yes, I wish we weren't weird.
The reason libc++ is part of our build is that we want the headers so that our 
newly built Clang will be able to build c++ files. 
https://bugs.chromium.org/p/chromium/issues/detail?id=1067216#c7 has the 
background for our current situation.

In https://github.com/llvm/llvm-zorg/pull/28 you mentioned that "for a couple 
of years now, Clang and libc++ are entirely separate projects w.r.t. how they 
are shipped on Apple platforms, and it doesn't make sense to build libc++ at 
the same time as Clang anymore".
Does that mean the libc++ headers have moved to the SDK now, and if so from 
what version? If it's in the SDK version we use (I'd have to check), perhaps 
that would allow us to stop building like this.

> Anyways, I think the issue should be resolved by 
> 1d0f79558ca47f50119fb38c62ff5afd3160d260 
> .

Yes, that fixed it for us. Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128927

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added a comment.

@aaron.ballman @erichkeane I think this should be on your radar, it would be 
great to have it in 15. I did review it and I think it looks good but there are 
enough dragons here that i'd rather have more eye balls on it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

I believe the calculation of the constraints here means this isn't conforming 
with concepts.  I believe this means we have to delay calculating the 
constraint differences until this property is actually queried, and cannot do 
it early like this has.




Comment at: clang/lib/Sema/SemaDecl.cpp:17874
+  return true;
+if (M1->getParamDecl(0)->getType() != M2->getParamDecl(0)->getType())
+  return false;

I don't think you'd want to compare with equality here, it ends up not 
desugaring/etc.  I believe `ASTContext::HasSameType` should be what you want.  



Comment at: clang/lib/Sema/SemaDecl.cpp:17876
+  return false;
+if (M1->getThisType() != M2->getThisType())
+  return false;

Probably same here.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

This seems problematic, doesn't it?  Checking this constraint will (once I 
figure out how to get deferred instantiation to work) cause instantiation, 
which can cause issues with incomplete types/CRTP/etc.

I think the result is that we cannot 'calculate' this until it is queried, else 
we will cause incorrect errors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

___
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

2022-07-20 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added a comment.

The more I look at this... the more I question my approach.  I now am 
definitely sure we won't make Clang15, and hope that I can figure something 
better out for 16 :/


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] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits

royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17870
+///   parameter type and the same cv-qualifiers and ref-qualifier, if any.
+bool AreSpecialMemberFunctionsSameKind(CXXMethodDecl *M1, CXXMethodDecl *M2,
+ Sema::CXXSpecialMember CSM) {

cor3ntin wrote:
> 
It's in an anonymous namespace. Is it a Clang convention to use static 
functions instead? I saw both used in Sema, I think



Comment at: clang/lib/Sema/SemaDecl.cpp:17874
+  return true;
+if (M1->getParamDecl(0)->getType() != M2->getParamDecl(0)->getType())
+  return false;

erichkeane wrote:
> I don't think you'd want to compare with equality here, it ends up not 
> desugaring/etc.  I believe `ASTContext::HasSameType` should be what you want. 
>  
Good catch! added a test for this.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

erichkeane wrote:
> This seems problematic, doesn't it?  Checking this constraint will (once I 
> figure out how to get deferred instantiation to work) cause instantiation, 
> which can cause issues with incomplete types/CRTP/etc.
> 
> I think the result is that we cannot 'calculate' this until it is queried, 
> else we will cause incorrect errors.
Making this queried on demand is a relatively big change to how we handle type 
triviality, so I want to be sure we actually need to do this to be conformant.

When I started working on this I checked what GCC does and it instantiates 
those constraints during class completion as well. For example this CRTP case: 
https://godbolt.org/z/EdoYf96zq. MSVC seem to do it as well.

So maybe it's OK to check the constraints of SMFs specifically?




Comment at: clang/lib/Sema/SemaDecl.cpp:17960-17961
+}
+if (AnotherMethodIsMoreConstrained)
+  break;
+  }

cor3ntin wrote:
> royjacobson wrote:
> > cor3ntin wrote:
> > > Is that codepath ever taken?
> > > I wonder if there is a way to do that that is not n^2. Maybe not.
> > It's not, I didn't mean to leave it there. Thanks for the catch :)
> > 
> > 
> > Generally finding a maximal set in a partial ordering is O(n^2) in the 
> > worst case which is when no two objects are comparable.
> > 
> > We could optimize this a bit for some cases: We can group the functions by 
> > their kind, and we can skip comparing functions if we already know that 
> > they are not maximal. But since the number of SMFs expected in a class is 
> > very (1-3) small, I didn't think those possible improvements are worth the 
> > extra complexity.
> > 
> > Another possible optimization is we could short-circuit this evaluation if 
> > we know that a type is not trivially [copyable], since that's the only 
> > outcome we really care about, but then the AST is left in a very awkward 
> > state.
> > 
> Thanks, I guess this is fine, I cannot really think of a better way either
Ok, did another look on this and it's because `AnotherMethodIsMoreConstrained` 
is the output of `IsAtLeastAsConstrained`, so that's why the extra check. Added 
it back :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits

royjacobson updated this revision to Diff 446172.
royjacobson added a comment.

Fixed the tests, fixed the type comparison checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/AST/DeclCXX.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/conditionally-trivial-smfs.cpp
  clang/test/Lexer/cxx-features.cpp
  clang/test/SemaCXX/constrained-special-member-functions.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -927,7 +927,7 @@
   

 https://wg21.link/p0848r3";>P0848R3
-No
+Clang 15
   
   
 https://wg21.link/p1616r1";>P1616R1
Index: clang/test/SemaCXX/constrained-special-member-functions.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/constrained-special-member-functions.cpp
@@ -0,0 +1,195 @@
+// RUN: %clang_cc1 -verify -std=c++20 %s
+// expected-no-diagnostics
+
+template 
+concept C0 = (N == 0);
+template 
+concept C1 = (N == 1);
+template 
+concept C2 = (N == 2);
+
+// Checks are indexed by:
+// Definition:
+//  1. Explicitly defaulted definition
+//  2. Deleted definition
+//  3. User provided definition
+// We have a less constrained user provided method that should not disable
+// the (copyable) triviality of the type.
+
+// Note that because Clang does not implement DRs 1496 and 1734, we say some
+// classes are trivial when the SMFs are deleted.
+
+template 
+struct DefaultConstructorChecker {
+DefaultConstructorChecker() requires C0 = default;
+DefaultConstructorChecker() requires C1 = delete;
+DefaultConstructorChecker() requires C2;
+DefaultConstructorChecker();
+};
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<0>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<1>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<2>));
+static_assert(__is_trivially_copyable(DefaultConstructorChecker<3>));
+static_assert(__is_trivial(DefaultConstructorChecker<0>));
+// FIXME: DR1496
+static_assert(__is_trivial(DefaultConstructorChecker<1>));
+static_assert(!__is_trivial(DefaultConstructorChecker<2>));
+static_assert(!__is_trivial(DefaultConstructorChecker<3>));
+
+template 
+struct CopyConstructorChecker {
+CopyConstructorChecker(const CopyConstructorChecker&) requires C0 = default;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C1 = delete;
+CopyConstructorChecker(const CopyConstructorChecker&) requires C2;
+CopyConstructorChecker(const CopyConstructorChecker&);
+};
+
+static_assert(__is_trivially_copyable(CopyConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(CopyConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(CopyConstructorChecker<3>));
+static_assert(!__is_trivial(CopyConstructorChecker<0>));
+static_assert(!__is_trivial(CopyConstructorChecker<1>));
+static_assert(!__is_trivial(CopyConstructorChecker<2>));
+static_assert(!__is_trivial(CopyConstructorChecker<3>));
+
+template 
+struct MoveConstructorChecker {
+MoveConstructorChecker(MoveConstructorChecker&&) requires C0 = default;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C1 = delete;
+MoveConstructorChecker(MoveConstructorChecker&&) requires C2;
+MoveConstructorChecker(MoveConstructorChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveConstructorChecker<0>));
+// FIXME: DR1734
+static_assert(__is_trivially_copyable(MoveConstructorChecker<1>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<2>));
+static_assert(!__is_trivially_copyable(MoveConstructorChecker<3>));
+static_assert(!__is_trivial(MoveConstructorChecker<0>));
+static_assert(!__is_trivial(MoveConstructorChecker<1>));
+static_assert(!__is_trivial(MoveConstructorChecker<2>));
+static_assert(!__is_trivial(MoveConstructorChecker<3>));
+
+template 
+struct MoveAssignmentChecker {
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C0 = default;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C1 = delete;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&) requires C2;
+MoveAssignmentChecker& operator=(MoveAssignmentChecker&&);
+};
+
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<0>));
+// FIXME: DR1734.
+static_assert(__is_trivially_copyable(MoveAssignmentChecker<1>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<2>));
+static_assert(!__is_trivially_copyable(MoveAssignmentChecker<3>));
+static_assert(__is_trivial(MoveAs

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

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov updated this revision to Diff 446173.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112374

Files:
  clang-tools-extra/clang-change-namespace/ChangeNamespace.cpp
  clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SmartPtrArrayMismatchCheck.cpp
  clang-tools-extra/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/MultiwayPathsCoveredCheck.cpp
  clang-tools-extra/clang-tidy/misc/MisplacedConstCheck.cpp
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/DumpASTTests.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/HoverTests.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/copy-constructor-init.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/shared-ptr-array-mismatch.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-memory-comparison-32bits.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone/suspicious-memory-comparison.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/const-return-type.cpp
  clang-tools-extra/unittests/clang-change-namespace/ChangeNamespaceTests.cpp
  clang/bindings/python/tests/cindex/test_type.py
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeLoc.h
  clang/lib/ARCMigrate/ObjCMT.cpp
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTDiagnostic.cpp
  clang/lib/AST/DeclCXX.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/FormatString.cpp
  clang/lib/AST/PrintfFormatString.cpp
  clang/lib/AST/QualTypeNames.cpp
  clang/lib/AST/ScanfFormatString.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypeLoc.cpp
  clang/lib/Analysis/RetainSummaryManager.cpp
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaExprObjC.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaType.cpp
  clang/lib/Sema/TreeTransform.h
  clang/lib/Sema/TypeLocBuilder.cpp
  clang/lib/Sema/TypeLocBuilder.h
  clang/lib/StaticAnalyzer/Checkers/NonnullGlobalConstantsChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/NumberObjectConversionChecker.cpp
  clang/lib/Tooling/DumpTool/ASTSrcLocProcessor.cpp
  clang/test/AST/ast-dump-APValue-anon-union.cpp
  clang/test/AST/ast-dump-APValue-struct.cpp
  clang/test/AST/ast-dump-APValue-union.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-expr-json.cpp
  clang/test/AST/ast-dump-expr.cpp
  clang/test/AST/ast-dump-funcs.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
  clang/test/AST/ast-dump-overloaded-operators.cpp
  clang/test/AST/ast-dump-records-json.cpp
  clang/test/AST/ast-dump-recovery.cpp
  clang/test/AST/ast-dump-stmt-json.cpp
  clang/test/AST/ast-dump-stmt.cpp
  clang/test/AST/ast-dump-template-decls-json.cpp
  clang/test/AST/ast-dump-temporaries-json.cpp
  clang/test/AST/ast-dump-using-template.cpp
  clang/test/AST/ast-dump-using.cpp
  clang/test/AST/coroutine-locals-cleanup-exp-namespace.cpp
  clang/test/AST/coroutine-locals-cleanup.cpp
  clang/test/AST/float16.cpp
  clang/test/AST/sourceranges.cpp
  clang/test/Analysis/Inputs/expected-plists/NewDelete-path-notes.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/cxx-for-range.cpp.plist
  clang/test/Analysis/Inputs/expected-plists/method-call-path-notes.cpp.plist
  clang/test/Analysis/analyzer-display-progress.cpp
  clang/test/Analysis/auto-obj-dtors-cfg-output.cpp
  clang/test/Analysis/blocks.mm
  clang/test/Analysis/bug_hash_test.cpp
  clang/test/Analysis/cast-value-notes.cpp
  clang/test/Analysis/cast-value-state-dump.cpp
  clang/test/Analysis/cfg-rich-constructors.cpp
  clang/test/Analysis/cfg-rich-constructors.mm
  clang/test/Analysis/cfg.cpp
  clang/test/Analysis/copy-elision.cpp
  clang/test/Analysis/cxx-uninitialized-object-inheritance.cpp
  clang/test/Analysis/dump_egraph.cpp
  clang/test/Analysis/exploded-graph-rewriter/dynamic_types.cpp
  clang/test/Analysis/initializers-cfg-output.cpp
  clang/test/Analysis/inlining/Inputs/expected-plists/path-notes.cpp.plist
  clang/test/Analysis/lambdas.cpp
  clang/test/Analysis/lifetime-cfg-output.cpp
  clang/test/Analysis/malloc-sizeof.c

[PATCH] D130168: [CUDA] remove duplicate condition

2022-07-20 Thread Yaxun Liu via Phabricator via cfe-commits

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130168

___
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

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added a comment.

In D112374#3665249 , @sammccall wrote:

> I've seen plenty of (useful) out-of-tree tidy checks written by people fuzzy 
> on the difference between a Type and a TypeLoc, or what sugar is. Clang makes 
> it (almost) easy to write tools but hard to write robust tools.

I agree.

> All of this is to say I like this change & appreciate how willing you are to 
> help out-of-tree tools (which is best-effort), but I expect a lot of churn 
> downstream. (And LLVM has a clear policy that that's OK).

Thanks!

> (BTW, last time I landed such a change, investigating the LLDB tests was 
> indeed the most difficult part, and I'm not even on windows. Running a linux 
> VM of some sort might be your best bet, unfortunately)

Yeah I finally managed to build and run the tests on WSL2 running debian 
testing. The exact configuration used by the lldb-bot seems not to be supported 
there anymore.

I added the needed changes there, it was really only a change in expectation on 
the type printer.
These tests are so easy to break not only because they depend exactly on how 
clang prints types, but they also depend on how libc++ devs write internal 
implementation details.

If anyone wants to take a look at the new changes to lldb tests, be my guest. 
Otherwise I will try to land this again soon. It might well be that we figure 
out some other in-tree user is affected, but I'd rather do that sooner than 
later.

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] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added a comment.

It seems this change broke build on windows for me:

  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(59): error C2838: 
'noptr_declarator_0declarator_id': illegal qualified name in member declaration
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(59): error C2065: 
'noptr_declarator_0declarator_id': undeclared identifier
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(59): error C2131: expression did not 
evaluate to a constant
clang-tools-extra\pseudo\lib\cxx\CXX.cpp(59): note: a non-constant 
(sub-)expression was encountered
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(62): error C2838: 
'ptr_declarator_0ptr_operator_1ptr_declarator': illegal qualified name in 
member declaration
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(62): error C2065: 
'ptr_declarator_0ptr_operator_1ptr_declarator': undeclared identifier
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(62): error C2131: expression did not 
evaluate to a constant
clang-tools-extra\pseudo\lib\cxx\CXX.cpp(62): note: a non-constant 
(sub-)expression was encountered
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(68): error C2838: 
'declarator_0noptr_declarator_1parameters_and_qualifiers_2trailing_return_type':
 illegal qualified name in member declaration
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(68): error C2065: 
'declarator_0noptr_declarator_1parameters_and_qualifiers_2trailing_return_type':
 undeclared identifier
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(68): error C2131: expression did not 
evaluate to a constant
clang-tools-extra\pseudo\lib\cxx\CXX.cpp(68): note: a non-constant 
(sub-)expression was encountered
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(69): error C2838: 
'noptr_declarator_0noptr_declarator_1parameters_and_qualifiers': illegal 
qualified name in member declaration
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(69): error C2065: 
'noptr_declarator_0noptr_declarator_1parameters_and_qualifiers': undeclared 
identifier
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(69): error C2131: expression did not 
evaluate to a constant
clang-tools-extra\pseudo\lib\cxx\CXX.cpp(69): note: a non-constant 
(sub-)expression was encountered
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(75): error C2838: 
'noptr_declarator_0noptr_declarator_1l_square_2constant_expression_3r_square': 
illegal qualified name in member declaration
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(75): error C2065: 
'noptr_declarator_0noptr_declarator_1l_square_2constant_expression_3r_square': 
undeclared identifier
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(75): error C2131: expression did not 
evaluate to a constant
clang-tools-extra\pseudo\lib\cxx\CXX.cpp(75): note: a non-constant 
(sub-)expression was encountered
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(76): error C2838: 
'noptr_declarator_0noptr_declarator_1l_square_2r_square': illegal qualified 
name in member declaration
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(76): error C2065: 
'noptr_declarator_0noptr_declarator_1l_square_2r_square': undeclared identifier
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(76): error C2131: expression did not 
evaluate to a constant
clang-tools-extra\pseudo\lib\cxx\CXX.cpp(76): note: a non-constant 
(sub-)expression was encountered
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(81): error C2838: 
'noptr_declarator_0l_paren_1ptr_declarator_2r_paren': illegal qualified name in 
member declaration
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(81): error C2065: 
'noptr_declarator_0l_paren_1ptr_declarator_2r_paren': undeclared identifier
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(81): error C2131: expression did not 
evaluate to a constant
clang-tools-extra\pseudo\lib\cxx\CXX.cpp(81): note: a non-constant 
(sub-)expression was encountered
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(84): error C2838: 
'ptr_declarator_0noptr_declarator': illegal qualified name in member declaration
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(84): error C2065: 
'ptr_declarator_0noptr_declarator': undeclared identifier
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(84): error C2131: expression did not 
evaluate to a constant
clang-tools-extra\pseudo\lib\cxx\CXX.cpp(84): note: a non-constant 
(sub-)expression was encountered
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(85): error C2838: 
'declarator_0ptr_declarator': illegal qualified name in member declaration
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(85): error C2065: 
'declarator_0ptr_declarator': undeclared identifier
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(85): error C2131: expression did not 
evaluate to a constant
clang-tools-extra\pseudo\lib\cxx\CXX.cpp(85): note: a non-constant 
(sub-)expression was encountered
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(59): error C2051: case expression 
not constant
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(62): error C2051: case expression 
not constant
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(67): error C2051: case expression 
not constant
  clang-tools-extra\pseudo\lib\cxx\CXX.cpp(69): error C2051: case

[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

2022-07-20 Thread Arthur Eubanks via Phabricator via cfe-commits

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

update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128955

Files:
  clang/lib/CodeGen/CGClass.cpp
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/test/CodeGenCXX/cfi-mfcall.cpp
  clang/test/CodeGenCXX/thinlto-distributed-type-metadata.cpp
  clang/test/CodeGenCXX/thinlto_public_type_test_distributed.ll
  clang/test/CodeGenCXX/type-metadata.cpp
  lld/test/ELF/lto/update_public_type_test.ll
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/ModuleSummaryIndex.h
  llvm/include/llvm/Transforms/IPO/WholeProgramDevirt.h
  llvm/lib/Analysis/ModuleSummaryAnalysis.cpp
  llvm/lib/Analysis/TypeMetadataUtils.cpp
  llvm/lib/IR/ModuleSummaryIndex.cpp
  llvm/lib/LTO/LTO.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
  llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp
  llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
  llvm/tools/llvm-lto2/llvm-lto2.cpp

Index: llvm/tools/llvm-lto2/llvm-lto2.cpp
===
--- llvm/tools/llvm-lto2/llvm-lto2.cpp
+++ llvm/tools/llvm-lto2/llvm-lto2.cpp
@@ -65,6 +65,10 @@
cl::desc("Alias Analysis Pipeline"),
cl::value_desc("aapipeline"));
 
+static cl::opt WholeProgramVisibilityConf(
+"whole-program-visibility-conf",
+cl::desc("Set whole program visibility in LTO configuration"));
+
 static cl::opt SaveTemps("save-temps", cl::desc("Save temporary files"));
 
 static cl::list SelectSaveTemps(
@@ -302,6 +306,7 @@
   // Run a custom pipeline, if asked for.
   Conf.OptPipeline = OptPipeline;
   Conf.AAPipeline = AAPipeline;
+  Conf.HasWholeProgramVisibility = WholeProgramVisibilityConf;
 
   Conf.OptLevel = OptLevel - '0';
   Conf.Freestanding = EnableFreestanding;
Index: llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
===
--- llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
+++ llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
@@ -10,6 +10,7 @@
 ; RUN:   -whole-program-visibility \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t2.o,test,px \
+; RUN:   -r=%t2.o,test_public,px \
 ; RUN:   -r=%t2.o,_ZN1A1nEi,p \
 ; RUN:   -r=%t2.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t2.o,_ZN1C1fEi,p \
@@ -28,6 +29,7 @@
 ; RUN:   -verify-machineinstrs=0 \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t.o,test,px \
+; RUN:   -r=%t.o,test_public,px \
 ; RUN:   -r=%t.o,_ZN1A1nEi,p \
 ; RUN:   -r=%t.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t.o,_ZN1C1fEi,p \
@@ -50,6 +52,7 @@
 ; RUN:   -whole-program-visibility \
 ; RUN:   -o %t5 \
 ; RUN:   -r=%t4.o,test,px \
+; RUN:   -r=%t4.o,test_public,px \
 ; RUN:   -r=%t4.o,_ZN1A1nEi,p \
 ; RUN:   -r=%t4.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t4.o,_ZN1C1fEi,p \
@@ -59,6 +62,8 @@
 ; RUN:   -r=%t4.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=REMARK
 ; RUN: llvm-dis %t5.0.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
 
+; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
+; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEi
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1A1nEi
 ; REMARK-DAG: single-impl: devirtualized a call to _ZN1D1mEi
 
@@ -69,6 +74,7 @@
 ; RUN: llvm-lto2 run %t2.o -save-temps -pass-remarks=. \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t2.o,test,px \
+; RUN:   -r=%t2.o,test_public,px \
 ; RUN:   -r=%t2.o,_ZN1A1nEi,p \
 ; RUN:   -r=%t2.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t2.o,_ZN1C1fEi,p \
@@ -83,6 +89,7 @@
 ; RUN:   -verify-machineinstrs=0 \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t.o,test,px \
+; RUN:   -r=%t.o,test_public,px \
 ; RUN:   -r=%t.o,_ZN1A1nEi,p \
 ; RUN:   -r=%t.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t.o,_ZN1C1fEi,p \
@@ -103,6 +110,7 @@
 ; RUN: llvm-lto2 run %t4.o -save-temps -pass-remarks=. \
 ; RUN:   -o %t5 \
 ; RUN:   -r=%t4.o,test,px \
+; RUN:   -r=%t4.o,test_public,px \
 ; RUN:   -r=%t4.o,_ZN1A1nEi,p \
 ; RUN:   -r=%t4.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t4.o,_ZN1C1fEi,p \
@@ -120,6 +128,7 @@
 ; RUN:   -disable-whole-program-visibility \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t2.o,test,px \
+; RUN:   -r=%t2.o,test_public,px \
 ; RUN:   -r=%t2.o,_ZN1A1nEi,p \
 ; RUN:   -r=%t2.o,_ZN1B1fEi,p \
 ; RUN:   -r=%t2.o,_ZN1C1fEi,p \
@@ -178,7 +187,44 @@
 ; CHECK-IR-LABEL: ret i32
 ; CHECK-IR-LABEL: }
 
+; CHECK-IR-LABEL: define i32 @test_public
+define i32 @test_public(ptr %obj, ptr %obj2, i32 %a) {
+entry:
+  %vtable = load ptr, ptr %obj
+  %p = call i1 @llvm.public.type.test(ptr %vtable, metadata !"_ZTS1A")
+  call void @llvm.assume(i1 %p)
+  %fptrptr = getelementptr ptr, ptr %vtable, i32 1
+  %fptr1 = load ptr, ptr %fptrptr, align 8
+
+  ; Check that the call was devirtualized.
+  ; CHECK-IR: %call = tail call i32 @_ZN1A1nEi
+  ; CHECK-NODEVIRT-IR: %call = tail call i32 %fptr1
+  %call = tail call i32 %fptr1(ptr nonnull %obj, i32 %a)
+
+  %fptr22 = load ptr, ptr %vtable, align 8
+
+  ; We still have to ca

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-07-20 Thread Nathan James via Phabricator via cfe-commits

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

Add a check to flag loops and functions where an `if` at the end of the body 
could be negated and made into an early exit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130181

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
@@ -0,0 +1,158 @@
+// RUN: %check_clang_tidy %s readability-use-early-exits -format-style=llvm %t -- \
+// RUN:   -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 2}}"
+
+// Just to hit the threshold
+void padLines();
+
+void nomralFunc(int *A) {
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+  if (A) {
+*A = 10;
+  }
+  //   CHECK-FIXES: if (!A)
+  //  CHECK-FIXES-NEXT: return;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: *A = 10;
+}
+
+void funcWithTrailing(int *B) {
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+  if (B) {
+*B = 10;
+  }
+  return;
+  ;
+  //   CHECK-FIXES: if (!B)
+  //  CHECK-FIXES-NEXT: return;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: *B = 10;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: return;
+  //  CHECK-FIXES-NEXT: ;
+}
+
+void normal(int A, int B) {
+  
+  while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+if (A == B) {
+  padLines();
+}
+  }
+  //   CHECK-FIXES:  while (true) {
+  //  CHECK-FIXES-NEXT: if (A != B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: padLines();
+  //  CHECK-FIXES-NEXT: }
+
+  // Eat continue and empty nulls
+  while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+if (A != B) {
+  padLines();
+}
+continue;
+;
+continue;
+  } 
+  //   CHECK-FIXES:  while (true) {
+  //  CHECK-FIXES-NEXT: if (A == B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: padLines();
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT:   ;
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT: }
+}
+
+void toShort(int A, int B) {
+  while (true) {
+if (A == B) {
+}
+  }
+}
+
+void hasElse(int A, int B) {
+  while (true) {
+if (A == B) {
+
+} else {
+}
+  }
+}
+
+void hasTrailingStmt(int A, int B) {
+  while (true) {
+if (A == B) {
+}
+padLines();
+  }
+}
+
+void nested(int A, int B) {
+  // if (A > B) {
+  // CHECK-MESSAGES: [[@LINE+6]]:5: warning: use early exit
+  // if (B < A) {
+  // CHECK-MESSAGES: [[@LINE+5]]:7: warning: use early exit
+  // if (A == B) {
+  // CHECK-MESSAGES: [[@LINE+4]]:9: warning: use early exit
+  while (true) { // Nested
+if (A > B) {
+  if (B < A) {
+if (A != B) {
+  padLines();
+}
+  }
+}
+  } // EndLoop
+  //   CHECK-FIXES: while (true) { // Nested
+  //  CHECK-FIXES-NEXT: if (A <= B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: if (B >= A)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: if (A == B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: padLines();
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: } // EndLoop
+}
+
+void badNested(bool B) {
+  // Ensure check doesn't check for nested `if` if outer `if` has else.
+  while (true) {
+if (B) {
+  if (B) {
+  }
+} else {
+}
+  }
+}
+
+void semiNested(int A, int B) {
+  // CHECK-MESSAGES: [[@LINE+2]]:5: warning: use early exit
+  while (true) { // SemiNested
+if (A > B) {
+  if (B < A) {
+if (A != B) {
+  padLines();
+}
+  } else {
+  }
+}
+  }
+  //   CHECK-FIXES: while (true) { // SemiNested
+  //  CHECK-FIXES-NEXT:   if (A <= B)
+  //  CHECK-FIXES-NEXT: continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT:   if (B < A) {
+  //  CHECK-FIXES-NEXT: if (A != B) {
+  //  CHECK-FIXES-NEXT:   padLines();
+  //  CHECK-FIXES-NEXT: }
+  //  CHECK-FIXES-NEXT: } else {
+  //  CHECK-FIXES-NEXT: }

[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

2022-07-20 Thread Arthur Eubanks via Phabricator via cfe-commits

aeubanks added inline comments.



Comment at: llvm/lib/LTO/LTOBackend.cpp:595
 
+  updatePublicTypeTestCalls(Mod, Conf.HasWholeProgramVisibility);
+

aeubanks wrote:
> tejohnson wrote:
> > aeubanks wrote:
> > > I'm not sure where the best place to put these is
> > I don't think that this will work for distributed ThinLTO, where the 
> > ThinLTO Backends are run via clang, which isn't going to have this config 
> > variable set (since it is set only during LTO linking). I think something 
> > may need to be recorded in the index as a flag there at thin link / 
> > indexing time that can be checked here.
> > 
> > It would then be nice to consolidate this handling into WPD itself, e.g. at 
> > the start of DevirtModule::run, but unfortunately we won't have a summary 
> > index for pure regular LTO WPD so I don't think that would work. So in here 
> > is ok but I would do it early to handle some of the early return cases.
> > 
> > (Please add a distributed ThinLTO test too)
> Added a distributed ThinLTO test: 
> clang/test/CodeGenCXX/thinlto_public_type_test_distributed.ll.
> 
> I added `ModuleSummaryIndex::WithWholeProgramVisibility` but I'm not sure 
> where I'd set it, and that's causing the test to fail.
ah, I needed a separate WPV flag in llvm-lto2 to set `llvm::lto::Config`, now 
everything passes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128955

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

royjacobson wrote:
> erichkeane wrote:
> > This seems problematic, doesn't it?  Checking this constraint will (once I 
> > figure out how to get deferred instantiation to work) cause instantiation, 
> > which can cause issues with incomplete types/CRTP/etc.
> > 
> > I think the result is that we cannot 'calculate' this until it is queried, 
> > else we will cause incorrect errors.
> Making this queried on demand is a relatively big change to how we handle 
> type triviality, so I want to be sure we actually need to do this to be 
> conformant.
> 
> When I started working on this I checked what GCC does and it instantiates 
> those constraints during class completion as well. For example this CRTP 
> case: https://godbolt.org/z/EdoYf96zq. MSVC seem to do it as well.
> 
> So maybe it's OK to check the constraints of SMFs specifically?
> 
I think this is done on completeness already in this patch, unless i 
misunderstood the code.
I don't think doing it on demand is a great direction, as this does not only 
affect type traits but also code gen, etc. It would create instanciations in 
unexpected places. wouldn't it.
Does the standard has wording suggesting it should be done later than on type 
completeness?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D129222: [pseudo] Implement a guard to determine function declarator.

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added a comment.

I confirm reverting just this patch fixes the build for me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129222

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

cor3ntin wrote:
> royjacobson wrote:
> > erichkeane wrote:
> > > This seems problematic, doesn't it?  Checking this constraint will (once 
> > > I figure out how to get deferred instantiation to work) cause 
> > > instantiation, which can cause issues with incomplete types/CRTP/etc.
> > > 
> > > I think the result is that we cannot 'calculate' this until it is 
> > > queried, else we will cause incorrect errors.
> > Making this queried on demand is a relatively big change to how we handle 
> > type triviality, so I want to be sure we actually need to do this to be 
> > conformant.
> > 
> > When I started working on this I checked what GCC does and it instantiates 
> > those constraints during class completion as well. For example this CRTP 
> > case: https://godbolt.org/z/EdoYf96zq. MSVC seem to do it as well.
> > 
> > So maybe it's OK to check the constraints of SMFs specifically?
> > 
> I think this is done on completeness already in this patch, unless i 
> misunderstood the code.
> I don't think doing it on demand is a great direction, as this does not only 
> affect type traits but also code gen, etc. It would create instanciations in 
> unexpected places. wouldn't it.
> Does the standard has wording suggesting it should be done later than on type 
> completeness?
The problem, at least for the deferred concepts, is that it breaks in the CRTP 
as required to implement ranges.  So something like this: 
https://godbolt.org/z/hPqrcqhx5 breaks.

I'm currently working to 'fix' that, so if this patch causes us to 'check' 
constraints early, it'll go back to breaking that example.  The example that 
Roy showed seems to show that it is actually checking 'delayed' right now (that 
is, on demand) in GCC/MSVC.  I don't know of the consequence/standardeeze that 
causes that though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Corentin Jabot via Phabricator via cfe-commits

cor3ntin added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

erichkeane wrote:
> cor3ntin wrote:
> > royjacobson wrote:
> > > erichkeane wrote:
> > > > This seems problematic, doesn't it?  Checking this constraint will 
> > > > (once I figure out how to get deferred instantiation to work) cause 
> > > > instantiation, which can cause issues with incomplete types/CRTP/etc.
> > > > 
> > > > I think the result is that we cannot 'calculate' this until it is 
> > > > queried, else we will cause incorrect errors.
> > > Making this queried on demand is a relatively big change to how we handle 
> > > type triviality, so I want to be sure we actually need to do this to be 
> > > conformant.
> > > 
> > > When I started working on this I checked what GCC does and it 
> > > instantiates those constraints during class completion as well. For 
> > > example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC seem to do 
> > > it as well.
> > > 
> > > So maybe it's OK to check the constraints of SMFs specifically?
> > > 
> > I think this is done on completeness already in this patch, unless i 
> > misunderstood the code.
> > I don't think doing it on demand is a great direction, as this does not 
> > only affect type traits but also code gen, etc. It would create 
> > instanciations in unexpected places. wouldn't it.
> > Does the standard has wording suggesting it should be done later than on 
> > type completeness?
> The problem, at least for the deferred concepts, is that it breaks in the 
> CRTP as required to implement ranges.  So something like this: 
> https://godbolt.org/z/hPqrcqhx5 breaks.
> 
> I'm currently working to 'fix' that, so if this patch causes us to 'check' 
> constraints early, it'll go back to breaking that example.  The example that 
> Roy showed seems to show that it is actually checking 'delayed' right now 
> (that is, on demand) in GCC/MSVC.  I don't know of the 
> consequence/standardeeze that causes that though.
Thanks, 
Follow up stupid question then, do we care about the triviality of dependant 
types?
I think doing the check on complete non-dependant types might be a better 
solution than trying to do it lazily on triviality checks?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D130089: update-test-checks: safely handle tests with #if's

2022-07-20 Thread Matheus Izvekov via Phabricator via cfe-commits

mizvekov added a comment.

This seems to have introduced a test which always fails on windows:

  $ ":" "RUN: at line 4"
  $ "cp" "clang\test\utils\update_cc_test_checks/Inputs/ifdef.c" 
"build\llvm\tools\clang\test\utils\update_cc_test_checks\Output\ifdef.test.tmp.c"
  $ "C:/Program Files (x86)/Microsoft Visual 
Studio/Shared/Python39_64/python.exe" "llvm\utils\update_cc_test_checks.py" 
"--clang" "build\llvm\RelWithDebInfo\bin\clang" "--opt" 
"build\llvm\RelWithDebInfo\bin\opt" 
"build\llvm\tools\clang\test\utils\update_cc_test_checks\Output\ifdef.test.tmp.c"
  $ ":" "RUN: at line 5"
  $ "diff" "-u" 
"clang\test\utils\update_cc_test_checks/Inputs/ifdef.c.expected" 
"build\llvm\tools\clang\test\utils\update_cc_test_checks\Output\ifdef.test.tmp.c"
  # command output:
  --- clang\test\utils\update_cc_test_checks/Inputs/ifdef.c.expected
  +++ 
build\llvm\tools\clang\test\utils\update_cc_test_checks\Output\ifdef.test.tmp.c
  @@ -1,21 +1,21 @@
  -// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
  -// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK %s
  -// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -DFOO 
| FileCheck -check-prefixes=CHECK,FOO %s
  -
  -#ifdef FOO
  -// FOO-LABEL: @foo(
  -// FOO-NEXT:  entry:
  -// FOO-NEXT:ret i32 1
  -//
  -int foo() {
  -  return 1;
  -}
  -#endif
  -
  -// CHECK-LABEL: @bar(
  -// CHECK-NEXT:  entry:
  -// CHECK-NEXT:ret i32 2
  -//
  -int bar() {
  -  return 2;
  -}
  +// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
  +// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s | 
FileCheck -check-prefixes=CHECK %s
  +// RUN: %clang_cc1 -triple=x86_64-unknown-linux-gnu -emit-llvm -o - %s -DFOO 
| FileCheck -check-prefixes=CHECK,FOO %s
  +
  +#ifdef FOO
  +// FOO-LABEL: @foo(
  +// FOO-NEXT:  entry:
  +// FOO-NEXT:ret i32 1
  +//
  +int foo() {
  +  return 1;
  +}
  +#endif
  +
  +// CHECK-LABEL: @bar(
  +// CHECK-NEXT:  entry:
  +// CHECK-NEXT:ret i32 2
  +//
  +int bar() {
  +  return 2;
  +}
  
  error: command failed with exit status: 1
  
  --
  
  
  Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
  
  Failed Tests (1):
Clang :: utils/update_cc_test_checks/ifdef.test
  
  
  Testing Time: 0.28s
Failed: 1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130089

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

cor3ntin wrote:
> erichkeane wrote:
> > cor3ntin wrote:
> > > royjacobson wrote:
> > > > erichkeane wrote:
> > > > > This seems problematic, doesn't it?  Checking this constraint will 
> > > > > (once I figure out how to get deferred instantiation to work) cause 
> > > > > instantiation, which can cause issues with incomplete types/CRTP/etc.
> > > > > 
> > > > > I think the result is that we cannot 'calculate' this until it is 
> > > > > queried, else we will cause incorrect errors.
> > > > Making this queried on demand is a relatively big change to how we 
> > > > handle type triviality, so I want to be sure we actually need to do 
> > > > this to be conformant.
> > > > 
> > > > When I started working on this I checked what GCC does and it 
> > > > instantiates those constraints during class completion as well. For 
> > > > example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC seem to 
> > > > do it as well.
> > > > 
> > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > 
> > > I think this is done on completeness already in this patch, unless i 
> > > misunderstood the code.
> > > I don't think doing it on demand is a great direction, as this does not 
> > > only affect type traits but also code gen, etc. It would create 
> > > instanciations in unexpected places. wouldn't it.
> > > Does the standard has wording suggesting it should be done later than on 
> > > type completeness?
> > The problem, at least for the deferred concepts, is that it breaks in the 
> > CRTP as required to implement ranges.  So something like this: 
> > https://godbolt.org/z/hPqrcqhx5 breaks.
> > 
> > I'm currently working to 'fix' that, so if this patch causes us to 'check' 
> > constraints early, it'll go back to breaking that example.  The example 
> > that Roy showed seems to show that it is actually checking 'delayed' right 
> > now (that is, on demand) in GCC/MSVC.  I don't know of the 
> > consequence/standardeeze that causes that though.
> Thanks, 
> Follow up stupid question then, do we care about the triviality of dependant 
> types?
> I think doing the check on complete non-dependant types might be a better 
> solution than trying to do it lazily on triviality checks?
I would think it would be not a problem on non-dependent types.  BUT concepts 
are only allowed on templated functions (note not only on function-templates!) 
anyway, so I don't think this would be a problem?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D130130: [clang-tidy] Add an IgnoreMacros option to readability-avoid-const-params-in-decls

2022-07-20 Thread Nathan James via Phabricator via cfe-commits

njames93 added a comment.

Please can you upload your diff with full context(or use arcanist which does it 
for you).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130130

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

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-07-20 Thread Eugene Zelenko via Phabricator via cfe-commits

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst:52
+  many lines. Default value is `10`.
\ No newline at end of file


Please add newline.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

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

[PATCH] D129784: [HIP] Allow the new driver to compile HIP in non-RDC mode

2022-07-20 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 added a comment.

ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129784

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

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

2022-07-20 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added a comment.

In D130096#3663411 , @arsenm wrote:

> In D130096#3663398 , @jhuber6 wrote:
>
>> In D130096#3663295 , @yaxunl wrote:
>>
>>> There is no constant propagation for globals with weak linage, right? 
>>> Otherwise, it won't work. My concern is that there may be optimization 
>>> passes which do not respect the weak linkage and uses the incorrect default 
>>> value for OpenCL or HIP. Therefore I am not very confident to enable this 
>>> for OpenCL or HIP unless these variables have the correct value based on 
>>> the compilation options.
>>
>> Instead of `weak_odr` we could probably use add this to compiler used 
>> instead if that's an issue.
>
> the libraries get internalized as-is. Why does this need to be weak_odr?

The current patch does not consider HIP/OpenCL compile options, therefore the 
value of these variables are not correct for OpenCL/HIP. They need to be 
overridden by the variables with the same name in device libraries by clang 
through -mlink-builtin-bitcode.

If the patch check HIP/OpenCL compilation options to set the correct value for 
these variables, then it does not need weak linkage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130096

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

[PATCH] D128955: [WPD] Use new llvm.public.type.test intrinsic for potentially publicly visible classes

2022-07-20 Thread Teresa Johnson via Phabricator via cfe-commits

tejohnson added inline comments.



Comment at: clang/test/CodeGenCXX/thinlto-distributed-type-metadata.cpp:22
 // OPT-NOT: @llvm.type.test
-// OPT-NOT: call void @llvm.assume
 // We should have only one @llvm.assume call, the one that was expanded

aeubanks wrote:
> tejohnson wrote:
> > Why remove this one here and below?
> we end up RAUWing the `public.type.test` with true and end up with 
> `assume(true)` which is harmless and gets removed by something like 
> instcombine
Sure, but it should still be gone and can confirm that here? If it isn't useful 
to check for, then the comment should probably be changed too since it mentions 
assumes.



Comment at: clang/test/CodeGenCXX/thinlto_public_type_test_distributed.ll:23
+; RUN:   -o %t.native.o -x ir %t.o --save-temps=obj
+; RUN: llvm-dis %t.native.o.1.promote.bc -o - | FileCheck %s 
--check-prefix=HIDDEN
+

Is there a reason why the hidden version checks after promote but the public 
version above checks after importing?



Comment at: clang/test/CodeGenCXX/type-metadata.cpp:212
   // CFI-NVT-NOT: call i1 @llvm.type.test
-  // CFI-VT: [[P:%[^ ]*]] = call i1 @llvm.type.test
+  // CFI-VT-ITANIUM: [[P:%[^ ]*]] = call i1 @llvm.type.test
+  // CFI-VT-MS: [[P:%[^ ]*]] = call i1 @llvm.type.test

Is the CFI-VT-ITANIUM and CFI-VT-MS split needed? I don't see any differences 
in their checking. Can we go back to just a single CFI-VT here?



Comment at: lld/test/ELF/lto/update_public_type_test.ll:1
+; REQUIRES: x86
+

aeubanks wrote:
> tejohnson wrote:
> > Add comments about what is being tested. Also good to test via llvm-lto2 
> > which has a similar -whole-program-visibility option, rather than just via 
> > lld here. See for example llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll.
> added comments
> 
> I extended llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll, is that ok?
Yeah sure that's fine. But maybe also add a direct check there in both cases 
after promote that the public.type.test/assume were transformed as expected as 
you do in this test.



Comment at: llvm/lib/Analysis/ModuleSummaryAnalysis.cpp:169
+  case Intrinsic::public_type_test: {
+// errs() << "HIHI " << CI->getParent()->getParent()->getName() <<"\n";
+// CI->dump();

Remove commented debugging code here and below



Comment at: llvm/lib/LTO/LTO.cpp:1106
 DynamicExportSymbols);
+  updatePublicTypeTestCalls(*RegularLTO.CombinedModule,
+Conf.HasWholeProgramVisibility);

Probably need to do for the legacy LTO API as well, which also calls  
updateVCallVisibilityInModule (LTOCodeGenerator.cpp). This is used both some 
other linkers such as ld64. Otherwise I think the public.type.test intrinsics 
will never get converted for those LTO users, leading to errors probably? You 
can test the legacy LTO via llvm-lto (e.g. see tests in llvm/test/LTO/X86).



Comment at: llvm/lib/LTO/LTOBackend.cpp:565
+  updatePublicTypeTestCalls(Mod,
+Conf.HasWholeProgramVisibility ||
+CombinedIndex.withWholeProgramVisibility());

Can we just check the combined index flag now?



Comment at: llvm/lib/LTO/LTOBackend.cpp:595
 
+  updatePublicTypeTestCalls(Mod, Conf.HasWholeProgramVisibility);
+

aeubanks wrote:
> tejohnson wrote:
> > aeubanks wrote:
> > > I'm not sure where the best place to put these is
> > I don't think that this will work for distributed ThinLTO, where the 
> > ThinLTO Backends are run via clang, which isn't going to have this config 
> > variable set (since it is set only during LTO linking). I think something 
> > may need to be recorded in the index as a flag there at thin link / 
> > indexing time that can be checked here.
> > 
> > It would then be nice to consolidate this handling into WPD itself, e.g. at 
> > the start of DevirtModule::run, but unfortunately we won't have a summary 
> > index for pure regular LTO WPD so I don't think that would work. So in here 
> > is ok but I would do it early to handle some of the early return cases.
> > 
> > (Please add a distributed ThinLTO test too)
> Added a distributed ThinLTO test: 
> clang/test/CodeGenCXX/thinlto_public_type_test_distributed.ll.
> 
> I added `ModuleSummaryIndex::WithWholeProgramVisibility` but I'm not sure 
> where I'd set it, and that's causing the test to fail.
I see from your most recent update that you added code to set/check this new 
index flag. But I think you would want to set it around the same place where we 
update the vcall visibility during the thin link (see calls to 
updateVCallVisibilityInIndex). And I would do it via a new method in the WPD 
source file that uses the existing hasWholeProgramVisibility() that checks the 
OR of the config fla

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-07-20 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 446197.
njames93 added a comment.

Add option to wrap early exit in braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-braces.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
@@ -0,0 +1,158 @@
+// RUN: %check_clang_tidy %s readability-use-early-exits -format-style=llvm %t -- \
+// RUN:   -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 2}}"
+
+// Just to hit the threshold
+void padLines();
+
+void nomralFunc(int *A) {
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+  if (A) {
+*A = 10;
+  }
+  //   CHECK-FIXES: if (!A)
+  //  CHECK-FIXES-NEXT: return;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: *A = 10;
+}
+
+void funcWithTrailing(int *B) {
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+  if (B) {
+*B = 10;
+  }
+  return;
+  ;
+  //   CHECK-FIXES: if (!B)
+  //  CHECK-FIXES-NEXT: return;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: *B = 10;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: return;
+  //  CHECK-FIXES-NEXT: ;
+}
+
+void normal(int A, int B) {
+  
+  while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+if (A == B) {
+  padLines();
+}
+  }
+  //   CHECK-FIXES:  while (true) {
+  //  CHECK-FIXES-NEXT: if (A != B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: padLines();
+  //  CHECK-FIXES-NEXT: }
+
+  // Eat continue and empty nulls
+  while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+if (A != B) {
+  padLines();
+}
+continue;
+;
+continue;
+  } 
+  //   CHECK-FIXES:  while (true) {
+  //  CHECK-FIXES-NEXT: if (A == B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: padLines();
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT:   ;
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT: }
+}
+
+void toShort(int A, int B) {
+  while (true) {
+if (A == B) {
+}
+  }
+}
+
+void hasElse(int A, int B) {
+  while (true) {
+if (A == B) {
+
+} else {
+}
+  }
+}
+
+void hasTrailingStmt(int A, int B) {
+  while (true) {
+if (A == B) {
+}
+padLines();
+  }
+}
+
+void nested(int A, int B) {
+  // if (A > B) {
+  // CHECK-MESSAGES: [[@LINE+6]]:5: warning: use early exit
+  // if (B < A) {
+  // CHECK-MESSAGES: [[@LINE+5]]:7: warning: use early exit
+  // if (A == B) {
+  // CHECK-MESSAGES: [[@LINE+4]]:9: warning: use early exit
+  while (true) { // Nested
+if (A > B) {
+  if (B < A) {
+if (A != B) {
+  padLines();
+}
+  }
+}
+  } // EndLoop
+  //   CHECK-FIXES: while (true) { // Nested
+  //  CHECK-FIXES-NEXT: if (A <= B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: if (B >= A)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: if (A == B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: padLines();
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: } // EndLoop
+}
+
+void badNested(bool B) {
+  // Ensure check doesn't check for nested `if` if outer `if` has else.
+  while (true) {
+if (B) {
+  if (B) {
+  }
+} else {
+}
+  }
+}
+
+void semiNested(int A, int B) {
+  // CHECK-MESSAGES: [[@LINE+2]]:5: warning: use early exit
+  while (true) { // SemiNested
+if (A > B) {
+  if (B < A) {
+if (A != B) {
+  padLines();
+}
+  } else {
+  }
+}
+  }
+  //   CHECK-FIXES: while (true) { // SemiNested
+  //  CHECK-FIXES-NEXT:   if (A <= B)
+  //  CHECK-FIXES-NEXT: continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT:   if (B < A) {
+  //  CHECK-FIXES-NEXT: if (A != B) {
+  //  CHECK-FIXES-NEXT:   padLines();
+  //  CHECK-FIXES-NEXT: }
+  //  CHECK-FIXES-NEXT: } else {
+  //  CHECK-FIXES-NEXT: }
+  //  CHECK-FIXES-NEXT: }
+}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-braces.cpp

[PATCH] D130186: Addressing review comments.

2022-07-20 Thread Jacques Légaré via Phabricator via cfe-commits

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

Eliminated the (newly introduced) llvm::Function argument
of ShouldInstrumentFunction() and used CurFuncDecl instead.

Added in file name based exclusions, as the change had apparently
been left out of my previous submission.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130186

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h

Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -2338,7 +2338,7 @@
 
   /// ShouldInstrumentFunction - Return true if the current function should be
   /// instrumented with __cyg_profile_func_* calls
-  bool ShouldInstrumentFunction(llvm::Function *F);
+  bool ShouldInstrumentFunction();
 
   /// ShouldSkipSanitizerInstrumentation - Return true if the current function
   /// should not be instrumented with sanitizers.
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -30,10 +30,12 @@
 #include "clang/AST/StmtObjC.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TargetInfo.h"
 #include "clang/CodeGen/CGFunctionInfo.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Demangle/Demangle.h"
 #include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Dominators.h"
@@ -375,7 +377,7 @@
   // Emit function epilog (to return).
   llvm::DebugLoc Loc = EmitReturnBlock();
 
-  if (ShouldInstrumentFunction(CurFn)) {
+  if (ShouldInstrumentFunction()) {
 if (CGM.getCodeGenOpts().InstrumentFunctions)
   CurFn->addFnAttr("instrument-function-exit", "__cyg_profile_func_exit");
 if (CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining)
@@ -521,7 +523,7 @@
 
 /// ShouldInstrumentFunction - Return true if the current function should be
 /// instrumented with __cyg_profile_func_* calls
-bool CodeGenFunction::ShouldInstrumentFunction(llvm::Function *F) {
+bool CodeGenFunction::ShouldInstrumentFunction() {
   if (!CGM.getCodeGenOpts().InstrumentFunctions &&
   !CGM.getCodeGenOpts().InstrumentFunctionsAfterInlining &&
   !CGM.getCodeGenOpts().InstrumentFunctionEntryBare)
@@ -534,25 +536,56 @@
   // generate tracing for it: at link-time, the function will be gone and the
   // first argument to the __cyg_profile_* function will be unresolvable.
   const FunctionDecl *FDecl = dyn_cast(CurFuncDecl);
-  if (FDecl && FDecl->isInlined() &&
+  if (FDecl &&
+  FDecl->isInlined() &&
   !FDecl->isInlineDefinitionExternallyVisible())
 return false;
 
+  // The list of excluded file name substrings specified on the command line
+  // is searched to see if any of them is a substring the current function's
+  // declaration location.
+  static std::map CachedFilePaths;
+  SourceLocation DeclarationLocation = CurFuncDecl->getLocation();
+
+  // If the declaration location doesn't correspond directly to a file, it
+  // can't be excluded based file name.
+  if (DeclarationLocation.isFileID()) {
+if (CachedFilePaths.find(DeclarationLocation) == CachedFilePaths.end()) {
+  ASTContext &Context = CurFuncDecl->getASTContext();
+  const SourceManager &SM = Context.getSourceManager();
+
+  PresumedLoc PresumedLocation = SM.getPresumedLoc(DeclarationLocation);
+  CachedFilePaths[DeclarationLocation] = PresumedLocation.getFilename();
+}
+
+std::string Path = CachedFilePaths[DeclarationLocation];
+
+typedef std::vector::const_iterator ExcludedPathIterator;
+
+const std::vector &ExcludedPaths =
+  CGM.getCodeGenOpts().InstrumentFunctionsExcludedPaths;
+
+for (ExcludedPathIterator ExcludedPath = ExcludedPaths.begin(),
+   Sentinel = ExcludedPaths.end(); ExcludedPath != Sentinel; ++ExcludedPath)
+  if (Path.find(*ExcludedPath) != std::string::npos)
+return false;
+  }
+
   typedef std::vector::const_iterator ExcludedFunctionIterator;
 
   // The list of excluded function name substrings specified on the command-line
   // is searched to see if any of them is a substring of our (current) function
   // name.
   const std::vector &ExcludedFunctions =
-  CGM.getCodeGenOpts().InstrumentFunctionsExcludedFunctions;
+CGM.getCodeGenOpts().InstrumentFunctionsExcludedFunctions;
 
-  std::string FName(F->getName());
+  std::string MangledName(CurFn->getName());
+  std::string DemangledName(llvm::demangle(MangledName));
   for (ExcludedFunctionIterator ExcludedFunction = ExcludedFunctions.begin(),
-Sentinel = Excluded

[PATCH] D130186: Addressing review comments.

2022-07-20 Thread Jacques Légaré via Phabricator via cfe-commits

jlegare abandoned this revision.
jlegare added a comment.

This was not supposed to be committed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130186

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

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

2022-07-20 Thread Yaxun Liu via Phabricator via cfe-commits

yaxunl added inline comments.



Comment at: clang/lib/CodeGen/TargetInfo.cpp:9480
+  AddGlobal("__oclc_ISA_version", Minor + Major * 1000, 32);
+  AddGlobal("__oclc_ABI_version", 400, 32);
+}

jhuber6 wrote:
> yaxunl wrote:
> > should be determined by the code object version option.
> Yes I wasn't sure about this one. Could you elaborate where we derive that?
> Yes I wasn't sure about this one. Could you elaborate where we derive that?


CGM.getTarget().getTargetOpts().CodeObjectVersion


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130096

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

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-07-20 Thread Nathan James via Phabricator via cfe-commits

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp:217
+  LineCountThreshold(Options.get("LineCountThreshold", 10U)),
+  WrapInBraces(Options.get("WrapInBraces", false)) {}
+

I have an idea that we could infer this using the 
`readability-braces-around-statements` and its hicpp alias.
If enabled, probe the ShortStatementLines option to infer whether or not that 
check would fire if we make this transformation without using braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

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

[PATCH] D128462: [HLSL] add -I option for dxc mode.

2022-07-20 Thread Chris Bieneman via Phabricator via cfe-commits

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128462

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

[PATCH] D67025: Add .inl as valid C++ header type

2022-07-20 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added a comment.

In D67025#3665293 , @aaron.ballman 
wrote:

> this is why we have the `-x` option so that users can specify a specific 
> language mode to use. Is there a reason that option does not suffice for 
> clangd?

One reason is that the most commonly used tools that generate 
`compile_commands.json` files, like CMake, do not create entries for header 
files. Users need to turn to ad-hoc post-processing or third-party tools like 
CompDB  to add entries for header files, to 
even have a place to put the `-x c++header` option.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread Jacques Légaré via Phabricator via cfe-commits

jlegare updated this revision to Diff 446210.
jlegare added a comment.

Addressing review comments.

- Dropped argument to ShouldInstrumentFunction().
- Added file name based exclusion, which had accidentally been left out of 
previous commit.
- Added demangler.
- Applied git clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130161

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp


Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -536,8 +536,7 @@
   // generate tracing for it: at link-time, the function will be gone and the
   // first argument to the __cyg_profile_* function will be unresolvable.
   const FunctionDecl *FDecl = dyn_cast(CurFuncDecl);
-  if (FDecl &&
-  FDecl->isInlined() &&
+  if (FDecl && FDecl->isInlined() &&
   !FDecl->isInlineDefinitionExternallyVisible())
 return false;
 
@@ -563,10 +562,11 @@
 typedef std::vector::const_iterator ExcludedPathIterator;
 
 const std::vector &ExcludedPaths =
-  CGM.getCodeGenOpts().InstrumentFunctionsExcludedPaths;
+CGM.getCodeGenOpts().InstrumentFunctionsExcludedPaths;
 
 for (ExcludedPathIterator ExcludedPath = ExcludedPaths.begin(),
-   Sentinel = ExcludedPaths.end(); ExcludedPath != Sentinel; 
++ExcludedPath)
+  Sentinel = ExcludedPaths.end();
+ ExcludedPath != Sentinel; ++ExcludedPath)
   if (Path.find(*ExcludedPath) != std::string::npos)
 return false;
   }
@@ -577,12 +577,12 @@
   // is searched to see if any of them is a substring of our (current) function
   // name.
   const std::vector &ExcludedFunctions =
-CGM.getCodeGenOpts().InstrumentFunctionsExcludedFunctions;
+  CGM.getCodeGenOpts().InstrumentFunctionsExcludedFunctions;
 
   std::string MangledName(CurFn->getName());
   std::string DemangledName(llvm::demangle(MangledName));
   for (ExcludedFunctionIterator ExcludedFunction = ExcludedFunctions.begin(),
- Sentinel = ExcludedFunctions.end();
+Sentinel = ExcludedFunctions.end();
ExcludedFunction != Sentinel; ++ExcludedFunction)
 if (DemangledName.find(*ExcludedFunction) != std::string::npos)
   return false;


Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -536,8 +536,7 @@
   // generate tracing for it: at link-time, the function will be gone and the
   // first argument to the __cyg_profile_* function will be unresolvable.
   const FunctionDecl *FDecl = dyn_cast(CurFuncDecl);
-  if (FDecl &&
-  FDecl->isInlined() &&
+  if (FDecl && FDecl->isInlined() &&
   !FDecl->isInlineDefinitionExternallyVisible())
 return false;
 
@@ -563,10 +562,11 @@
 typedef std::vector::const_iterator ExcludedPathIterator;
 
 const std::vector &ExcludedPaths =
-  CGM.getCodeGenOpts().InstrumentFunctionsExcludedPaths;
+CGM.getCodeGenOpts().InstrumentFunctionsExcludedPaths;
 
 for (ExcludedPathIterator ExcludedPath = ExcludedPaths.begin(),
-   Sentinel = ExcludedPaths.end(); ExcludedPath != Sentinel; ++ExcludedPath)
+  Sentinel = ExcludedPaths.end();
+ ExcludedPath != Sentinel; ++ExcludedPath)
   if (Path.find(*ExcludedPath) != std::string::npos)
 return false;
   }
@@ -577,12 +577,12 @@
   // is searched to see if any of them is a substring of our (current) function
   // name.
   const std::vector &ExcludedFunctions =
-CGM.getCodeGenOpts().InstrumentFunctionsExcludedFunctions;
+  CGM.getCodeGenOpts().InstrumentFunctionsExcludedFunctions;
 
   std::string MangledName(CurFn->getName());
   std::string DemangledName(llvm::demangle(MangledName));
   for (ExcludedFunctionIterator ExcludedFunction = ExcludedFunctions.begin(),
- Sentinel = ExcludedFunctions.end();
+Sentinel = ExcludedFunctions.end();
ExcludedFunction != Sentinel; ++ExcludedFunction)
 if (DemangledName.find(*ExcludedFunction) != std::string::npos)
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO.

2022-07-20 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

Mostly looks good, with a nit in the test and some suggestion to the summary.

In D129401#3662857 , @quinnp wrote:

>> If this is for the legacy LTO interface, please state so.  `lld/*/LTO.cpp` 
>> sets `c.Options.DataSections = true;` to enable data sections by default.
>
> Hey @MaskRay, I'm not sure what is considered the legacy LTO interface, but 
> this change is to make the `libLTO` codegen match the behaviour of `LTO` used 
> through `lld` and `gold plugin`. Both `lld` and `gold plugin` turn on 
> `data-sections` for `LTO` by default:
>
> - as you mentioned `lld/*/LTO.cpp` sets `c.Options.DataSections = true;` by 
> default.
> - and `llvm/tools/gold/gold-plugin.cpp` sets `Conf.Options.DataSections = 
> SplitSections;` provided that the user did not explicitly set/unset 
> `data-sections` where `SplitSections` is `true` unless `gold plugin` is doing 
> a relocatable link.

There is a legacy LTO interface (see 
llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h) and a resolution-based 
interface.
Change libLTO in "This patch changes libLTO to set data-sections by default." 
to legacy LTO.

> This patch also fixes the forwarding of the clang options -fno-data-sections 
> and -fno-function-sections to libLTO

This sentence can keep using libLTO or LLVMLTO (the library is LLVMLTO per 
llvm/lib/LTO/CMakeLists.txt)




Comment at: llvm/test/LTO/PowerPC/data-sections-linux.ll:9
+; RUN:   %t/no-data-sections.o
+; RUN: obj2yaml %t/default.o | FileCheck --match-full-lines %s
+; RUN: obj2yaml %t/data-sections.o | FileCheck --match-full-lines %s

Using obj2yaml for symbol attributes is overkill and is less readable.

Use `llvm-objdump -t`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129401

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

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread Jacques Légaré via Phabricator via cfe-commits

jlegare added a comment.

@hans To address your concern about loose matching: I went with this approach 
because it fit my use-case. Do you think that an exact match would be more 
appropriate?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130161

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

[PATCH] D67025: Add .inl as valid C++ header type

2022-07-20 Thread Fangrui Song via Phabricator via cfe-commits

MaskRay added a comment.

I agree that this is not suitable in clang, but can be in clangd.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67025

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

[PATCH] D129536: [CUDA][FIX] Make shfl[_sync] for unsigned long long non-recursive

2022-07-20 Thread Artem Belevich via Phabricator via cfe-commits

tra added a comment.

In D129536#3663957 , @jdoerfert wrote:

> @tra, unsure about the crash. For me this passes fine (no gpu), is anything 
> missing?

The tests in the patch are running with `-emit-llvm`, so they are not actually 
lowering to NVPTX and that's where the failure happens. 
https://godbolt.org/z/cchaWxrhn

Comment at: clang/lib/Headers/__clang_cuda_intrinsics.h:237-238

-inline __device__ unsigned int
-__match64_any_sync(unsigned int mask, unsigned long long value) {
+inline __device__ unsigned int __match64_any_sync(unsigned int mask,
+  unsigned long long value) {
   return __nvvm_match_any_sync_i64(mask, value);

Nit: this change is irrelevant to the patch and can be removed.

Comment at: clang/test/CodeGenCUDA/shuffle_long_long.cu:9
+#undef __CUDA_ARCH__
+#define __CUDA_ARCH__ 300
+

This macro should not be set. If you do need something to be compiled for 
sm_30, you should've specified via `-target-cpu sm_30`.

Comment at: clang/test/CodeGenCUDA/shuffle_long_long.cu:14
+#define warpSize 32
+#include "__clang_cuda_intrinsics.h"
+

Nit: this should be `<...>` as we want the include to be found in compiler's 
include paths.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129536

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

[clang] 7e77d31 - [test] Remove unnecessary -verify-machineinstrs=0

2022-07-20 Thread Arthur Eubanks via cfe-commits


Author: Arthur Eubanks
Date: 2022-07-20T10:55:54-07:00
New Revision: 7e77d31af75e48e2ac7899026040a66fe601961a

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

LOG: [test] Remove unnecessary -verify-machineinstrs=0

Issue #38784 seems to be fixed and removing these doesn't cause any issues.

Added: 


Modified: 
clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
llvm/test/ThinLTO/X86/devirt.ll
llvm/test/ThinLTO/X86/devirt_vcall_vis_hidden.ll
llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
llvm/test/ThinLTO/X86/type_test_noindircall.ll

Removed: 




diff  --git a/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll 
b/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
index e02b30b392e17..959d89d61ab27 100644
--- a/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
+++ b/clang/test/CodeGen/thinlto-distributed-cfi-devirt.ll
@@ -6,10 +6,8 @@
 
 ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
 
-; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. 
PR39436.
 ; RUN: llvm-lto2 run -thinlto-distributed-indexes -disable-thinlto-funcattrs=0 
%t.o \
 ; RUN:   -whole-program-visibility \
-; RUN:   -verify-machineinstrs=0 \
 ; RUN:   -o %t2.index \
 ; RUN:   -r=%t.o,test,px \
 ; RUN:   -r=%t.o,_ZN1A1nEi,p \

diff  --git a/llvm/test/ThinLTO/X86/devirt.ll b/llvm/test/ThinLTO/X86/devirt.ll
index cf7d583069437..18bd34840bbfd 100644
--- a/llvm/test/ThinLTO/X86/devirt.ll
+++ b/llvm/test/ThinLTO/X86/devirt.ll
@@ -62,10 +62,8 @@
 ; RUN:   -r=%t2.o,_ZTV1C,px \
 ; RUN:   -r=%t2.o,_ZTV1D,px 2>&1 | FileCheck %s --check-prefix=SKIP
 
-; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. 
PR39436.
 ; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \
 ; RUN:   -whole-program-visibility \
-; RUN:   -verify-machineinstrs=0 \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t.o,test,px \
 ; RUN:   -r=%t.o,_ZN1A1nEi,p \

diff  --git a/llvm/test/ThinLTO/X86/devirt_vcall_vis_hidden.ll 
b/llvm/test/ThinLTO/X86/devirt_vcall_vis_hidden.ll
index c416b8f91d532..611be11ff8a7a 100644
--- a/llvm/test/ThinLTO/X86/devirt_vcall_vis_hidden.ll
+++ b/llvm/test/ThinLTO/X86/devirt_vcall_vis_hidden.ll
@@ -21,9 +21,7 @@
 ; Hybrid WPD
 ; Generate split module with summary for hybrid Thin/Regular LTO WPD.
 ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
-; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. 
PR39436.
 ; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \
-; RUN:   -verify-machineinstrs=0 \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t.o,test,px \
 ; RUN:   -r=%t.o,_ZN1A1nEi,p \

diff  --git a/llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll 
b/llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
index ef752a7d85f23..4451d17709416 100644
--- a/llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
+++ b/llvm/test/ThinLTO/X86/devirt_vcall_vis_public.ll
@@ -22,10 +22,8 @@
 ; Hybrid WPD
 ; Generate split module with summary for hybrid Thin/Regular LTO WPD.
 ; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t.o %s
-; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. 
PR39436.
 ; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \
 ; RUN:   -whole-program-visibility \
-; RUN:   -verify-machineinstrs=0 \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t.o,test,px \
 ; RUN:   -r=%t.o,_ZN1A1nEi,p \
@@ -80,7 +78,6 @@
 
 ; Hybrid WPD
 ; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \
-; RUN:   -verify-machineinstrs=0 \
 ; RUN:   -o %t3 \
 ; RUN:   -r=%t.o,test,px \
 ; RUN:   -r=%t.o,_ZN1A1nEi,p \

diff  --git a/llvm/test/ThinLTO/X86/type_test_noindircall.ll 
b/llvm/test/ThinLTO/X86/type_test_noindircall.ll
index 3a2badcaea693..2e2f9f8565968 100644
--- a/llvm/test/ThinLTO/X86/type_test_noindircall.ll
+++ b/llvm/test/ThinLTO/X86/type_test_noindircall.ll
@@ -6,10 +6,8 @@
 
 ; RUN: opt -thinlto-bc -o %t.o %s
 
-; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. 
PR39436.
 ; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \
 ; RUN:   -whole-program-visibility \
-; RUN:   -verify-machineinstrs=0 \
 ; RUN:  -r=%t.o,_ZTVN12_GLOBAL__N_18RealFileE,px \
 ; RUN:   -o %t2
 ; RUN: llvm-dis %t2.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR
@@ -18,7 +16,6 @@
 ; RUN: opt -thinlto-bc -thinlto-split-lto-unit=false -o %t3.o %s
 ; RUN: llvm-lto2 run %t.o -save-temps -pass-remarks=. \
 ; RUN:   -whole-program-visibility \
-; RUN:   -verify-machineinstrs=0 \
 ; RUN:  -r=%t.o,_ZTVN12_GLOBAL__N_18RealFileE,px \
 ; RUN:   -o %t4
 ; RUN: llvm-dis %t4.1.4.opt.bc -o - | FileCheck %s --check-prefix=CHECK-IR



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

[PATCH] D130123: Extend ptr32 support to be applied on typedef

2022-07-20 Thread Reid Kleckner via Phabricator via cfe-commits

rnk added inline comments.



Comment at: clang/lib/Sema/SemaType.cpp:7116
+  for (;;) {
+if (const TypedefType *TT = dyn_cast(Desugared)) {
+  Desugared = TT->desugar();

Ariel-Burton wrote:
> rnk wrote:
> > This seems like a good place to use getSingleStepDesugaredType to look 
> > through all type sugar (parens, typedefs, template substitutions, etc).
> > This seems like a good place to use getSingleStepDesugaredType to look 
> > through all type sugar (parens, typedefs, template substitutions, etc).
> 
> I'm not sure what you mean.  Could you expand a little, please?
Clang's AST has lots of "type sugar nodes". These are types which usually don't 
have any semantic meaning, they just carry source location information, like 
whether there was a typedef or extra parens in the type. AttributedType is also 
a type sugar node, so we cannot do a full desugaring here, we have to step 
through each node one at a time to accumulate the attributes.

Your code looks through one kind of type sugar, but this loop should probably 
be generalized to handle all kinds of type sugar. I think 
getSingleStepDesugaredType will do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130123

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

[clang] a73a84c - [HLSL] add -I option for dxc mode.

2022-07-20 Thread Xiang Li via cfe-commits


Author: Xiang Li
Date: 2022-07-20T11:03:22-07:00
New Revision: a73a84c44753402b33d619035d2c5acec3b04859

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

LOG: [HLSL] add -I option for dxc mode.

A new option -I is added for dxc mode.
It is just alias of existing cc1 -I option.

Reviewed By: beanz

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

Added: 
clang/test/Driver/dxc_I.hlsl

Modified: 
clang-tools-extra/clangd/CompileCommands.cpp
clang/include/clang/Driver/Options.h
clang/include/clang/Driver/Options.td
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/Clang.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CompileCommands.cpp 
b/clang-tools-extra/clangd/CompileCommands.cpp
index 39198d0711998..12aa9dacc2dfe 100644
--- a/clang-tools-extra/clangd/CompileCommands.cpp
+++ b/clang-tools-extra/clangd/CompileCommands.cpp
@@ -423,6 +423,8 @@ unsigned char getModes(const llvm::opt::Option &Opt) {
   if (!Opt.hasFlag(driver::options::NoDriverOption)) {
 if (Opt.hasFlag(driver::options::CLOption)) {
   Result |= DM_CL;
+} else if (Opt.hasFlag(driver::options::CLDXCOption)) {
+  Result |= DM_CL;
 } else {
   Result |= DM_GCC;
   if (Opt.hasFlag(driver::options::CoreOption)) {

diff  --git a/clang/include/clang/Driver/Options.h 
b/clang/include/clang/Driver/Options.h
index f9b9632ee7cbe..f7ee154b7a7ab 100644
--- a/clang/include/clang/Driver/Options.h
+++ b/clang/include/clang/Driver/Options.h
@@ -36,7 +36,8 @@ enum ClangFlags {
   FC1Option = (1 << 15),
   FlangOnlyOption = (1 << 16),
   DXCOption = (1 << 17),
-  Ignored = (1 << 18),
+  CLDXCOption = (1 << 18),
+  Ignored = (1 << 19),
 };
 
 enum ID {

diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 88b675ee8a2cb..3bd3550c9c60f 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -53,6 +53,10 @@ def CC1AsOption : OptionFlag;
 // are made available when the driver is running in DXC compatibility mode.
 def DXCOption : OptionFlag;
 
+// CLDXCOption - This is a cl.exe/dxc.exe compatibility option. Options with 
this flag
+// are made available when the driver is running in CL/DXC compatibility mode.
+def CLDXCOption : OptionFlag;
+
 // NoDriverOption - This option should not be accepted by the driver.
 def NoDriverOption : OptionFlag;
 
@@ -6355,7 +6359,7 @@ def defsym : Separate<["-"], "defsym">,
 // clang-cl Options
 
//===--===//
 
-def cl_Group : OptionGroup<"">, Flags<[CLOption]>,
+def cl_Group : OptionGroup<"">, Flags<[CLDXCOption]>,
   HelpText<"CL.EXE COMPATIBILITY OPTIONS">;
 
 def cl_compile_Group : OptionGroup<"">,
@@ -6385,6 +6389,9 @@ class CLIgnoredJoined : Option<["/", "-"], 
name, KIND_JOINED>,
 class CLJoinedOrSeparate : Option<["/", "-"], name,
   KIND_JOINED_OR_SEPARATE>, Group, Flags<[CLOption, NoXarchOption]>;
 
+class CLDXCJoinedOrSeparate : Option<["/", "-"], name,
+  KIND_JOINED_OR_SEPARATE>, Group, Flags<[CLDXCOption, 
NoXarchOption]>;
+
 class CLCompileJoinedOrSeparate : Option<["/", "-"], name,
   KIND_JOINED_OR_SEPARATE>, Group,
   Flags<[CLOption, NoXarchOption]>;
@@ -6462,7 +6469,7 @@ def _SLASH_help : CLFlag<"help">, Alias,
 def _SLASH_HELP : CLFlag<"HELP">, Alias;
 def _SLASH_hotpatch : CLFlag<"hotpatch">, Alias,
   HelpText<"Create hotpatchable image">;
-def _SLASH_I : CLJoinedOrSeparate<"I">,
+def _SLASH_I : CLDXCJoinedOrSeparate<"I">,
   HelpText<"Add directory to include search path">, MetaVarName<"">,
   Alias;
 def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index bd2b9a5b39b39..e50fdd61423be 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -6262,6 +6262,7 @@ Driver::getIncludeExcludeOptionFlagMasks(bool 
IsClCompatMode) const {
   if (IsClCompatMode) {
 // Include CL and Core options.
 IncludedFlagsBitmask |= options::CLOption;
+IncludedFlagsBitmask |= options::CLDXCOption;
 IncludedFlagsBitmask |= options::CoreOption;
   } else {
 ExcludedFlagsBitmask |= options::CLOption;
@@ -6269,10 +6270,14 @@ Driver::getIncludeExcludeOptionFlagMasks(bool 
IsClCompatMode) const {
   if (IsDXCMode()) {
 // Include DXC and Core options.
 IncludedFlagsBitmask |= options::DXCOption;
+IncludedFlagsBitmask |= options::CLDXCOption;
 IncludedFlagsBitmask |= options::CoreOption;
   } else {
 ExcludedFlagsBitmask |= options::DXCOption;
   }
+  if (!IsClCompatMode && !IsDXCMode())
+ExcludedFlagsBitmask |= options::CLDXCOption;
+
   return std::make_pair(IncludedFlagsBitmask, ExcludedFlagsBitmask);
 }
 

diff  --git a/clang/lib/Driver/

[PATCH] D128462: [HLSL] add -I option for dxc mode.

2022-07-20 Thread Xiang Li via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa73a84c44753: [HLSL] add -I option for dxc mode. (authored 
by python3kgae).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128462

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang/include/clang/Driver/Options.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/dxc_I.hlsl

Index: clang/test/Driver/dxc_I.hlsl
===
--- /dev/null
+++ clang/test/Driver/dxc_I.hlsl
@@ -0,0 +1,4 @@
+// RUN: %clang_dxc -I test  -### %s 2>&1 | FileCheck %s
+
+// Make sure -I send to cc1.
+// CHECK:"-I" "test"
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -3511,6 +3511,7 @@
   types::ID InputType) {
   const unsigned ForwardedArguments[] = {options::OPT_dxil_validator_version,
  options::OPT_D,
+ options::OPT_I,
  options::OPT_S,
  options::OPT_emit_llvm,
  options::OPT_disable_llvm_passes,
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -6262,6 +6262,7 @@
   if (IsClCompatMode) {
 // Include CL and Core options.
 IncludedFlagsBitmask |= options::CLOption;
+IncludedFlagsBitmask |= options::CLDXCOption;
 IncludedFlagsBitmask |= options::CoreOption;
   } else {
 ExcludedFlagsBitmask |= options::CLOption;
@@ -6269,10 +6270,14 @@
   if (IsDXCMode()) {
 // Include DXC and Core options.
 IncludedFlagsBitmask |= options::DXCOption;
+IncludedFlagsBitmask |= options::CLDXCOption;
 IncludedFlagsBitmask |= options::CoreOption;
   } else {
 ExcludedFlagsBitmask |= options::DXCOption;
   }
+  if (!IsClCompatMode && !IsDXCMode())
+ExcludedFlagsBitmask |= options::CLDXCOption;
+
   return std::make_pair(IncludedFlagsBitmask, ExcludedFlagsBitmask);
 }
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -53,6 +53,10 @@
 // are made available when the driver is running in DXC compatibility mode.
 def DXCOption : OptionFlag;
 
+// CLDXCOption - This is a cl.exe/dxc.exe compatibility option. Options with this flag
+// are made available when the driver is running in CL/DXC compatibility mode.
+def CLDXCOption : OptionFlag;
+
 // NoDriverOption - This option should not be accepted by the driver.
 def NoDriverOption : OptionFlag;
 
@@ -6355,7 +6359,7 @@
 // clang-cl Options
 //===--===//
 
-def cl_Group : OptionGroup<"">, Flags<[CLOption]>,
+def cl_Group : OptionGroup<"">, Flags<[CLDXCOption]>,
   HelpText<"CL.EXE COMPATIBILITY OPTIONS">;
 
 def cl_compile_Group : OptionGroup<"">,
@@ -6385,6 +6389,9 @@
 class CLJoinedOrSeparate : Option<["/", "-"], name,
   KIND_JOINED_OR_SEPARATE>, Group, Flags<[CLOption, NoXarchOption]>;
 
+class CLDXCJoinedOrSeparate : Option<["/", "-"], name,
+  KIND_JOINED_OR_SEPARATE>, Group, Flags<[CLDXCOption, NoXarchOption]>;
+
 class CLCompileJoinedOrSeparate : Option<["/", "-"], name,
   KIND_JOINED_OR_SEPARATE>, Group,
   Flags<[CLOption, NoXarchOption]>;
@@ -6462,7 +6469,7 @@
 def _SLASH_HELP : CLFlag<"HELP">, Alias;
 def _SLASH_hotpatch : CLFlag<"hotpatch">, Alias,
   HelpText<"Create hotpatchable image">;
-def _SLASH_I : CLJoinedOrSeparate<"I">,
+def _SLASH_I : CLDXCJoinedOrSeparate<"I">,
   HelpText<"Add directory to include search path">, MetaVarName<"">,
   Alias;
 def _SLASH_J : CLFlag<"J">, HelpText<"Make char type unsigned">,
Index: clang/include/clang/Driver/Options.h
===
--- clang/include/clang/Driver/Options.h
+++ clang/include/clang/Driver/Options.h
@@ -36,7 +36,8 @@
   FC1Option = (1 << 15),
   FlangOnlyOption = (1 << 16),
   DXCOption = (1 << 17),
-  Ignored = (1 << 18),
+  CLDXCOption = (1 << 18),
+  Ignored = (1 << 19),
 };
 
 enum ID {
Index: clang-tools-extra/clangd/CompileCommands.cpp
===
--- clang-tools-extra/clangd/CompileCommands.cpp
+++ clang-tools-extra/clangd/CompileCommands.cpp
@@ -423,6 +423,8 @@
   if (!Opt.hasFlag(driver::options::NoDriverOption)) {
 if (Opt.hasFlag(driver::options::CLOption)) {
   Result |= DM_CL;
+} else if (Opt.hasFlag(driver::options::CLDXCOption)) {
+  R

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

2022-07-20 Thread Joseph Huber via Phabricator via cfe-commits

jhuber6 added a comment.

In D130096#3666155 , @yaxunl wrote:

> The current patch does not consider HIP/OpenCL compile options, therefore the 
> value of these variables are not correct for OpenCL/HIP. They need to be 
> overridden by the variables with the same name in device libraries by clang 
> through -mlink-builtin-bitcode.
>
> If the patch check HIP/OpenCL compilation options to set the correct value 
> for these variables, then it does not need weak linkage.

Is we instead add it to `compiler.used` it should be propagated while staying 
alive for the linker https://godbolt.org/z/MG5n1MWWj. The downside is that this 
symbol will not be removed and a symbol to it will live in the binary. The 
symbol will have weak binding, so it won't cause any linker errors. But it's a 
little annoying to have things stick around like that. I'm considering making 
this code generation be controlled by a clang driver flag so we could 
potentially change behavior as needed there.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130096

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

[PATCH] D130161: Added command-line options to exclude functions from function instrumentation output.

2022-07-20 Thread Jacques Légaré via Phabricator via cfe-commits

jlegare abandoned this revision.
jlegare added a comment.

I appear to have butchered something in trying to apply a patch. I will revisit 
and resubmit later when I've straightened things out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130161

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

[PATCH] D130181: [clang-tidy] Add readability-use-early-exits check

2022-07-20 Thread Nathan James via Phabricator via cfe-commits

njames93 updated this revision to Diff 446219.
njames93 added a comment.

Added logic to infer WrapInBraces option if unspecified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130181

Files:
  clang-tools-extra/clang-tidy/readability/CMakeLists.txt
  clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp
  clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.cpp
  clang-tools-extra/clang-tidy/readability/UseEarlyExitsCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/use-early-exits.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits-braces.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/use-early-exits.cpp
@@ -0,0 +1,164 @@
+// RUN: %check_clang_tidy %s readability-use-early-exits -format-style=llvm %t -- \
+// RUN:   -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 2}}"
+
+// Run the check with the braces around statements check also enabled, but 
+// ShortStatementLines is set so that we shouldn't add braces.
+// RUN: %check_clang_tidy %s readability-use-early-exits,readability-braces-around-statements -format-style=llvm %t -- \
+// RUN:   -config="{CheckOptions: {readability-use-early-exits.LineCountThreshold: 2, \
+// RUN:readability-braces-around-statements.ShortStatementLines: 2}}"
+
+// Just to hit the threshold
+void padLines();
+
+void nomralFunc(int *A) {
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+  if (A) {
+*A = 10;
+  }
+  //   CHECK-FIXES: if (!A)
+  //  CHECK-FIXES-NEXT: return;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: *A = 10;
+}
+
+void funcWithTrailing(int *B) {
+  // CHECK-MESSAGES: [[@LINE+1]]:3: warning: use early exit
+  if (B) {
+*B = 10;
+  }
+  return;
+  ;
+  //   CHECK-FIXES: if (!B)
+  //  CHECK-FIXES-NEXT: return;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: *B = 10;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: return;
+  //  CHECK-FIXES-NEXT: ;
+}
+
+void normal(int A, int B) {
+  
+  while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+if (A == B) {
+  padLines();
+}
+  }
+  //   CHECK-FIXES:  while (true) {
+  //  CHECK-FIXES-NEXT: if (A != B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: padLines();
+  //  CHECK-FIXES-NEXT: }
+
+  // Eat continue and empty nulls
+  while (true) { // CHECK-MESSAGES: [[@LINE+1]]:5: warning: use early exit
+if (A != B) {
+  padLines();
+}
+continue;
+;
+continue;
+  } 
+  //   CHECK-FIXES:  while (true) {
+  //  CHECK-FIXES-NEXT: if (A == B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: padLines();
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT:   ;
+  //  CHECK-FIXES-NEXT:   continue;
+  //  CHECK-FIXES-NEXT: }
+}
+
+void toShort(int A, int B) {
+  while (true) {
+if (A == B) {
+}
+  }
+}
+
+void hasElse(int A, int B) {
+  while (true) {
+if (A == B) {
+
+} else {
+}
+  }
+}
+
+void hasTrailingStmt(int A, int B) {
+  while (true) {
+if (A == B) {
+}
+padLines();
+  }
+}
+
+void nested(int A, int B) {
+  // if (A > B) {
+  // CHECK-MESSAGES: [[@LINE+6]]:5: warning: use early exit
+  // if (B < A) {
+  // CHECK-MESSAGES: [[@LINE+5]]:7: warning: use early exit
+  // if (A == B) {
+  // CHECK-MESSAGES: [[@LINE+4]]:9: warning: use early exit
+  while (true) { // Nested
+if (A > B) {
+  if (B < A) {
+if (A != B) {
+  padLines();
+}
+  }
+}
+  } // EndLoop
+  //   CHECK-FIXES: while (true) { // Nested
+  //  CHECK-FIXES-NEXT: if (A <= B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: if (B >= A)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: if (A == B)
+  //  CHECK-FIXES-NEXT:   continue;
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: padLines();
+  // CHECK-FIXES-EMPTY:
+  //  CHECK-FIXES-NEXT: } // EndLoop
+}
+
+void badNested(bool B) {
+  // Ensure check doesn't check for nested `if` if outer `if` has else.
+  while (true) {
+if (B) {
+  if (B) {
+  }
+} else {
+}
+  }
+}
+
+void semiNested(int A, int B) {
+  // CHECK-MESSAGES: [[@LINE+2]]:5: warning: use early exit
+  while (true) { // SemiNested
+if (A > B) {
+  if (B < A) {
+if (A != B) {
+  padLines();
+}
+  } else {
+  }
+}
+  }
+  //   CHECK-FIXES: while (true) { // SemiNested
+  //  CHECK-FI

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits

royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

erichkeane wrote:
> cor3ntin wrote:
> > erichkeane wrote:
> > > cor3ntin wrote:
> > > > royjacobson wrote:
> > > > > erichkeane wrote:
> > > > > > This seems problematic, doesn't it?  Checking this constraint will 
> > > > > > (once I figure out how to get deferred instantiation to work) cause 
> > > > > > instantiation, which can cause issues with incomplete 
> > > > > > types/CRTP/etc.
> > > > > > 
> > > > > > I think the result is that we cannot 'calculate' this until it is 
> > > > > > queried, else we will cause incorrect errors.
> > > > > Making this queried on demand is a relatively big change to how we 
> > > > > handle type triviality, so I want to be sure we actually need to do 
> > > > > this to be conformant.
> > > > > 
> > > > > When I started working on this I checked what GCC does and it 
> > > > > instantiates those constraints during class completion as well. For 
> > > > > example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC seem to 
> > > > > do it as well.
> > > > > 
> > > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > > 
> > > > I think this is done on completeness already in this patch, unless i 
> > > > misunderstood the code.
> > > > I don't think doing it on demand is a great direction, as this does not 
> > > > only affect type traits but also code gen, etc. It would create 
> > > > instanciations in unexpected places. wouldn't it.
> > > > Does the standard has wording suggesting it should be done later than 
> > > > on type completeness?
> > > The problem, at least for the deferred concepts, is that it breaks in the 
> > > CRTP as required to implement ranges.  So something like this: 
> > > https://godbolt.org/z/hPqrcqhx5 breaks.
> > > 
> > > I'm currently working to 'fix' that, so if this patch causes us to 
> > > 'check' constraints early, it'll go back to breaking that example.  The 
> > > example that Roy showed seems to show that it is actually checking 
> > > 'delayed' right now (that is, on demand) in GCC/MSVC.  I don't know of 
> > > the consequence/standardeeze that causes that though.
> > Thanks, 
> > Follow up stupid question then, do we care about the triviality of 
> > dependant types?
> > I think doing the check on complete non-dependant types might be a better 
> > solution than trying to do it lazily on triviality checks?
> I would think it would be not a problem on non-dependent types.  BUT concepts 
> are only allowed on templated functions (note not only on 
> function-templates!) anyway, so I don't think this would be a problem?  
Erich, I'm a bit confused by your response. I think my example demonstrates 
that for default constructors (and other SMFs) GCC and MSVC instantiate the 
constraints on class completion and **not** on demand. This is what I would 
like to do as well, if we don't have a good reason not to. (For destructors, 
performing the checks is even explicit in the standard.)

Not doing this can introduce some REALLY bad edge cases. What does this do if 
we defer the triviality computation?

```c++

template 
struct Base {
  Base() = default;
  Base() requires (!std::is_trivial_v);
};

struct Child : Base { };
```
We defer the computation of the constraints on `Base`, and complete `Child` 
somehow, but if `Child` is complete then `std::is_trivial_v` should be 
well-formed, right? But we get a logical contradiction instead.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

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

[PATCH] D129654: [Clang][Driver] Fix include paths for `--sysroot /` on OpenBSD/FreeBSD

2022-07-20 Thread Egor Zhdan via Phabricator via cfe-commits

egorzhdan updated this revision to Diff 446223.
egorzhdan added a comment.

- Add a test for OpenBSD
- Modify existing test file instead of adding a new file


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129654

Files:
  clang/lib/Driver/ToolChains/FreeBSD.cpp
  clang/lib/Driver/ToolChains/OpenBSD.cpp
  clang/test/Driver/Inputs/basic_freebsd_libcxx_tree/usr/bin/.keep
  clang/test/Driver/Inputs/basic_freebsd_libcxx_tree/usr/include/c++/v1/.keep
  clang/test/Driver/Inputs/basic_freebsd_libcxx_tree/usr/lib/.keep
  clang/test/Driver/Inputs/basic_openbsd_libcxx_tree/usr/bin/.keep
  clang/test/Driver/Inputs/basic_openbsd_libcxx_tree/usr/include/c++/v1/.keep
  clang/test/Driver/Inputs/basic_openbsd_libcxx_tree/usr/lib/.keep
  clang/test/Driver/freebsd.cpp
  clang/test/Driver/openbsd.cpp

Index: clang/test/Driver/openbsd.cpp
===
--- clang/test/Driver/openbsd.cpp
+++ clang/test/Driver/openbsd.cpp
@@ -19,3 +19,23 @@
 // RUN: %clangxx %s -### -pg -o %t.o -target arm-unknown-openbsd 2>&1 \
 // RUN:   | FileCheck --check-prefix=CHECK-PG-CXX %s
 // CHECK-PG-CXX: "-lc++_p" "-lc++abi_p" "-lpthread_p" "-lm_p"
+
+// Test include paths with a sysroot.
+// RUN: %clangxx %s -### -fsyntax-only 2>&1 \
+// RUN: --target=amd64-pc-openbsd \
+// RUN: --sysroot=%S/Inputs/basic_openbsd_libcxx_tree \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-SYSROOT %s
+// CHECK-LIBCXX-SYSROOT: "-cc1"
+// CHECK-LIBCXX-SYSROOT-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LIBCXX-SYSROOT-SAME: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+
+// Test include paths when the sysroot path ends with `/`.
+// RUN: %clangxx %s -### -fsyntax-only 2>&1 \
+// RUN: --target=amd64-pc-openbsd \
+// RUN: --sysroot=%S/Inputs/basic_openbsd_libcxx_tree/ \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-SYSROOT-SLASH %s
+// CHECK-LIBCXX-SYSROOT-SLASH: "-cc1"
+// CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]"
+// CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/c++/v1"
Index: clang/test/Driver/freebsd.cpp
===
--- clang/test/Driver/freebsd.cpp
+++ clang/test/Driver/freebsd.cpp
@@ -20,3 +20,23 @@
 // CHECK-PG-FOURTEEN: "-lc++" "-lm"
 // CHECK-PG-TEN: "-lc++_p" "-lm_p"
 // CHECK-PG-NINE: "-lstdc++_p" "-lm_p"
+
+// Test include paths with a sysroot.
+// RUN: %clangxx %s -### -fsyntax-only 2>&1 \
+// RUN: --target=amd64-unknown-freebsd \
+// RUN: --sysroot=%S/Inputs/basic_openbsd_libcxx_tree \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-SYSROOT %s
+// CHECK-LIBCXX-SYSROOT: "-cc1"
+// CHECK-LIBCXX-SYSROOT-SAME: "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK-LIBCXX-SYSROOT-SAME: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/v1"
+
+// Test include paths when the sysroot path ends with `/`.
+// RUN: %clangxx %s -### -fsyntax-only 2>&1 \
+// RUN: --target=amd64-unknown-freebsd \
+// RUN: --sysroot=%S/Inputs/basic_openbsd_libcxx_tree/ \
+// RUN: -resource-dir=%S/Inputs/resource_dir \
+// RUN:   | FileCheck --check-prefix=CHECK-LIBCXX-SYSROOT-SLASH %s
+// CHECK-LIBCXX-SYSROOT-SLASH: "-cc1"
+// CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-isysroot" "[[SYSROOT:[^"]+/]]"
+// CHECK-LIBCXX-SYSROOT-SLASH-SAME: "-internal-isystem" "[[SYSROOT]]usr/include/c++/v1"
Index: clang/lib/Driver/ToolChains/OpenBSD.cpp
===
--- clang/lib/Driver/ToolChains/OpenBSD.cpp
+++ clang/lib/Driver/ToolChains/OpenBSD.cpp
@@ -284,7 +284,7 @@
 OpenBSD::OpenBSD(const Driver &D, const llvm::Triple &Triple,
  const ArgList &Args)
 : Generic_ELF(D, Triple, Args) {
-  getFilePaths().push_back(getDriver().SysRoot + "/usr/lib");
+  getFilePaths().push_back(concat(getDriver().SysRoot, "/usr/lib"));
 }
 
 void OpenBSD::AddClangSystemIncludeArgs(
@@ -317,13 +317,14 @@
 return;
   }
 
-  addExternCSystemInclude(DriverArgs, CC1Args, D.SysRoot + "/usr/include");
+  addExternCSystemInclude(DriverArgs, CC1Args,
+  concat(D.SysRoot, "/usr/include"));
 }
 
 void OpenBSD::addLibCxxIncludePaths(const llvm::opt::ArgList &DriverArgs,
 llvm::opt::ArgStringList &CC1Args) const {
   addSystemInclude(DriverArgs, CC1Args,
-   getDriver().SysRoot + "/usr/include/c++/v1");
+   concat(getDriver().SysRoot, "/usr/include/c++/v1"));
 }
 
 void OpenBSD::AddCXXStdlibLibArgs(const ArgList &Args,
Index: clang/lib/Driver/ToolChains/FreeBSD.cpp
===
--- clang/lib/Driver/ToolChains/FreeBSD.cpp
+++ clang/lib/Driver/ToolChains/FreeBSD.cpp
@@ -389,10 +389,1

[PATCH] D130190: [Driver] Error for -gsplit-dwarf with RISC-V linker relaxation

2022-07-20 Thread Fangrui Song via Phabricator via cfe-commits

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

-gsplit-dwarf produces a .dwo file which will not be processed by the linker. If
.dwo files contain relocations, they will not be resolved. Therefore the
practice is that .dwo files do not contain relocations. Since features like
address ranges need relocations and cannot be resolved without linking, split
DWARF is foundamentally incompatible with linker relaxation.

Currently there is a difficult-to-read MC error with -gsplit-dwarf with RISC-V 
-mrelax

  % clang --target=riscv64-linux-gnu -g -gsplit-dwarf -c a.c
  error: A dwo section may not contain relocations

Report a driver error instead.

Close https://github.com/llvm/llvm-project/issues/56642


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D130190

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/ToolChains/Arch/RISCV.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/Clang.h
  clang/test/Driver/riscv-features.c


Index: clang/test/Driver/riscv-features.c
===
--- clang/test/Driver/riscv-features.c
+++ clang/test/Driver/riscv-features.c
@@ -35,3 +35,10 @@
 // RUN: not %clang -cc1 -triple riscv64-unknown-elf -target-feature +e 2>&1 | 
FileCheck %s -check-prefix=RV64-WITH-E
 
 // RV64-WITH-E: error: invalid feature combination: standard user-level 
extension 'e' requires 'rv32'
+
+// RUN: not %clang -c --target=riscv64-linux-gnu -gsplit-dwarf %s 2>&1 | 
FileCheck %s --check-prefix=ERR-SPLIT-DWARF
+// RUN: not %clang -c --target=riscv64 -gsplit-dwarf=single %s 2>&1 | 
FileCheck %s --check-prefix=ERR-SPLIT-DWARF
+// RUN: %clang -### -c --target=riscv64 -mno-relax -g -gsplit-dwarf %s 2>&1 | 
FileCheck %s --check-prefix=SPLIT-DWARF
+
+// ERR-SPLIT-DWARF: error: -gsplit-dwarf{{.*}} is incompatible with RISC-V 
linker relaxation (-mrelax)
+// SPLIT-DWARF: "-split-dwarf-file"
Index: clang/lib/Driver/ToolChains/Clang.h
===
--- clang/lib/Driver/ToolChains/Clang.h
+++ clang/lib/Driver/ToolChains/Clang.h
@@ -198,6 +198,12 @@
 const char *LinkingOutput) const override;
 };
 
+enum class DwarfFissionKind { None, Split, Single };
+
+DwarfFissionKind getDebugFissionKind(const Driver &D,
+ const llvm::opt::ArgList &Args,
+ llvm::opt::Arg *&Arg);
+
 } // end namespace tools
 
 } // end namespace driver
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4051,9 +4051,7 @@
  options::OPT_fno_spell_checking);
 }
 
-enum class DwarfFissionKind { None, Split, Single };
-
-static DwarfFissionKind getDebugFissionKind(const Driver &D,
+DwarfFissionKind tools::getDebugFissionKind(const Driver &D,
 const ArgList &Args, Arg *&Arg) {
   Arg = Args.getLastArg(options::OPT_gsplit_dwarf, 
options::OPT_gsplit_dwarf_EQ,
 options::OPT_gno_split_dwarf);
Index: clang/lib/Driver/ToolChains/Arch/RISCV.cpp
===
--- clang/lib/Driver/ToolChains/Arch/RISCV.cpp
+++ clang/lib/Driver/ToolChains/Arch/RISCV.cpp
@@ -7,6 +7,7 @@
 
//===--===//
 
 #include "RISCV.h"
+#include "../Clang.h"
 #include "ToolChains/CommonArgs.h"
 #include "clang/Basic/CharInfo.h"
 #include "clang/Driver/Driver.h"
@@ -137,10 +138,15 @@
 Features.push_back("+reserve-x31");
 
   // -mrelax is default, unless -mno-relax is specified.
-  if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true))
+  if (Args.hasFlag(options::OPT_mrelax, options::OPT_mno_relax, true)) {
 Features.push_back("+relax");
-  else
+Arg *A;
+if (getDebugFissionKind(D, Args, A) != DwarfFissionKind::None)
+  D.Diag(clang::diag::err_drv_riscv_incompatible_with_linker_relaxation)
+  << A->getAsString(Args);
+  } else {
 Features.push_back("-relax");
+  }
 
   // GCC Compatibility: -mno-save-restore is default, unless -msave-restore is
   // specified.
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDrive

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

royjacobson wrote:
> erichkeane wrote:
> > cor3ntin wrote:
> > > erichkeane wrote:
> > > > cor3ntin wrote:
> > > > > royjacobson wrote:
> > > > > > erichkeane wrote:
> > > > > > > This seems problematic, doesn't it?  Checking this constraint 
> > > > > > > will (once I figure out how to get deferred instantiation to 
> > > > > > > work) cause instantiation, which can cause issues with incomplete 
> > > > > > > types/CRTP/etc.
> > > > > > > 
> > > > > > > I think the result is that we cannot 'calculate' this until it is 
> > > > > > > queried, else we will cause incorrect errors.
> > > > > > Making this queried on demand is a relatively big change to how we 
> > > > > > handle type triviality, so I want to be sure we actually need to do 
> > > > > > this to be conformant.
> > > > > > 
> > > > > > When I started working on this I checked what GCC does and it 
> > > > > > instantiates those constraints during class completion as well. For 
> > > > > > example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC seem 
> > > > > > to do it as well.
> > > > > > 
> > > > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > > > 
> > > > > I think this is done on completeness already in this patch, unless i 
> > > > > misunderstood the code.
> > > > > I don't think doing it on demand is a great direction, as this does 
> > > > > not only affect type traits but also code gen, etc. It would create 
> > > > > instanciations in unexpected places. wouldn't it.
> > > > > Does the standard has wording suggesting it should be done later than 
> > > > > on type completeness?
> > > > The problem, at least for the deferred concepts, is that it breaks in 
> > > > the CRTP as required to implement ranges.  So something like this: 
> > > > https://godbolt.org/z/hPqrcqhx5 breaks.
> > > > 
> > > > I'm currently working to 'fix' that, so if this patch causes us to 
> > > > 'check' constraints early, it'll go back to breaking that example.  The 
> > > > example that Roy showed seems to show that it is actually checking 
> > > > 'delayed' right now (that is, on demand) in GCC/MSVC.  I don't know of 
> > > > the consequence/standardeeze that causes that though.
> > > Thanks, 
> > > Follow up stupid question then, do we care about the triviality of 
> > > dependant types?
> > > I think doing the check on complete non-dependant types might be a better 
> > > solution than trying to do it lazily on triviality checks?
> > I would think it would be not a problem on non-dependent types.  BUT 
> > concepts are only allowed on templated functions (note not only on 
> > function-templates!) anyway, so I don't think this would be a problem?  
> Erich, I'm a bit confused by your response. I think my example demonstrates 
> that for default constructors (and other SMFs) GCC and MSVC instantiate the 
> constraints on class completion and **not** on demand. This is what I would 
> like to do as well, if we don't have a good reason not to. (For destructors, 
> performing the checks is even explicit in the standard.)
> 
> Not doing this can introduce some REALLY bad edge cases. What does this do if 
> we defer the triviality computation?
> 
> ```c++
> 
> template 
> struct Base {
>   Base() = default;
>   Base() requires (!std::is_trivial_v);
> };
> 
> struct Child : Base { };
> ```
> We defer the computation of the constraints on `Base`, and complete `Child` 
> somehow, but if `Child` is complete then `std::is_trivial_v` should be 
> well-formed, right? But we get a logical contradiction instead.
> 
> 
>Erich, I'm a bit confused by your response
It is entirely possible we're talking past eachother, or misunderstanding 
eachothers examples.  I'm totally open to that being part of this issue.

In that example, if we calculate the triviality at '`Base` Class completion', 
`Child` is not yet complete, right?  So the is_trivial_v would be UB.  If you 
instead require `sizeof(T)`, we typically give a diagnostic.

In this case, you'd at MINIMUM have to wait to calculate the requires-clause 
until after `Child` is complete.  And it isn't clear to me that we're delaying 
it until then.

That is what I intend to show with: https://godbolt.org/z/1YjsdY73P

As far as doing it 'on demand', I definitely see your circular argument here, 
but I think the is_trivial_v test is UB if we calculate it at `Base' completion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128619

___
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

2022-07-20 Thread Jonas Devlieghere via Phabricator via cfe-commits

JDevlieghere added a comment.

In D112374#3665989 , @mizvekov wrote:

> If anyone wants to take a look at the new changes to lldb tests, be my guest. 
> Otherwise I will try to land this again soon. It might well be that we figure 
> out some other in-tree user is affected, but I'd rather do that sooner than 
> later.

Please allow @teemperor some time to take a look at the llvm changes before 
landing this.


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] D129401: [libLTO] Set data-sections by default in libLTO.

2022-07-20 Thread Quinn Pham via Phabricator via cfe-commits

quinnp updated this revision to Diff 446228.
quinnp marked an inline comment as done.
quinnp added a comment.

Addressing review comments. Changing test cases to use `llvm-objdump -t` 
instead of `obj2yaml`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129401

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/test/Driver/forwarding-sections-liblto.c
  clang/test/Driver/gold-lto-sections.c
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/test/LTO/PowerPC/data-sections-aix.ll
  llvm/test/LTO/PowerPC/data-sections-linux.ll

Index: llvm/test/LTO/PowerPC/data-sections-linux.ll
===
--- /dev/null
+++ llvm/test/LTO/PowerPC/data-sections-linux.ll
@@ -0,0 +1,25 @@
+; RUN: rm -rf %t
+; RUN: mkdir %t
+; RUN: llvm-as %s -o %t/bc.bc
+; RUN: llvm-lto -exported-symbol var -O0 %t/bc.bc -o %t/default.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=1 %t/bc.bc -o \
+; RUN:   %t/data-sections.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=0 %t/bc.bc -o \
+; RUN:   %t/no-data-sections.o
+; RUN: llvm-objdump -t %t/default.o | FileCheck %s
+; RUN: llvm-objdump -t %t/data-sections.o | FileCheck %s
+; RUN: llvm-objdump -t %t/no-data-sections.o | FileCheck --check-prefix \
+; RUN:   CHECK-NO-DATA-SECTIONS %s
+
+target triple = "powerpc64le-unknown-linux-gnu"
+
+@var = global i32 0
+
+; CHECK:  SYMBOL TABLE:
+; CHECK:  .bss.var
+; CHECK-SAME: var
+
+; CHECK-NO-DATA-SECTIONS:  SYMBOL TABLE:
+; CHECK-NO-DATA-SECTIONS:  .bss
+; CHECK-NO-DATA-SECTIONS-NOT:  .var
+; CHECK-NO-DATA-SECTIONS-SAME: var
Index: llvm/test/LTO/PowerPC/data-sections-aix.ll
===
--- /dev/null
+++ llvm/test/LTO/PowerPC/data-sections-aix.ll
@@ -0,0 +1,25 @@
+; RUN: rm -rf %t
+; RUN: mkdir %t
+; RUN: llvm-as %s -o %t/bc.bc
+; RUN: llvm-lto -exported-symbol var -O0 %t/bc.bc -o %t/default.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=1 %t/bc.bc -o \
+; RUN:   %t/data-sections.o
+; RUN: llvm-lto -exported-symbol var -O0 --data-sections=0 %t/bc.bc -o \
+; RUN:   %t/no-data-sections.o
+; RUN: llvm-objdump -t %t/default.o | FileCheck %s
+; RUN: llvm-objdump -t %t/data-sections.o | FileCheck %s
+; RUN: llvm-objdump -t %t/no-data-sections.o | FileCheck --check-prefix \
+; RUN:   CHECK-NO-DATA-SECTIONS %s
+
+target triple = "powerpc-ibm-aix7.2.0.0"
+
+@var = global i32 0
+
+; CHECK:  SYMBOL TABLE:
+; CHECK:  .data
+; CHECK-NOT:  (csect: .data)
+; CHECK-SAME: var
+
+; CHECK-NO-DATA-SECTIONS:  SYMBOL TABLE:
+; CHECK-NO-DATA-SECTIONS:  .data (csect: .data)
+; CHECK-NO-DATA-SECTIONS-SAME: var
Index: llvm/lib/LTO/LTOCodeGenerator.cpp
===
--- llvm/lib/LTO/LTOCodeGenerator.cpp
+++ llvm/lib/LTO/LTOCodeGenerator.cpp
@@ -19,6 +19,7 @@
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Bitcode/BitcodeWriter.h"
+#include "llvm/CodeGen/CommandFlags.h"
 #include "llvm/CodeGen/ParallelCG.h"
 #include "llvm/CodeGen/TargetSubtargetInfo.h"
 #include "llvm/Config/config.h"
@@ -344,6 +345,11 @@
   Config.CPU = "cyclone";
   }
 
+  // If data-sections is not explicitly set or unset, set data-sections by
+  // default to match the behaviour of lld and gold plugin.
+  if (!codegen::getExplicitDataSections())
+Config.Options.DataSections = true;
+
   TargetMach = createTargetMachine();
   assert(TargetMach && "Unable to create target machine");
 
Index: clang/test/Driver/gold-lto-sections.c
===
--- clang/test/Driver/gold-lto-sections.c
+++ clang/test/Driver/gold-lto-sections.c
@@ -4,5 +4,5 @@
 // RUN: -Wl,-plugin-opt=foo -O3 \
 // RUN: -ffunction-sections -fdata-sections \
 // RUN: | FileCheck %s
-// CHECK: "-plugin-opt=-function-sections"
-// CHECK: "-plugin-opt=-data-sections"
+// CHECK: "-plugin-opt=-function-sections=1"
+// CHECK: "-plugin-opt=-data-sections=1"
Index: clang/test/Driver/forwarding-sections-liblto.c
===
--- /dev/null
+++ clang/test/Driver/forwarding-sections-liblto.c
@@ -0,0 +1,15 @@
+// RUN: touch %t.o
+// RUN: %clang %t.o -### -flto 2>&1 | FileCheck %s
+// RUN: %clang %t.o -### -flto 2>&1 -ffunction-sections -fdata-sections | \
+// RUN:   FileCheck %s --check-prefix=CHECK-SECTIONS
+// RUN: %clang %t.o -### -flto 2>&1 -fno-function-sections -fno-data-sections \
+// RUN:   | FileCheck %s --check-prefix=CHECK-NO-SECTIONS
+
+// CHECK-NOT: "-plugin-opt=-function-sections=1"
+// CHECK-NOT: "-plugin-opt=-function-sections=0"
+// CHECK-NOT: "-plugin-opt=-data-sections=1"
+// CHECK-NOT: "-plugin-opt=-data-sections=0"
+// CHECK-SECTIONS: "-plugin-opt=-function-sections=1"
+// CHECK-SECTIONS: "-plugin-opt=-data-sections=1"
+// CHECK-NO-SEC

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Roy Jacobson via Phabricator via cfe-commits

royjacobson added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

erichkeane wrote:
> royjacobson wrote:
> > erichkeane wrote:
> > > cor3ntin wrote:
> > > > erichkeane wrote:
> > > > > cor3ntin wrote:
> > > > > > royjacobson wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > This seems problematic, doesn't it?  Checking this constraint 
> > > > > > > > will (once I figure out how to get deferred instantiation to 
> > > > > > > > work) cause instantiation, which can cause issues with 
> > > > > > > > incomplete types/CRTP/etc.
> > > > > > > > 
> > > > > > > > I think the result is that we cannot 'calculate' this until it 
> > > > > > > > is queried, else we will cause incorrect errors.
> > > > > > > Making this queried on demand is a relatively big change to how 
> > > > > > > we handle type triviality, so I want to be sure we actually need 
> > > > > > > to do this to be conformant.
> > > > > > > 
> > > > > > > When I started working on this I checked what GCC does and it 
> > > > > > > instantiates those constraints during class completion as well. 
> > > > > > > For example this CRTP case: https://godbolt.org/z/EdoYf96zq. MSVC 
> > > > > > > seem to do it as well.
> > > > > > > 
> > > > > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > > > > 
> > > > > > I think this is done on completeness already in this patch, unless 
> > > > > > i misunderstood the code.
> > > > > > I don't think doing it on demand is a great direction, as this does 
> > > > > > not only affect type traits but also code gen, etc. It would create 
> > > > > > instanciations in unexpected places. wouldn't it.
> > > > > > Does the standard has wording suggesting it should be done later 
> > > > > > than on type completeness?
> > > > > The problem, at least for the deferred concepts, is that it breaks in 
> > > > > the CRTP as required to implement ranges.  So something like this: 
> > > > > https://godbolt.org/z/hPqrcqhx5 breaks.
> > > > > 
> > > > > I'm currently working to 'fix' that, so if this patch causes us to 
> > > > > 'check' constraints early, it'll go back to breaking that example.  
> > > > > The example that Roy showed seems to show that it is actually 
> > > > > checking 'delayed' right now (that is, on demand) in GCC/MSVC.  I 
> > > > > don't know of the consequence/standardeeze that causes that though.
> > > > Thanks, 
> > > > Follow up stupid question then, do we care about the triviality of 
> > > > dependant types?
> > > > I think doing the check on complete non-dependant types might be a 
> > > > better solution than trying to do it lazily on triviality checks?
> > > I would think it would be not a problem on non-dependent types.  BUT 
> > > concepts are only allowed on templated functions (note not only on 
> > > function-templates!) anyway, so I don't think this would be a problem?  
> > Erich, I'm a bit confused by your response. I think my example demonstrates 
> > that for default constructors (and other SMFs) GCC and MSVC instantiate the 
> > constraints on class completion and **not** on demand. This is what I would 
> > like to do as well, if we don't have a good reason not to. (For 
> > destructors, performing the checks is even explicit in the standard.)
> > 
> > Not doing this can introduce some REALLY bad edge cases. What does this do 
> > if we defer the triviality computation?
> > 
> > ```c++
> > 
> > template 
> > struct Base {
> >   Base() = default;
> >   Base() requires (!std::is_trivial_v);
> > };
> > 
> > struct Child : Base { };
> > ```
> > We defer the computation of the constraints on `Base`, and complete `Child` 
> > somehow, but if `Child` is complete then `std::is_trivial_v` should 
> > be well-formed, right? But we get a logical contradiction instead.
> > 
> > 
> >Erich, I'm a bit confused by your response
> It is entirely possible we're talking past eachother, or misunderstanding 
> eachothers examples.  I'm totally open to that being part of this issue.
> 
> In that example, if we calculate the triviality at '`Base` Class completion', 
> `Child` is not yet complete, right?  So the is_trivial_v would be UB.  If you 
> instead require `sizeof(T)`, we typically give a diagnostic.
> 
> In this case, you'd at MINIMUM have to wait to calculate the requires-clause 
> until after `Child` is complete.  And it isn't clear to me that we're 
> delaying it until then.
> 
> That is what I intend to show with: https://godbolt.org/z/1YjsdY73P
> 
> As far as doing it 'on demand', I definitely see your circular argument here, 
> but I think the is_trivial_v test is UB if we calculate it at `Base' 
> completion.
I'm arguing for checking the constraints at the completion of `Base`, and for 
making `is_trivial_v/sizeof` ill formed in this context.

Your GCC example is a bit misle

[PATCH] D129401: [libLTO] Set data-sections by default in libLTO.

2022-07-20 Thread Quinn Pham via Phabricator via cfe-commits

quinnp added a comment.

In D129401#3666238 , @MaskRay wrote:

> Mostly looks good, with a nit in the test and some suggestion to the summary.
>
> In D129401#3662857 , @quinnp wrote:
>
>>> If this is for the legacy LTO interface, please state so.  `lld/*/LTO.cpp` 
>>> sets `c.Options.DataSections = true;` to enable data sections by default.
>>
>> Hey @MaskRay, I'm not sure what is considered the legacy LTO interface, but 
>> this change is to make the `libLTO` codegen match the behaviour of `LTO` 
>> used through `lld` and `gold plugin`. Both `lld` and `gold plugin` turn on 
>> `data-sections` for `LTO` by default:
>>
>> - as you mentioned `lld/*/LTO.cpp` sets `c.Options.DataSections = true;` by 
>> default.
>> - and `llvm/tools/gold/gold-plugin.cpp` sets `Conf.Options.DataSections = 
>> SplitSections;` provided that the user did not explicitly set/unset 
>> `data-sections` where `SplitSections` is `true` unless `gold plugin` is 
>> doing a relocatable link.
>
> There is a legacy LTO interface (see 
> llvm/include/llvm/LTO/legacy/LTOCodeGenerator.h) and a resolution-based 
> interface.
> Change libLTO in "This patch changes libLTO to set data-sections by default." 
> to legacy LTO.
>
>> This patch also fixes the forwarding of the clang options -fno-data-sections 
>> and -fno-function-sections to libLTO
>
> This sentence can keep using libLTO or LLVMLTO (the library is LLVMLTO per 
> llvm/lib/LTO/CMakeLists.txt)

Ah I see, thank you @MaskRay! I've updated the testcases and the summary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129401

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

[PATCH] D128619: [Clang] Implement P0848 (Conditionally Trivial Special Member Functions)

2022-07-20 Thread Erich Keane via Phabricator via cfe-commits

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:17899
+  ConstraintSatisfaction Satisfaction;
+  if (S.CheckFunctionConstraints(Method, Satisfaction))
+SatisfactionStatus.push_back(false);

royjacobson wrote:
> erichkeane wrote:
> > royjacobson wrote:
> > > erichkeane wrote:
> > > > cor3ntin wrote:
> > > > > erichkeane wrote:
> > > > > > cor3ntin wrote:
> > > > > > > royjacobson wrote:
> > > > > > > > erichkeane wrote:
> > > > > > > > > This seems problematic, doesn't it?  Checking this constraint 
> > > > > > > > > will (once I figure out how to get deferred instantiation to 
> > > > > > > > > work) cause instantiation, which can cause issues with 
> > > > > > > > > incomplete types/CRTP/etc.
> > > > > > > > > 
> > > > > > > > > I think the result is that we cannot 'calculate' this until 
> > > > > > > > > it is queried, else we will cause incorrect errors.
> > > > > > > > Making this queried on demand is a relatively big change to how 
> > > > > > > > we handle type triviality, so I want to be sure we actually 
> > > > > > > > need to do this to be conformant.
> > > > > > > > 
> > > > > > > > When I started working on this I checked what GCC does and it 
> > > > > > > > instantiates those constraints during class completion as well. 
> > > > > > > > For example this CRTP case: https://godbolt.org/z/EdoYf96zq. 
> > > > > > > > MSVC seem to do it as well.
> > > > > > > > 
> > > > > > > > So maybe it's OK to check the constraints of SMFs specifically?
> > > > > > > > 
> > > > > > > I think this is done on completeness already in this patch, 
> > > > > > > unless i misunderstood the code.
> > > > > > > I don't think doing it on demand is a great direction, as this 
> > > > > > > does not only affect type traits but also code gen, etc. It would 
> > > > > > > create instanciations in unexpected places. wouldn't it.
> > > > > > > Does the standard has wording suggesting it should be done later 
> > > > > > > than on type completeness?
> > > > > > The problem, at least for the deferred concepts, is that it breaks 
> > > > > > in the CRTP as required to implement ranges.  So something like 
> > > > > > this: https://godbolt.org/z/hPqrcqhx5 breaks.
> > > > > > 
> > > > > > I'm currently working to 'fix' that, so if this patch causes us to 
> > > > > > 'check' constraints early, it'll go back to breaking that example.  
> > > > > > The example that Roy showed seems to show that it is actually 
> > > > > > checking 'delayed' right now (that is, on demand) in GCC/MSVC.  I 
> > > > > > don't know of the consequence/standardeeze that causes that though.
> > > > > Thanks, 
> > > > > Follow up stupid question then, do we care about the triviality of 
> > > > > dependant types?
> > > > > I think doing the check on complete non-dependant types might be a 
> > > > > better solution than trying to do it lazily on triviality checks?
> > > > I would think it would be not a problem on non-dependent types.  BUT 
> > > > concepts are only allowed on templated functions (note not only on 
> > > > function-templates!) anyway, so I don't think this would be a problem?  
> > > Erich, I'm a bit confused by your response. I think my example 
> > > demonstrates that for default constructors (and other SMFs) GCC and MSVC 
> > > instantiate the constraints on class completion and **not** on demand. 
> > > This is what I would like to do as well, if we don't have a good reason 
> > > not to. (For destructors, performing the checks is even explicit in the 
> > > standard.)
> > > 
> > > Not doing this can introduce some REALLY bad edge cases. What does this 
> > > do if we defer the triviality computation?
> > > 
> > > ```c++
> > > 
> > > template 
> > > struct Base {
> > >   Base() = default;
> > >   Base() requires (!std::is_trivial_v);
> > > };
> > > 
> > > struct Child : Base { };
> > > ```
> > > We defer the computation of the constraints on `Base`, and complete 
> > > `Child` somehow, but if `Child` is complete then 
> > > `std::is_trivial_v` should be well-formed, right? But we get a 
> > > logical contradiction instead.
> > > 
> > > 
> > >Erich, I'm a bit confused by your response
> > It is entirely possible we're talking past eachother, or misunderstanding 
> > eachothers examples.  I'm totally open to that being part of this issue.
> > 
> > In that example, if we calculate the triviality at '`Base` Class 
> > completion', `Child` is not yet complete, right?  So the is_trivial_v would 
> > be UB.  If you instead require `sizeof(T)`, we typically give a diagnostic.
> > 
> > In this case, you'd at MINIMUM have to wait to calculate the 
> > requires-clause until after `Child` is complete.  And it isn't clear to me 
> > that we're delaying it until then.
> > 
> > That is what I intend to show with: https://godbolt.org/z/1YjsdY73P
> > 
> > As far as doing it 'on demand', I definitely see your circular argument 
> > here, but I think the is_trivial_v test is UB

1 2 >

1 - 100 of 178 matches

Mail list logo