[clang-tools-extra] 0ab57bd - [include-cleaner] Fix the member-expr-access usage for sugar type.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-12-19T08:48:32+01:00
New Revision: 0ab57bdc9745bfc8147831c09ed05073f87e7040

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

LOG: [include-cleaner] Fix the member-expr-access usage for sugar type.

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

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

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/WalkAST.cpp
clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
clang/include/clang/AST/Type.h
clang/lib/AST/Type.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp 
b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index ff15e62efdfd5..cf2f0d7bcff63 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -27,16 +27,6 @@ using DeclCallback =
 class ASTWalker : public RecursiveASTVisitor {
   DeclCallback Callback;
 
-  bool handleTemplateName(SourceLocation Loc, TemplateName TN) {
-// For using-templates, only mark the alias.
-if (auto *USD = TN.getAsUsingShadowDecl()) {
-  report(Loc, USD);
-  return true;
-}
-report(Loc, TN.getAsTemplateDecl());
-return true;
-  }
-
   void report(SourceLocation Loc, NamedDecl *ND,
   RefType RT = RefType::Explicit) {
 if (!ND || Loc.isInvalid())
@@ -44,10 +34,33 @@ class ASTWalker : public RecursiveASTVisitor {
 Callback(Loc, *cast(ND->getCanonicalDecl()), RT);
   }
 
-  NamedDecl *resolveType(QualType Type) {
-if (Type->isPointerType())
-  Type = Type->getPointeeType();
-return Type->getAsRecordDecl();
+  NamedDecl *resolveTemplateName(TemplateName TN) {
+// For using-templates, only mark the alias.
+if (auto *USD = TN.getAsUsingShadowDecl())
+  return USD;
+return TN.getAsTemplateDecl();
+  }
+  NamedDecl *getMemberProvider(QualType Base) {
+if (Base->isPointerType())
+  Base = Base->getPointeeType();
+// Unwrap the sugar ElaboratedType.
+if (const auto *ElTy = dyn_cast(Base))
+  Base = ElTy->getNamedType();
+
+if (const auto *TT = dyn_cast(Base))
+  return TT->getDecl();
+if (const auto *UT = dyn_cast(Base))
+  return UT->getFoundDecl();
+// A heuristic: to resolve a template type to **only** its template name.
+// We're only using this method for the base type of MemberExpr, in general
+// the template provides the member, and the critical case 
`unique_ptr`
+// is supported (the base type is a Foo*).
+//
+// There are some exceptions that this heuristic could fail (dependent 
base,
+// dependent typealias), but we believe these are rare.
+if (const auto *TST = dyn_cast(Base))
+  return resolveTemplateName(TST->getTemplateName());
+return Base->getAsRecordDecl();
   }
 
 public:
@@ -59,12 +72,16 @@ class ASTWalker : public RecursiveASTVisitor {
   }
 
   bool VisitMemberExpr(MemberExpr *E) {
-// A member expr implies a usage of the class type
-// (e.g., to prevent inserting a header of base class when using base
-// members from a derived object).
+// Reporting a usage of the member decl would cause issues (e.g. force
+// including the base class for inherited members). Instead, we report a
+// usage of the base type of the MemberExpr, so that e.g. code
+// `returnFoo().bar` can keep #include "foo.h" (rather than inserting
+// "bar.h" for the underlying base type `Bar`).
+//
 // FIXME: support dependent types, e.g., "std::vector().size()".
 QualType Type = E->getBase()->IgnoreImpCasts()->getType();
-report(E->getMemberLoc(), resolveType(Type));
+// FIXME: this should report as implicit reference.
+report(E->getMemberLoc(), getMemberProvider(Type));
 return true;
   }
 
@@ -126,15 +143,17 @@ class ASTWalker : public RecursiveASTVisitor {
 
   bool VisitTemplateSpecializationTypeLoc(TemplateSpecializationTypeLoc TL) {
 // FIXME: Handle explicit specializations.
-return handleTemplateName(TL.getTemplateNameLoc(),
-  TL.getTypePtr()->getTemplateName());
+report(TL.getTemplateNameLoc(),
+   resolveTemplateName(TL.getTypePtr()->getTemplateName()));
+return true;
   }
 
   bool VisitDeducedTemplateSpecializationTypeLoc(
   DeducedTemplateSpecializationTypeLoc TL) {
 // FIXME: Handle specializations.
-return handleTemplateName(TL.getTemplateNameLoc(),
-  TL.getTypePtr()->getTemplateName());
+report(TL.getTemplateNameLoc(),
+   resolveTemplateName(TL.getTypePtr()->getTemplateName()));
+return true;
   }
 
   bool TraverseTemplateArgumentLoc(const TemplateArgumentLoc &TL) {
@@ -142,9 +161,11 @@ class ASTWalker : public Re

[PATCH] D140095: [include-cleaner] Fix the member-expr-access usage for sugar type.

[Haojian Wu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rG0ab57bdc9745: [include-cleaner] Fix the member-expr-access 
usage for sugar type. (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140095

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
  clang/include/clang/AST/Type.h
  clang/lib/AST/Type.cpp

Index: clang/lib/AST/Type.cpp
===
--- clang/lib/AST/Type.cpp
+++ clang/lib/AST/Type.cpp
@@ -524,6 +524,10 @@
   return getAsSugar(this);
 }
 
+template <> const UsingType *Type::getAs() const {
+  return getAsSugar(this);
+}
+
 template <> const TemplateSpecializationType *Type::getAs() const {
   return getAsSugar(this);
 }
Index: clang/include/clang/AST/Type.h
===
--- clang/include/clang/AST/Type.h
+++ clang/include/clang/AST/Type.h
@@ -2594,6 +2594,7 @@
 /// This will check for a TypedefType by removing any existing sugar
 /// until it reaches a TypedefType or a non-sugared type.
 template <> const TypedefType *Type::getAs() const;
+template <> const UsingType *Type::getAs() const;
 
 /// This will check for a TemplateSpecializationType by removing any
 /// existing sugar until it reaches a TemplateSpecializationType or a
Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -210,6 +210,25 @@
   };
   struct Foo {};)cpp",
"void test(unique_ptr &V) { V.^release(); }");
+  // Respect the sugar type (typedef, using-type).
+  testWalk(R"cpp(
+  namespace ns { struct Foo { int a; }; }
+  using $explicit^Bar = ns::Foo;)cpp",
+   "void test(Bar b) { b.^a; }");
+  testWalk(R"cpp(
+  namespace ns { struct Foo { int a; }; }
+  using ns::$explicit^Foo;)cpp",
+   "void test(Foo b) { b.^a; }");
+  testWalk(R"cpp(
+  namespace ns { struct Foo { int a; }; }
+  namespace ns2 { using Bar = ns::Foo; }
+  using ns2::$explicit^Bar;
+  )cpp",
+   "void test(Bar b) { b.^a; }");
+  testWalk(R"cpp(
+  namespace ns { template struct Foo { int a; }; }
+  using ns::$explicit^Foo;)cpp",
+   "void k(Foo b) { b.^a; }");
 }
 
 TEST(WalkAST, ConstructExprs) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -27,16 +27,6 @@
 class ASTWalker : public RecursiveASTVisitor {
   DeclCallback Callback;
 
-  bool handleTemplateName(SourceLocation Loc, TemplateName TN) {
-// For using-templates, only mark the alias.
-if (auto *USD = TN.getAsUsingShadowDecl()) {
-  report(Loc, USD);
-  return true;
-}
-report(Loc, TN.getAsTemplateDecl());
-return true;
-  }
-
   void report(SourceLocation Loc, NamedDecl *ND,
   RefType RT = RefType::Explicit) {
 if (!ND || Loc.isInvalid())
@@ -44,10 +34,33 @@
 Callback(Loc, *cast(ND->getCanonicalDecl()), RT);
   }
 
-  NamedDecl *resolveType(QualType Type) {
-if (Type->isPointerType())
-  Type = Type->getPointeeType();
-return Type->getAsRecordDecl();
+  NamedDecl *resolveTemplateName(TemplateName TN) {
+// For using-templates, only mark the alias.
+if (auto *USD = TN.getAsUsingShadowDecl())
+  return USD;
+return TN.getAsTemplateDecl();
+  }
+  NamedDecl *getMemberProvider(QualType Base) {
+if (Base->isPointerType())
+  Base = Base->getPointeeType();
+// Unwrap the sugar ElaboratedType.
+if (const auto *ElTy = dyn_cast(Base))
+  Base = ElTy->getNamedType();
+
+if (const auto *TT = dyn_cast(Base))
+  return TT->getDecl();
+if (const auto *UT = dyn_cast(Base))
+  return UT->getFoundDecl();
+// A heuristic: to resolve a template type to **only** its template name.
+// We're only using this method for the base type of MemberExpr, in general
+// the template provides the member, and the critical case `unique_ptr`
+// is supported (the base type is a Foo*).
+//
+// There are some exceptions that this heuristic could fail (dependent base,
+// dependent typealias), but we believe these are rare.
+if (const auto *TST = dyn_cast(Base))
+  return resolveTemplateName(TST->getTemplateName());
+return Base->getAsRecordDecl();
   }
 
 public:
@@ -59,12 +72,16 @@
   }
 
   bool VisitMemberExpr(MemberExpr *E) {
-// A member expr implies a usage of the class type
-// (e.g., to prevent inserting a header of base clas

[clang-tools-extra] 8551563 - [include-cleaner] Fix a missing review comment.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2022-12-19T09:14:33+01:00
New Revision: 8551563c0d77e54c3904fa853c32f82649f65fc1

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

LOG: [include-cleaner] Fix a missing review comment.

I forgot to add the change to the commit when committing.

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/WalkAST.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp 
b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index cf2f0d7bcff63..639add8f5c4be 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -42,10 +42,10 @@ class ASTWalker : public RecursiveASTVisitor {
   }
   NamedDecl *getMemberProvider(QualType Base) {
 if (Base->isPointerType())
-  Base = Base->getPointeeType();
+  return getMemberProvider(Base->getPointeeType());
 // Unwrap the sugar ElaboratedType.
 if (const auto *ElTy = dyn_cast(Base))
-  Base = ElTy->getNamedType();
+  return getMemberProvider(ElTy->getNamedType());
 
 if (const auto *TT = dyn_cast(Base))
   return TT->getDecl();



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

[PATCH] D140284: [include-cleaner] Base-type usage from member exprs is implicit.

[Haojian Wu via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140284

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -180,32 +180,32 @@
 }
 
 TEST(WalkAST, MemberExprs) {
-  testWalk("struct $explicit^S { void foo(); };", "void foo() { S{}.^foo(); 
}");
+  testWalk("struct $implicit^S { void foo(); };", "void foo() { S{}.^foo(); 
}");
   testWalk(
-  "struct S { void foo(); }; struct $explicit^X : S { using S::foo; };",
+  "struct S { void foo(); }; struct $implicit^X : S { using S::foo; };",
   "void foo() { X{}.^foo(); }");
-  testWalk("struct Base { int a; }; struct $explicit^Derived : public Base 
{};",
+  testWalk("struct Base { int a; }; struct $implicit^Derived : public Base 
{};",
"void fun(Derived d) { d.^a; }");
-  testWalk("struct Base { int a; }; struct $explicit^Derived : public Base 
{};",
+  testWalk("struct Base { int a; }; struct $implicit^Derived : public Base 
{};",
"void fun(Derived* d) { d->^a; }");
-  testWalk("struct Base { int a; }; struct $explicit^Derived : public Base 
{};",
+  testWalk("struct Base { int a; }; struct $implicit^Derived : public Base 
{};",
"void fun(Derived& d) { d.^a; }");
-  testWalk("struct Base { int a; }; struct $explicit^Derived : public Base 
{};",
+  testWalk("struct Base { int a; }; struct $implicit^Derived : public Base 
{};",
"void fun() { Derived().^a; }");
-  testWalk("struct Base { int a; }; struct $explicit^Derived : public Base 
{};",
+  testWalk("struct Base { int a; }; struct $implicit^Derived : public Base 
{};",
"Derived foo(); void fun() { foo().^a; }");
-  testWalk("struct Base { int a; }; struct $explicit^Derived : public Base 
{};",
+  testWalk("struct Base { int a; }; struct $implicit^Derived : public Base 
{};",
"Derived& foo(); void fun() { foo().^a; }");
   testWalk(R"cpp(
   template 
   struct unique_ptr {
 T *operator->();
   };
-  struct $explicit^Foo { int a; };)cpp",
+  struct $implicit^Foo { int a; };)cpp",
"void test(unique_ptr &V) { V->^a; }");
   testWalk(R"cpp(
   template 
-  struct $explicit^unique_ptr {
+  struct $implicit^unique_ptr {
 void release();
   };
   struct Foo {};)cpp",
@@ -213,21 +213,21 @@
   // Respect the sugar type (typedef, using-type).
   testWalk(R"cpp(
   namespace ns { struct Foo { int a; }; }
-  using $explicit^Bar = ns::Foo;)cpp",
+  using $implicit^Bar = ns::Foo;)cpp",
"void test(Bar b) { b.^a; }");
   testWalk(R"cpp(
   namespace ns { struct Foo { int a; }; }
-  using ns::$explicit^Foo;)cpp",
+  using ns::$implicit^Foo;)cpp",
"void test(Foo b) { b.^a; }");
   testWalk(R"cpp(
   namespace ns { struct Foo { int a; }; }
   namespace ns2 { using Bar = ns::Foo; }
-  using ns2::$explicit^Bar;
+  using ns2::$implicit^Bar;
   )cpp",
"void test(Bar b) { b.^a; }");
   testWalk(R"cpp(
   namespace ns { template struct Foo { int a; }; }
-  using ns::$explicit^Foo;)cpp",
+  using ns::$implicit^Foo;)cpp",
"void k(Foo b) { b.^a; }");
 }
 
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -80,8 +80,7 @@
 //
 // FIXME: support dependent types, e.g., "std::vector().size()".
 QualType Type = E->getBase()->IgnoreImpCasts()->getType();
-// FIXME: this should report as implicit reference.
-report(E->getMemberLoc(), getMemberProvider(Type));
+report(E->getMemberLoc(), getMemberProvider(Type), RefType::Implicit);
 return true;
   }
 


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -180,32 +180,32 @@
 }
 
 TEST(WalkAST, MemberExprs) {
-  testWalk("struct $explicit^S { void foo(); };", "void foo() { S{}.^foo(); }");
+  testWalk("struct $implicit^S { void foo(); };", "void foo() { S{}.^foo(); }");
   testWalk(
-  "struct S { void foo(); }; struct $explicit^X : S { using S::foo; };",
+  "struct S { void foo(); }; struct $implicit^X : S { using S::foo; };",
   "void foo() { X{}.^foo()

[PATCH] D139409: [include-cleaner] Handle dependent type members in AST

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:48
 
   NamedDecl *resolveType(QualType Type) {
 if (Type->isPointerType())

please rebase the patch, this API is changed in 
https://github.com/llvm/llvm-project/commit/0ab57bdc9745bfc8147831c09ed05073f87e7040,
 and the `TemplateSpecializationType` is handled already (to fix the sugar type 
issue).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139409

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

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)`

[Nathan Ridge via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138300

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

[clang] 8e029d9 - [CLANG] XFAIL c-strings.c & volatile-1.c AArch64/Windows

[Muhammad Omair Javaid via cfe-commits]

Author: Muhammad Omair Javaid
Date: 2022-12-19T14:03:30+05:00
New Revision: 8e029d9e35092d1440dafc8991e73fb8c3b323d7

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

LOG: [CLANG] XFAIL c-strings.c & volatile-1.c AArch64/Windows

c-strings.c was marked XFAIL for msvc ABI on AArch64/Windows platform.
But it is failing due to alignment issue on WoA for both msvc and gnu
gnu ABIs. I am going to mark them as XFAIL for both.

Added: 


Modified: 
clang/test/CodeGen/c-strings.c

Removed: 




diff  --git a/clang/test/CodeGen/c-strings.c b/clang/test/CodeGen/c-strings.c
index 9e1ac0f02b5e..abb4a836477f 100644
--- a/clang/test/CodeGen/c-strings.c
+++ b/clang/test/CodeGen/c-strings.c
@@ -1,4 +1,4 @@
-// XFAIL: taraget=aarch64-pc-windows-msvc
+// XFAIL: taraget=aarch64-pc-windows-{{.*}}
 // RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck 
%s --check-prefix=CHECK --check-prefix=ITANIUM
 // RUN: %clang_cc1 -triple %ms_abi_triple -emit-llvm -o - %s | FileCheck %s 
--check-prefix=CHECK --check-prefix=MSABI
 



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

[PATCH] D140275: [clangd] Tweak to add doxygen comment to the function declaration

[Oleg Skoromnik via Phabricator via cfe-commits]

tupos added a comment.

Also I tried to use it now, and I'm not sure where it is a good idea to allow 
this tweak from within a function body. Because it is quite annoying that in 
every function you get this code action.

On the other hand maybe it is ok to allow it from within the body, but the 
tweak then should be smarter and should check whether on the function 
declaration the doxygen comment is already present. Also if the tweak is 
smarter it is fine to get this code action, as it will motivate me to get rid 
of it and provide a proper documentation ;-).

What do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140275

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

[PATCH] D140261: [C++20][Modules] Do not allow non-inline external definitions in header units.

[Iain Sandoe via Phabricator via cfe-commits]

iains added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:12957-12958
+  // units.
+  if (getLangOpts().CPlusPlus20 && getLangOpts().CPlusPlusModules &&
+  !ModuleScopes.empty() && ModuleScopes.back().Module->isHeaderUnit()) {
+if (VDecl->getFormalLinkage() == Linkage::ExternalLinkage &&

ChuanqiXu wrote:
> `getLangOpts().CPlusPlus20` is redundant. It is also good to define a helper 
> interface `withinHeaderUnit` in Sema (not required).
I do not mind making this change - but note that the constraint is specific to 
C++20 and there are some people who want to remove it (the constraint).  I 
guess  you meant `getCurrentModule()` ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140261

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

[PATCH] D140217: [lit] Script to automate use of %(line-n). Use in CodeComplete tests.

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks! The change looks good to me (I assume our internal lit support part is 
completed). (I think we can do a cleanup in `clang/test/Index` as well).




Comment at: llvm/utils/relative_lines.py:17
+
+USAGE = """Example usage:
+find clang/test/CodeCompletion | grep -v /Inputs/ | \

This variable is not used in anywhere, and not print out in `--help` mode. I 
think it is more useful to move it to the above file comment, at least it is 
shown in `--help` mode.



Comment at: llvm/utils/relative_lines.py:18
+USAGE = """Example usage:
+find clang/test/CodeCompletion | grep -v /Inputs/ | \
+xargs relative_lines.py --dry-run --verbose --near=100 \

nit: find command is missing `-type f`.



Comment at: llvm/utils/relative_lines.py:20
+xargs relative_lines.py --dry-run --verbose --near=100 \
+--pattern='-code-completion-at[ =]%s:(\\d+)' \
+--pattern='requires fix-it: {(\d+):\d+-(\d+):\d+}'

`\\d` should be `\d`?



Comment at: llvm/utils/relative_lines.py:31
+ formatter_class=argparse.RawTextHelpFormatter)
+parser.add_argument('--near', type=int, default=0,
+help = "maximum line distance to make relative")

nit: the default value 0 basically does nothing, set a reasonable number, 20?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140217

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

[PATCH] D140261: [C++20][Modules] Do not allow non-inline external definitions in header units.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:12957-12958
+  // units.
+  if (getLangOpts().CPlusPlus20 && getLangOpts().CPlusPlusModules &&
+  !ModuleScopes.empty() && ModuleScopes.back().Module->isHeaderUnit()) {
+if (VDecl->getFormalLinkage() == Linkage::ExternalLinkage &&

iains wrote:
> ChuanqiXu wrote:
> > `getLangOpts().CPlusPlus20` is redundant. It is also good to define a 
> > helper interface `withinHeaderUnit` in Sema (not required).
> I do not mind making this change - but note that the constraint is specific 
> to C++20 and there are some people who want to remove it (the constraint).  I 
> guess  you meant `getCurrentModule()` ?
> I do not mind making this change - but note that the constraint is specific 
> to C++20 and there are some people who want to remove it (the constraint).

Got it. Let's make it when it comes true. It won't be a big deal.

> I guess you meant getCurrentModule() ?

It looks **a little bit** better to me to not access `ModuleScopes` directly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140261

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

[PATCH] D139409: [include-cleaner] Handle dependent type members in AST

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 483881.
VitaNuo marked 6 inline comments as done.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139409

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -229,6 +229,13 @@
   namespace ns { template struct Foo { int a; }; }
   using ns::$explicit^Foo;)cpp",
"void k(Foo b) { b.^a; }");
+  // Test the dependent-type case (CXXDependentScopeMemberExpr)
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base& t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base* t) { t->^method(); }");
 }
 
 TEST(WalkAST, ConstructExprs) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -84,6 +85,10 @@
 report(E->getMemberLoc(), getMemberProvider(Type));
 return true;
   }
+  bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+report(E->getMemberLoc(), getMemberProvider(E->getBaseType()));
+return true;
+  }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
 report(E->getLocation(), E->getConstructor(),


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -229,6 +229,13 @@
   namespace ns { template struct Foo { int a; }; }
   using ns::$explicit^Foo;)cpp",
"void k(Foo b) { b.^a; }");
+  // Test the dependent-type case (CXXDependentScopeMemberExpr)
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base& t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base* t) { t->^method(); }");
 }
 
 TEST(WalkAST, ConstructExprs) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -11,6 +11,7 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/NestedNameSpecifier.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
@@ -84,6 +85,10 @@
 report(E->getMemberLoc(), getMemberProvider(Type));
 return true;
   }
+  bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+report(E->getMemberLoc(), getMemberProvider(E->getBaseType()));
+return true;
+  }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
 report(E->getLocation(), E->getConstructor(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139409: [include-cleaner] Handle dependent type members in AST

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Thanks for the comments!




Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:51
   Type = Type->getPointeeType();
+if (const TemplateSpecializationType *TST =
+Type->getAs()) {

hokein wrote:
> nit: we can use `const auto*` here as the type is explicitly spelled in the 
> `getAs`
Seems to be obsolete with the above patch.



Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:52
+if (const TemplateSpecializationType *TST =
+Type->getAs()) {
+  return TST->getTemplateName().getAsTemplateDecl();

hokein wrote:
> nit: remove the {} around the if statement
Seems to be obsolete with the above patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139409

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

[PATCH] D139409: [include-cleaner] Handle dependent type members in AST

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 483882.
VitaNuo added a comment.

Remove unusued include.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139409

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -229,6 +229,13 @@
   namespace ns { template struct Foo { int a; }; }
   using ns::$explicit^Foo;)cpp",
"void k(Foo b) { b.^a; }");
+  // Test the dependent-type case (CXXDependentScopeMemberExpr)
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base& t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base* t) { t->^method(); }");
 }
 
 TEST(WalkAST, ConstructExprs) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -84,6 +84,10 @@
 report(E->getMemberLoc(), getMemberProvider(Type));
 return true;
   }
+  bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+report(E->getMemberLoc(), getMemberProvider(E->getBaseType()));
+return true;
+  }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
 report(E->getLocation(), E->getConstructor(),


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -229,6 +229,13 @@
   namespace ns { template struct Foo { int a; }; }
   using ns::$explicit^Foo;)cpp",
"void k(Foo b) { b.^a; }");
+  // Test the dependent-type case (CXXDependentScopeMemberExpr)
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base& t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base* t) { t->^method(); }");
 }
 
 TEST(WalkAST, ConstructExprs) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -84,6 +84,10 @@
 report(E->getMemberLoc(), getMemberProvider(Type));
 return true;
   }
+  bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+report(E->getMemberLoc(), getMemberProvider(E->getBaseType()));
+return true;
+  }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
 report(E->getLocation(), E->getConstructor(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

[gehry via Phabricator via cfe-commits]

Sockke added a comment.

Friendly ping.


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

https://reviews.llvm.org/D138655

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

[PATCH] D140217: [lit] Script to automate use of %(line-n). Use in CodeComplete tests.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: llvm/utils/relative_lines.py:45
+for file in args.files:
+contents = open(file).read()
+failures = 0

it would be nice to make the script more robust (capture and bailout the 
`UnicodeDecodeError` exception)-- there are some test files that contain 
invalid utf-8 characters



Comment at: llvm/utils/relative_lines.py:94
+for pattern in args.pattern:
+contents = re.sub(pattern, replace_match, contents)
+if failures > 0 and not args.partial:

looks like we modify the file more eagerly -- if there are no matching pattern, 
we will replace the content as well (most cases are ok, I saw the CRLF => LF 
change, I think we'd better only overwrite the content if there are matching 
patterns). 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140217

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

[PATCH] D139409: [include-cleaner] Handle dependent type members in AST

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139409

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

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Jannik Silvanus via Phabricator via cfe-commits]

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

LGTM, but let's wait for the other reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[PATCH] D138807: [RISCV] Support vector crypto extension ISA string and assembly

[Brandon Wu via Phabricator via cfe-commits]

4vtomat updated this revision to Diff 483884.
4vtomat added a comment.

Address most of Eric's comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138807

Files:
  clang/test/Preprocessor/riscv-target-features.c
  llvm/docs/RISCVUsage.rst
  llvm/lib/Support/RISCVISAInfo.cpp
  llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
  llvm/lib/Target/RISCV/MCTargetDesc/RISCVBaseInfo.h
  llvm/lib/Target/RISCV/RISCV.td
  llvm/lib/Target/RISCV/RISCVInstrFormats.td
  llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
  llvm/lib/Target/RISCV/RISCVInstrInfo.td
  llvm/lib/Target/RISCV/RISCVInstrInfoV.td
  llvm/lib/Target/RISCV/RISCVInstrInfoZvk.td
  llvm/lib/Target/RISCV/RISCVSubtarget.h
  llvm/test/CodeGen/RISCV/attributes.ll
  llvm/test/MC/RISCV/attribute-arch.s
  llvm/test/MC/RISCV/rvv/rv64zvkb.s
  llvm/test/MC/RISCV/rvv/rv64zvkg.s
  llvm/test/MC/RISCV/rvv/rv64zvknha.s
  llvm/test/MC/RISCV/rvv/rv64zvknhb.s
  llvm/test/MC/RISCV/rvv/rv64zvkns.s
  llvm/test/MC/RISCV/rvv/rv64zvkns_invalid.s
  llvm/test/MC/RISCV/rvv/rv64zvksed.s
  llvm/test/MC/RISCV/rvv/rv64zvksed_invalid.s
  llvm/test/MC/RISCV/rvv/rv64zvksh.s

Index: llvm/test/MC/RISCV/rvv/rv64zvksh.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvv/rv64zvksh.s
@@ -0,0 +1,21 @@
+# RUN: llvm-mc -triple=riscv64 -show-encoding --mattr=+zve32x --mattr=+experimental-zvksh %s \
+# RUN:| FileCheck %s --check-prefixes=CHECK-ENCODING,CHECK-INST
+# RUN: not llvm-mc -triple=riscv64 -show-encoding %s 2>&1 \
+# RUN:| FileCheck %s --check-prefix=CHECK-ERROR
+# RUN: llvm-mc -triple=riscv64 -filetype=obj --mattr=+zve32x --mattr=+experimental-zvksh %s \
+# RUN:| llvm-objdump -d --mattr=+zve32x --mattr=+experimental-zvksh  - \
+# RUN:| FileCheck %s --check-prefix=CHECK-INST
+# RUN: llvm-mc -triple=riscv64 -filetype=obj --mattr=+zve32x --mattr=+experimental-zvksh %s \
+# RUN:| llvm-objdump -d - | FileCheck %s --check-prefix=CHECK-UNKNOWN
+
+vsm3c.vi v10, v9, 7
+# CHECK-INST: vsm3c.vi v10, v9, 7
+# CHECK-ENCODING: [0x77,0xa5,0x93,0xae]
+# CHECK-ERROR: instruction requires the following: 'Zvksh'
+# CHECK-UNKNOWN: 77 a5 93 ae   
+
+vsm3me.vv v10, v9, v8
+# CHECK-INST: vsm3me.vv v10, v9, v8
+# CHECK-ENCODING: [0x77,0x25,0x94,0x82]
+# CHECK-ERROR: instruction requires the following: 'Zvksh'
+# CHECK-UNKNOWN: 77 25 94 82   
Index: llvm/test/MC/RISCV/rvv/rv64zvksed_invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvv/rv64zvksed_invalid.s
@@ -0,0 +1,5 @@
+# RUN: not llvm-mc -triple=riscv64 --mattr=+zve32x --mattr=+experimental-zvksed -show-encoding %s 2>&1 \
+# RUN:| FileCheck %s --check-prefix=CHECK-ERROR
+
+vsm4k.vi v10, v9, 8
+# CHECK-ERROR: immediate must be an integer in the range [0, 7]
Index: llvm/test/MC/RISCV/rvv/rv64zvksed.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvv/rv64zvksed.s
@@ -0,0 +1,21 @@
+# RUN: llvm-mc -triple=riscv64 -show-encoding --mattr=+zve32x --mattr=+experimental-zvksed %s \
+# RUN:| FileCheck %s --check-prefixes=CHECK-ENCODING,CHECK-INST
+# RUN: not llvm-mc -triple=riscv64 -show-encoding %s 2>&1 \
+# RUN:| FileCheck %s --check-prefix=CHECK-ERROR
+# RUN: llvm-mc -triple=riscv64 -filetype=obj --mattr=+zve32x --mattr=+experimental-zvksed %s \
+# RUN:| llvm-objdump -d --mattr=+zve32x --mattr=+experimental-zvksed  - \
+# RUN:| FileCheck %s --check-prefix=CHECK-INST
+# RUN: llvm-mc -triple=riscv64 -filetype=obj --mattr=+zve32x --mattr=+experimental-zvksed %s \
+# RUN:| llvm-objdump -d - | FileCheck %s --check-prefix=CHECK-UNKNOWN
+
+vsm4k.vi v10, v9, 7
+# CHECK-INST: vsm4k.vi v10, v9, 7
+# CHECK-ENCODING: [0x77,0xa5,0x93,0x86]
+# CHECK-ERROR: instruction requires the following: 'Zvksed'
+# CHECK-UNKNOWN: 77 a5 93 86   
+
+vsm4r.vv v10, v9
+# CHECK-INST: vsm4r.vv v10, v9
+# CHECK-ENCODING: [0x77,0x25,0x98,0xa2]
+# CHECK-ERROR: instruction requires the following: 'Zvksed'
+# CHECK-UNKNOWN: 77 25 98 a2   
Index: llvm/test/MC/RISCV/rvv/rv64zvkns_invalid.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvv/rv64zvkns_invalid.s
@@ -0,0 +1,8 @@
+# RUN: not llvm-mc -triple=riscv64 -show-encoding --mattr=+zve32x --mattr=+experimental-zvkns %s 2>&1 \
+# RUN:| FileCheck %s --check-prefix=CHECK-ERROR
+
+vaeskf1.vi v10, v9, 0
+# CHECK-ERROR: immediate must be an integer in the range [1, 10]
+
+vaeskf2.vi v10, v9, 0
+# CHECK-ERROR: immediate must be an integer in the range [2, 14]
Index: llvm/test/MC/RISCV/rvv/rv64zvkns.s
===
--- /dev/null
+++ llvm/test/MC/RISCV/rvv/rv64zvkns.s
@@ -0,0 +1,75 @@
+# RUN: llvm-mc -triple=riscv64 -show-encoding --mattr=+zve32x --mattr=+experimental-zvkns %s \
+# RUN:| Fi

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

[Pavel Iliin via Phabricator via cfe-commits]

ilinpv added a comment.

Does anyone have any more objections? I'm going to merge it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127812

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

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

Just thoughts.

llvm::any_isa is usually paired with llvm::any_cast; replacing them with 
llvm::any_cast and nullptr check seems fine.
However,

- std::any uses RTTI, so it is unlikely to ever replace llvm::Any.
- llvm::any_isa<> is kind of convenient and is aligned with llvm::isa<>. Same 
for llvm::any_cast.
- With that in mind, introducing new llvm::any_cast_or_null to follow 
llvm::cast_or_null instead of changing the semantics of llvm::any_cast might be 
a better idea.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[PATCH] D139114: [Clang][Sema] Enabled Wshorten-64-to-32 for CompoundAssignment operator.

[Fahad Nayyar via Phabricator via cfe-commits]

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



Comment at: clang/lib/Sema/SemaChecking.cpp:14115
 
+/// Analyze the given compound assignment for the possible losing of
+/// floating-point precision.

aaron.ballman wrote:
> Moving this function to another spot in the file makes comparing the new and 
> old code pretty hard -- would it make more sense to forward declare the 
> needed static functions so this can stay where it was and it's easier to see 
> what's changed?
@aaron.ballman I've made the required changes. Please have a look, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139114

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

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added inline comments.



Comment at: lldb/include/lldb/Core/RichManglingContext.h:90-91
 assert(parser.has_value());
-assert(llvm::any_isa(parser));
+assert(llvm::any_cast(&parser));
 return llvm::any_cast(parser);
   }

This is not intuitively clear. In assert you pass an address, but in 'return' 
below the object is passed by reference.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

[Serge Pavlov via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D134859

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Hi,

The following starts crashing with this patch:

  clang -cc1 -analyze -analyzer-checker=core bbi-77010.c

It crashes with

  bbi-77010.c:6:1: warning: non-void function does not return a value 
[-Wreturn-type]
  }
  ^
  clang: ../../clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1622: 
clang::ento::RangeSet (anonymous 
namespace)::SymbolicRangeInferrer::VisitBinaryOperator(clang::ento::RangeSet, 
clang::ento::RangeSet, clang::QualType): Assertion `!LHS.isEmpty() && 
!RHS.isEmpty() && "Both ranges should be non-empty"' failed.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: ../../main-github/llvm/build-all/bin/clang -cc1 
-analyze -analyzer-checker=core bbi-77010.c
  1. parser at end of file
  2.While analyzing stack: 
#0 Calling g
  3.bbi-77010.c:13:12: Error evaluating statement
  4.bbi-77010.c:13:12: Error evaluating statement
   #0 0x02fddbb3 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(../../main-github/llvm/build-all/bin/clang+0x2fddbb3)
   #1 0x02fdb8de llvm::sys::RunSignalHandlers() 
(../../main-github/llvm/build-all/bin/clang+0x2fdb8de)
   #2 0x02fddf36 SignalHandler(int) Signals.cpp:0:0
   #3 0x7fe701eb8630 __restore_rt sigaction.c:0:0
   #4 0x7fe6ff5ff387 raise (/lib64/libc.so.6+0x36387)
   #5 0x7fe6ff600a78 abort (/lib64/libc.so.6+0x37a78)
   #6 0x7fe6ff5f81a6 __assert_fail_base (/lib64/libc.so.6+0x2f1a6)
   #7 0x7fe6ff5f8252 (/lib64/libc.so.6+0x2f252)
   #8 0x049caed2 (anonymous 
namespace)::SymbolicRangeInferrer::VisitBinaryOperator(clang::ento::RangeSet, 
clang::BinaryOperatorKind, clang::ento::RangeSet, clang::QualType) 
RangeConstraintManager.cpp:0:0
   #9 0x049c9867 (anonymous 
namespace)::SymbolicRangeInferrer::infer(clang::ento::SymExpr const*) 
RangeConstraintManager.cpp:0:0
  #10 0x049bebf5 (anonymous 
namespace)::RangeConstraintManager::assumeSymNE(llvm::IntrusiveRefCntPtr, clang::ento::SymExpr const*, llvm::APSInt const&, llvm::APSInt const&) 
RangeConstraintManager.cpp:0:0
  #11 0x049d368c 
clang::ento::RangedConstraintManager::assumeSymUnsupported(llvm::IntrusiveRefCntPtr, clang::ento::SymExpr const*, bool) 
(../../main-github/llvm/build-all/bin/clang+0x49d368c)
  #12 0x049f0b09 
clang::ento::SimpleConstraintManager::assumeAux(llvm::IntrusiveRefCntPtr, clang::ento::NonLoc, bool) 
(../../main-github/llvm/build-all/bin/clang+0x49f0b09)
  #13 0x049f096a 
clang::ento::SimpleConstraintManager::assume(llvm::IntrusiveRefCntPtr, clang::ento::NonLoc, bool) 
(../../main-github/llvm/build-all/bin/clang+0x49f096a)
  #14 0x049f086d 
clang::ento::SimpleConstraintManager::assumeInternal(llvm::IntrusiveRefCntPtr, clang::ento::DefinedSVal, bool) 
(../../main-github/llvm/build-all/bin/clang+0x49f086d)
  #15 0x0492d3e3 
clang::ento::ConstraintManager::assumeDual(llvm::IntrusiveRefCntPtr, clang::ento::DefinedSVal) 
(../../main-github/llvm/build-all/bin/clang+0x492d3e3)
  #16 0x04955b6d 
clang::ento::ExprEngine::evalEagerlyAssumeBinOpBifurcation(clang::ento::ExplodedNodeSet&,
 clang::ento::ExplodedNodeSet&, clang::Expr const*) 
(../../main-github/llvm/build-all/bin/clang+0x4955b6d)
  #17 0x049514b6 clang::ento::ExprEngine::Visit(clang::Stmt const*, 
clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&) 
(../../main-github/llvm/build-all/bin/clang+0x49514b6)
  #18 0x0494c73e clang::ento::ExprEngine::ProcessStmt(clang::Stmt 
const*, clang::ento::ExplodedNode*) 
(../../main-github/llvm/build-all/bin/clang+0x494c73e)
  #19 0x0494c459 
clang::ento::ExprEngine::processCFGElement(clang::CFGElement, 
clang::ento::ExplodedNode*, unsigned int, clang::ento::NodeBuilderContext*) 
(../../main-github/llvm/build-all/bin/clang+0x494c459)
  #20 0x0492f3d0 
clang::ento::CoreEngine::HandlePostStmt(clang::CFGBlock const*, unsigned int, 
clang::ento::ExplodedNode*) 
(../../main-github/llvm/build-all/bin/clang+0x492f3d0)
  #21 0x0492e1f6 
clang::ento::CoreEngine::ExecuteWorkList(clang::LocationContext const*, 
unsigned int, llvm::IntrusiveRefCntPtr) 
(../../main-github/llvm/build-all/bin/clang+0x492e1f6)
  #22 0x0454d931 (anonymous 
namespace)::AnalysisConsumer::HandleCode(clang::Decl*, unsigned int, 
clang::ento::ExprEngine::InliningModes, llvm::DenseSet>*) AnalysisConsumer.cpp:0:0
  #23 0x0453034e (anonymous 
namespace)::AnalysisConsumer::HandleTranslationUnit(clang::ASTContext&) 
AnalysisConsumer.cpp:0:0
  #24 0x04a441a5 clang::ParseAST(clang::Sema&, bool, bool) 
(../../main-github/llvm/build-all/bin/clang+0x4a441a5)
  #25 0x03ac97f6 clang::FrontendAction::Execute() 
(../../main-github/llvm/build-all/bin/clang+0x3ac97f6)
  #26 0x03a3b8a4 
clang::CompilerInstance::ExecuteAction(clang::FrontendAction&) 

[PATCH] D138655: [clang-tidy] Fix `cppcoreguidelines-init-variables` for invalid vardecl

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Looks good in general, IMHO it would be good with a couple more comments to 
make the code and test easier to understand.




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp:63
 
+  if (MatchedDecl->isInvalidDecl())
+return;

Add a comment as to why this is needed? I haven't seen this pattern often in 
checks.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp:4
 
+#include "unknown.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:10: error: 'unknown.h' file not found 
[clang-diagnostic-error]

Add a comment explaining the purpose of introducing this error? At first sight, 
it's not obvious to me why this is needed.




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/init-variables.cpp:141
+  // CHECK-FIXES: {{^}}  int a = 0;{{$}}
+  std::vector arr;
+}

Perhaps add a comment explaining that clang-tidy should not warn here, and why.


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

https://reviews.llvm.org/D138655

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

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Jannik Silvanus via Phabricator via cfe-commits]

jsilvanus added a comment.

In D139973#4004344 , @barannikov88 
wrote:

> - std::any uses RTTI, so it is unlikely to ever replace llvm::Any.

I just checked on Godbolt that gcc 12.2 with libstdc++, clang 15.0 with both 
libstd++ and libc++ and MSVC 19.33 support `std::any` without RTTI. 
But gcc and clang do not provide `std::any::type` without RTTI though.

Are you aware of any documentation on what can be relied on without RTTI?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

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

steakhal added a comment.

In D112621#4004372 , @uabelho wrote:

> Hi,
>
> The following starts crashing with this patch:
>
>   clang -cc1 -analyze -analyzer-checker=core bbi-77010.c
>
> It crashes with
>
>   bbi-77010.c:6:1: warning: non-void function does not return a value 
> [-Wreturn-type]
>   }
>   ^
>   clang: ../../clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1622: 
> clang::ento::RangeSet (anonymous 
> namespace)::SymbolicRangeInferrer::VisitBinaryOperator(clang::ento::RangeSet, 
> clang::ento::RangeSet, clang::QualType): Assertion `!LHS.isEmpty() && 
> !RHS.isEmpty() && "Both ranges should be non-empty"' failed.
>   PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
> and include the crash backtrace, preprocessed source, and associated run 
> script.
>   Abort (core dumped)
>
> F25730184: bbi-77010.c 

Thanks for the report. I'll fix it ASAP.
I think I'll replace the assertion with an early return.

BTW, was this uncovered by fuzzing? @uabelho


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D137790#3992216 , @balazske wrote:

> Some reports can be found here 
>  (if the 
> link works and the data does not expire), the runs stored on 2022-12-09.
>
> Results appeared only projects "postgres" and "curl" (from 
> memcached,tmux,curl,twin,vim,openssl,sqlite,ffmpeg,postgres,libwebm) from 
> checkers **StdCLibraryFunctionArgs** and **Errno**.
>
> On the postgres results, the second is one that can be fixed in the checker 
> (add special cases to `StdLibraryFunctionsChecker` for zero `len` or `size` 
> `fread` and `fwrite` arguments). The others are false positives because the 
> error path is impossible because implicit constraints (what is not known to 
> the analyzer) on variables.
>
> These curl 
> 
>  results look faulty, the last `fclose` call looks not recognized.

Please make the link a little more reviewer friendly. You can make a 
CodeChecker URL where the appropriate runs are already selected, with the 
appropriate checkers being filtered, and uniquing turned on, like this:
https://codechecker-demo.eastus.cloudapp.azure.com/Default/reports?run=libwebm_libwebm-1.0.0.27_baseline&run=postgres_REL_13_0_baseline&run=ffmpeg_n4.3.1_baseline&run=sqlite_version-3.33.0_baseline&run=openssl_openssl-3.0.0-alpha7_baseline&run=vim_v8.2.1920_baseline&run=twin_v0.8.1_baseline&run=curl_curl-7_66_0_baseline&run=tmux_2.6_baseline&run=memcached_1.6.8_baseline&is-unique=on&diff-type=New&checker-name=alpha.unix.Errno&checker-name=alpha.unix.StdCLibraryFunctionArgs

Also, I'd appreciate links to the specific reports you are describing. I'm not 
sure what postgres results you are talking about, because I suspect I listed 
them differently. You made a link to the curl result, so please do it for the 
others as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137790

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

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

In D112621#4004409 , @steakhal wrote:

> In D112621#4004372 , @uabelho wrote:
>
>> Hi,
>>
>> The following starts crashing with this patch:
>>
>>   clang -cc1 -analyze -analyzer-checker=core bbi-77010.c
>>
>> It crashes with
>>
>>   bbi-77010.c:6:1: warning: non-void function does not return a value 
>> [-Wreturn-type]
>>   }
>>   ^
>>   clang: 
>> ../../clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1622: 
>> clang::ento::RangeSet (anonymous 
>> namespace)::SymbolicRangeInferrer::VisitBinaryOperator(clang::ento::RangeSet,
>>  clang::ento::RangeSet, clang::QualType): Assertion `!LHS.isEmpty() && 
>> !RHS.isEmpty() && "Both ranges should be non-empty"' failed.
>>   PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
>> and include the crash backtrace, preprocessed source, and associated run 
>> script.
>>   Abort (core dumped)
>>
>> F25730184: bbi-77010.c 
>
> Thanks for the report. I'll fix it ASAP.
> I think I'll replace the assertion with an early return.
>
> BTW, was this uncovered by fuzzing? @uabelho

We found it compiling some random programs generated by Csmith.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[clang] f61a08b - [analyzer] Fix crash inside RangeConstraintManager.cpp introduced by D112621

[Balazs Benics via cfe-commits]

Author: Balazs Benics
Date: 2022-12-19T12:49:43+01:00
New Revision: f61a08b67f5c8b0202dd30821766ee029e880b7c

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

LOG: [analyzer] Fix crash inside RangeConstraintManager.cpp introduced by 
D112621

It seems like `LHS` and `RHS` could be empty range sets.
This caused an assertion failure inside RangeConstraintManager.

I'm hoisting out the check from the function into the call-site.
This way we could assert that we only want to deal with non-empty range
sets.

The relevant part of the trace:
```
 #6 0x7fe6ff5f81a6 __assert_fail_base (/lib64/libc.so.6+0x2f1a6)
 #7 0x7fe6ff5f8252 (/lib64/libc.so.6+0x2f252)
 #8 0x049caed2 (anonymous 
namespace)::SymbolicRangeInferrer::VisitBinaryOperator(clang::ento::RangeSet, 
clang::BinaryOperatorKind, clang::ento::RangeSet, clang::QualType) 
RangeConstraintManager.cpp:0:0
 #9 0x049c9867 (anonymous 
namespace)::SymbolicRangeInferrer::infer(clang::ento::SymExpr const*) 
RangeConstraintManager.cpp:0:0
#10 0x049bebf5 (anonymous 
namespace)::RangeConstraintManager::assumeSymNE(llvm::IntrusiveRefCntPtr, clang::ento::SymExpr const*, llvm::APSInt const&, llvm::APSInt const&) 
RangeConstraintManager.cpp:0:0
#11 0x049d368c 
clang::ento::RangedConstraintManager::assumeSymUnsupported(llvm::IntrusiveRefCntPtr, clang::ento::SymExpr const*, bool) 
(../../main-github/llvm/build-all/bin/clang+0x49d368c)
#12 0x049f0b09 
clang::ento::SimpleConstraintManager::assumeAux(llvm::IntrusiveRefCntPtr, clang::ento::NonLoc, bool) 
(../../main-github/llvm/build-all/bin/clang+0x49f0b09)
#13 0x049f096a 
clang::ento::SimpleConstraintManager::assume(llvm::IntrusiveRefCntPtr, clang::ento::NonLoc, bool) 
(../../main-github/llvm/build-all/bin/clang+0x49f096a)
#14 0x049f086d 
clang::ento::SimpleConstraintManager::assumeInternal(llvm::IntrusiveRefCntPtr, clang::ento::DefinedSVal, bool) 
(../../main-github/llvm/build-all/bin/clang+0x49f086d)
#15 0x0492d3e3 
clang::ento::ConstraintManager::assumeDual(llvm::IntrusiveRefCntPtr, clang::ento::DefinedSVal) 
(../../main-github/llvm/build-all/bin/clang+0x492d3e3)
#16 0x04955b6d 
clang::ento::ExprEngine::evalEagerlyAssumeBinOpBifurcation(clang::ento::ExplodedNodeSet&,
 clang::ento::ExplodedNodeSet&, clang::Expr const*) 
(../../main-github/llvm/build-all/bin/clang+0x4955b6d)
#17 0x049514b6 clang::ento::ExprEngine::Visit(clang::Stmt const*, 
clang::ento::ExplodedNode*, clang::ento::ExplodedNodeSet&) 
(../../main-github/llvm/build-all/bin/clang+0x49514b6)
#18 0x0494c73e clang::ento::ExprEngine::ProcessStmt(clang::Stmt const*, 
clang::ento::ExplodedNode*) 
(../../main-github/llvm/build-all/bin/clang+0x494c73e)
#19 0x0494c459 
clang::ento::ExprEngine::processCFGElement(clang::CFGElement, 
clang::ento::ExplodedNode*, unsigned int, clang::ento::NodeBuilderContext*) 
(../../main-github/llvm/build-all/bin/clang+0x494c459)
#20 0x0492f3d0 clang::ento::CoreEngine::HandlePostStmt(clang::CFGBlock 
const*, unsigned int, clang::ento::ExplodedNode*) 
(../../main-github/llvm/build-all/bin/clang+0x492f3d0)
#21 0x0492e1f6 
clang::ento::CoreEngine::ExecuteWorkList(clang::LocationContext const*, 
unsigned int, llvm::IntrusiveRefCntPtr) 
(../../main-github/llvm/build-all/bin/clang+0x492e1f6)
```

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

Added: 
clang/test/Analysis/constant-folding-crash.cpp

Modified: 
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp 
b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index eda06c64a9f4b..91e71558314ad 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -1363,11 +1363,7 @@ class SymbolicRangeInferrer
 
   template 
   RangeSet VisitBinaryOperator(RangeSet LHS, RangeSet RHS, QualType T) {
-// We should propagate information about unfeasbility of one of the
-// operands to the resulting range.
-if (LHS.isEmpty() || RHS.isEmpty()) {
-  return RangeFactory.getEmptySet();
-}
+assert(!LHS.isEmpty() && !RHS.isEmpty());
 
 Range CoarseLHS = fillGaps(LHS);
 Range CoarseRHS = fillGaps(RHS);
@@ -1618,8 +1614,7 @@ template <>
 RangeSet SymbolicRangeInferrer::VisitBinaryOperator(RangeSet LHS,
RangeSet RHS,
QualType T) {
-
-  assert(!LHS.isEmpty() && !RHS.isEmpty() && "Both ranges should be 
non-empty");
+  assert(!LHS.isEmpty() && !RHS.isEmpty());
 
   if (LHS.getAPSIntType() == RHS.getAPSIntType()) {
 if (intersect(RangeFactory, LH

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

[Carlos Galvez via Phabricator via cfe-commits]

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

Static declarations in header files is considered bad practice,
since header files are meant for sharing code, and static
leads to internal linkage. This can cause code bloat and lead
to subtle issues, for example ODR violations. Essentially, this
is as problematic as having anonymous namespaces in headers,
which we already have checks for.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140290

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp
@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy %s misc-static-declaration-in-header %t
+
+static int v1 = 123;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: static declaration in header; remove 'static' [misc-static-declaration-in-header]
+// CHECK-FIXES: {{^}}int v1 = 123;
+
+static const int c1 = 123;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: static declaration in header; remove 'static'
+// CHECK-FIXES: {{^}}const int c1 = 123;
+
+static constexpr int c2 = 123;
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: static declaration in header; remove 'static'
+// CHECK-FIXES: {{^}}constexpr int c2 = 123;
+
+static void f1(){}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static declaration in header; remove 'static'
+// CHECK-FIXES: {{^}}void f1(){}
+
+namespace foo {
+  static int v2 = 123;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static'
+  // CHECK-FIXES: {{^}}  int v2 = 123;
+
+  static int f2(){}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static'
+  // CHECK-FIXES: {{^}}  int f2(){}
+}
+
+namespace {
+  static int v3 = 123;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static'
+  // CHECK-FIXES: {{^}}  int v3 = 123;
+
+  static int f3(){}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static'
+  // CHECK-FIXES: {{^}}  int f3(){}
+}
+
+// OK
+struct Foo {
+  static const int v4 = 123;
+  static void f4(){}
+};
+
+// OK
+void f5() {
+  static int v5;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst
@@ -0,0 +1,41 @@
+.. title:: clang-tidy - misc-static-declaration-in-header
+
+misc-static-declaration-in-header
+=
+
+Warns about ``static`` variable and function declarations at file or namespace
+scope in header files. Provides fix-it hint to remove ``static``.
+
+Header files are used to share code among different translation units. ``static``,
+on the other hand, can be used to express that a given declaration has internal
+linkage. Using ``static`` in header files leads to each translation unit creating
+their own internal copy of the function or variable at hand, which is problematic.
+This can cause unexpected results, bloat the resulting executable or even trigger
+one-definition-rule (ODR) violations.
+
+This problem is similar to having anonymous namespaces in header files, already
+covered by :doc:`google-build-namespaces`.
+
+Example of problematic code:
+
+.. code-block:: c++
+
+  // foo.h
+
+  // Bad
+  static int x = 123;
+  static void f() {}
+
+  // Good
+  int x = 123;
+  void f() {}
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+   A semicolon-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an
+   empty string between ";" if there are other filename extensions.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -254,6 +254,7 @@
`misc-non-private-member-variables-in-classes `_,
`misc-redundant-expression `

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

In D139973#4004408 , @jsilvanus wrote:

> I just checked on Godbolt that gcc 12.2 with libstdc++, clang 15.0 with both 
> libstd++ and libc++ and MSVC 19.33 support `std::any` without RTTI. 
> But gcc and clang do not provide `std::any::type` without RTTI though.
>
> Are you aware of any documentation on what can be relied on without RTTI?

It is surprising to me that std::any can work without RTTI. Never thought it 
could be implemented.
This "compare function pointer" trick is awesome, but it might not always work 
as explained in this 

 commit.
This question 

 however, suggest that any_cast doesn't always work with RTTI either, which is 
weird.
I don't know of any other potential issues. std::visitor can't be used with 
std::any in absense of std::any::type, but that's minor, I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[clang] 658ed95 - Fix host call to nohost function with host variant.

[Doru Bercea via cfe-commits]

Author: Doru Bercea
Date: 2022-12-19T06:13:26-06:00
New Revision: 658ed9547cdd6657895339a6c390c31aa77a5698

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

LOG: Fix host call to nohost function with host variant.

Added: 
clang/test/OpenMP/declare_target_nohost_variant_messages.cpp

Modified: 
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/declare_target_messages.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 49f401dfff690..8d0754c9d7c94 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -2702,6 +2702,24 @@ void Sema::finalizeOpenMPDelayedAnalysis(const 
FunctionDecl *Caller,
   }
   if (!LangOpts.OpenMPIsDevice && !LangOpts.OpenMPOffloadMandatory && DevTy &&
   *DevTy == OMPDeclareTargetDeclAttr::DT_NoHost) {
+// In OpenMP 5.2 or later, if the function has a host variant then allow
+// that to be called instead
+auto &&HasHostAttr = [](const FunctionDecl *Callee) {
+  for (OMPDeclareVariantAttr *A :
+   Callee->specific_attrs()) {
+auto *DeclRefVariant = cast(A->getVariantFuncRef());
+auto *VariantFD = cast(DeclRefVariant->getDecl());
+Optional DevTy =
+OMPDeclareTargetDeclAttr::getDeviceType(
+VariantFD->getMostRecentDecl());
+if (!DevTy || *DevTy == OMPDeclareTargetDeclAttr::DT_Host)
+  return true;
+  }
+  return false;
+};
+if (getLangOpts().OpenMP >= 52 &&
+Callee->hasAttr() && HasHostAttr(Callee))
+  return;
 // Diagnose nohost function called during host codegen.
 StringRef NoHostDevTy = getOpenMPSimpleClauseTypeName(
 OMPC_device_type, OMPC_DEVICE_TYPE_nohost);

diff  --git a/clang/test/OpenMP/declare_target_messages.cpp 
b/clang/test/OpenMP/declare_target_messages.cpp
index 7e7cc60e75e35..bf23813999119 100644
--- a/clang/test/OpenMP/declare_target_messages.cpp
+++ b/clang/test/OpenMP/declare_target_messages.cpp
@@ -11,10 +11,12 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp-version=51 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp-version=51 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 
+// RUN: %clang_cc1 -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa 
-fopenmp-version=52 -DVERBOSE_MODE=1 -verify=expected,omp52 -fnoopenmp-use-tls 
-ferror-limit 100 -o - %s
+
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5 
-fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 #pragma omp end declare target // expected-error {{unexpected OpenMP directive 
'#pragma omp end declare target'}}
 
-int a, b, z; // omp5-error {{variable captured in declare target region must 
appear in a to clause}} // omp51-error {{variable captured in declare target 
region must appear in a to clause}}
+int a, b, z; // omp5-error {{variable captured in declare target region must 
appear in a to clause}} // omp51-error {{variable captured in declare target 
region must appear in a to clause}} omp52-error {{variable captured in declare 
target region must appear in a to clause}}
 __thread int t; // expected-note {{defined as threadprivate or thread local}}
 
 #pragma omp declare target . // expected-error {{expected '(' after 'declare 
target'}}
@@ -23,16 +25,16 @@ __thread int t; // expected-note {{defined as threadprivate 
or thread local}}
 void f();
 #pragma omp end declare target shared(a) // expected-warning {{extra tokens at 
the end of '#pragma omp end declare target' are ignored}}
 
-#pragma omp declare target map(a) // omp45-error {{expected at least one 'to' 
or 'link' clause}} omp5-error {{expected at least one 'to' or 'link' clause}} 
omp51-error {{expected at least one 'to', 'link' or 'indirect' clause}} 
omp45-error {{unexpected 'map' clause, only 'to' or 'link' clauses expected}} 
omp5-error {{unexpected 'map' clause, only 'to', 'link' or 'device_type' 
clauses expected}} omp51-error {{unexpected 'map' clause, only 'to', 'link', 
'device_type' or 'indirect' clauses expected}}
+#pragma omp declare target map(a) // omp45-error {{expected at least one 'to' 
or 'link' clause}} omp5-error {{expected at least one 'to' or 'link' clause}} 
omp51-error {{expected at least one 'to', 'link' or 'indirect' clause}} 
omp45-error {{unexpected 'map' clause, only 'to' or 'link' clauses expected}} 
omp5-error {{unexpected 'map' clause, only 'to', 'link' or 'device_type' 
clauses expected}} omp51-error {{unexpected 'map' clause, only 'to', 'link', 
'device_type' or 'indirect' clauses expected}} omp52-error {{unexpected 'map' 
clause, only 'enter', 'link', 'device_type' or 'indirect' clauses expected}} 
omp52

[PATCH] D140155: [Clang][OpenMP] Allow host call to nohost function with host variant

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 closed this revision.
doru1004 added a comment.

Commit 658ed9547cdd6657895339a6c390c31aa77a5698 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140155

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

[PATCH] D140292: [OpenMP] Migrate OpenMPOffloadMappingFlags from Clang CodeGen to OMPConstants

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis created this revision.
TIFitis added reviewers: kiranchandramohan, clementval, jdoerfert, jsjodin.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
TIFitis requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, sstefan1.
Herald added projects: clang, LLVM.

This patch moves the OpenMPOffloadMappingFlags enum definiition from Clang 
codegen to OMPConstants.h


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140292

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  llvm/include/llvm/Frontend/OpenMP/OMPConstants.h

Index: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
===
--- llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
+++ llvm/include/llvm/Frontend/OpenMP/OMPConstants.h
@@ -189,8 +189,63 @@
   OMP_TGT_EXEC_MODE_GENERIC = 1 << 0,
   OMP_TGT_EXEC_MODE_SPMD = 1 << 1,
   OMP_TGT_EXEC_MODE_GENERIC_SPMD =
-  OMP_TGT_EXEC_MODE_GENERIC | OMP_TGT_EXEC_MODE_SPMD,
-  LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue */ OMP_TGT_EXEC_MODE_GENERIC_SPMD)
+  OMP_TGT_EXEC_MODE_GENERIC | OMP_TGT_EXEC_MODE_SPMD
+};
+
+/// Values for bit flags used to specify the mapping type for
+/// offloading.
+enum OpenMPOffloadMappingFlags : uint64_t {
+  /// No flags
+  OMP_MAP_NONE = 0x0,
+  /// Allocate memory on the device and move data from host to device.
+  OMP_MAP_TO = 0x01,
+  /// Allocate memory on the device and move data from device to host.
+  OMP_MAP_FROM = 0x02,
+  /// Always perform the requested mapping action on the element, even
+  /// if it was already mapped before.
+  OMP_MAP_ALWAYS = 0x04,
+  /// Delete the element from the device environment, ignoring the
+  /// current reference count associated with the element.
+  OMP_MAP_DELETE = 0x08,
+  /// The element being mapped is a pointer-pointee pair; both the
+  /// pointer and the pointee should be mapped.
+  OMP_MAP_PTR_AND_OBJ = 0x10,
+  /// This flags signals that the base address of an entry should be
+  /// passed to the target kernel as an argument.
+  OMP_MAP_TARGET_PARAM = 0x20,
+  /// Signal that the runtime library has to return the device pointer
+  /// in the current position for the data being mapped. Used when we have the
+  /// use_device_ptr or use_device_addr clause.
+  OMP_MAP_RETURN_PARAM = 0x40,
+  /// This flag signals that the reference being passed is a pointer to
+  /// private data.
+  OMP_MAP_PRIVATE = 0x80,
+  /// Pass the element to the device by value.
+  OMP_MAP_LITERAL = 0x100,
+  /// Implicit map
+  OMP_MAP_IMPLICIT = 0x200,
+  /// Close is a hint to the runtime to allocate memory close to
+  /// the target device.
+  OMP_MAP_CLOSE = 0x400,
+  /// 0x800 is reserved for compatibility with XLC.
+  /// Produce a runtime error if the data is not already allocated.
+  OMP_MAP_PRESENT = 0x1000,
+  // Increment and decrement a separate reference counter so that the data
+  // cannot be unmapped within the associated region.  Thus, this flag is
+  // intended to be used on 'target' and 'target data' directives because they
+  // are inherently structured.  It is not intended to be used on 'target
+  // enter data' and 'target exit data' directives because they are inherently
+  // dynamic.
+  // This is an OpenMP extension for the sake of OpenACC support.
+  OMP_MAP_OMPX_HOLD = 0x2000,
+  /// Signal that the runtime library should use args as an array of
+  /// descriptor_dim pointers and use args_size as dims. Used when we have
+  /// non-contiguous list items in target update directive
+  OMP_MAP_NON_CONTIG = 0x1000,
+  /// The 16 MSBs of the flags indicate whether the entry is member of some
+  /// struct/class.
+  OMP_MAP_MEMBER_OF = 0x,
+  LLVM_MARK_AS_BITMASK_ENUM(/* LargestFlag = */ OMP_MAP_MEMBER_OF)
 };
 
 enum class AddressSpace : unsigned {
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6796,61 +6796,6 @@
 // code for that information.
 class MappableExprsHandler {
 public:
-  /// Values for bit flags used to specify the mapping type for
-  /// offloading.
-  enum OpenMPOffloadMappingFlags : uint64_t {
-/// No flags
-OMP_MAP_NONE = 0x0,
-/// Allocate memory on the device and move data from host to device.
-OMP_MAP_TO = 0x01,
-/// Allocate memory on the device and move data from device to host.
-OMP_MAP_FROM = 0x02,
-/// Always perform the requested mapping action on the element, even
-/// if it was already mapped before.
-OMP_MAP_ALWAYS = 0x04,
-/// Delete the element from the device environment, ignoring the
-/// current reference count associated with the element.
-OMP_MAP_DELETE = 0x08,
-/// The element being mapped is a pointer-pointee pair; both the
-/// pointer and the pointee should be mapped.
-OMP_MAP_PTR_AND_OBJ = 0x10,
-/// This flags si

[clang] 07ff3c5 - Fix abs labs and llabs to work in C code.

[Doru Bercea via cfe-commits]

Author: Doru Bercea
Date: 2022-12-19T06:28:15-06:00
New Revision: 07ff3c5ccce68aed6c1a270b3f89ea14de7aa250

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

LOG: Fix abs labs and llabs to work in C code.

Added: 
clang/lib/Headers/__clang_hip_stdlib.h
clang/lib/Headers/openmp_wrappers/stdlib.h
clang/test/Headers/amdgcn_openmp_device_math_c.c

Modified: 
clang/lib/Headers/CMakeLists.txt
clang/lib/Headers/__clang_hip_runtime_wrapper.h
clang/test/Headers/Inputs/include/stdlib.h
llvm/utils/gn/secondary/clang/lib/Headers/BUILD.gn

Removed: 




diff  --git a/clang/lib/Headers/CMakeLists.txt 
b/clang/lib/Headers/CMakeLists.txt
index 4206ef27e4ec3..d24691fc50fff 100644
--- a/clang/lib/Headers/CMakeLists.txt
+++ b/clang/lib/Headers/CMakeLists.txt
@@ -60,6 +60,7 @@ set(hip_files
   __clang_hip_libdevice_declares.h
   __clang_hip_cmath.h
   __clang_hip_math.h
+  __clang_hip_stdlib.h
   __clang_hip_runtime_wrapper.h
   )
 

diff  --git a/clang/lib/Headers/__clang_hip_runtime_wrapper.h 
b/clang/lib/Headers/__clang_hip_runtime_wrapper.h
index 10cec58ed12f1..0508731de1062 100644
--- a/clang/lib/Headers/__clang_hip_runtime_wrapper.h
+++ b/clang/lib/Headers/__clang_hip_runtime_wrapper.h
@@ -113,6 +113,7 @@ __attribute__((weak)) inline __device__ void free(void 
*__ptr) {
 
 #include <__clang_hip_libdevice_declares.h>
 #include <__clang_hip_math.h>
+#include <__clang_hip_stdlib.h>
 
 #if defined(__HIPCC_RTC__)
 #include <__clang_hip_cmath.h>

diff  --git a/clang/lib/Headers/__clang_hip_stdlib.h 
b/clang/lib/Headers/__clang_hip_stdlib.h
new file mode 100644
index 0..bd770e2415f95
--- /dev/null
+++ b/clang/lib/Headers/__clang_hip_stdlib.h
@@ -0,0 +1,43 @@
+/*=== __clang_hip_stdlib.h - Device-side HIP math support --===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+#ifndef __CLANG_HIP_STDLIB_H__
+
+#if !defined(__HIP__) && !defined(__OPENMP_AMDGCN__)
+#error "This file is for HIP and OpenMP AMDGCN device compilation only."
+#endif
+
+#if !defined(__cplusplus)
+
+#include 
+
+#ifdef __OPENMP_AMDGCN__
+#define __DEVICE__ static inline __attribute__((always_inline, nothrow))
+#else
+#define __DEVICE__ static __device__ inline __attribute__((always_inline))
+#endif
+
+__DEVICE__
+int abs(int __x) {
+  int __sgn = __x >> (sizeof(int) * CHAR_BIT - 1);
+  return (__x ^ __sgn) - __sgn;
+}
+__DEVICE__
+long labs(long __x) {
+  long __sgn = __x >> (sizeof(long) * CHAR_BIT - 1);
+  return (__x ^ __sgn) - __sgn;
+}
+__DEVICE__
+long long llabs(long long __x) {
+  long long __sgn = __x >> (sizeof(long long) * CHAR_BIT - 1);
+  return (__x ^ __sgn) - __sgn;
+}
+
+#endif // !defined(__cplusplus)
+
+#endif // #define __CLANG_HIP_STDLIB_H__

diff  --git a/clang/lib/Headers/openmp_wrappers/stdlib.h 
b/clang/lib/Headers/openmp_wrappers/stdlib.h
new file mode 100644
index 0..d607469e04f79
--- /dev/null
+++ b/clang/lib/Headers/openmp_wrappers/stdlib.h
@@ -0,0 +1,29 @@
+/*=== openmp_wrapper/stdlib.h -- OpenMP math.h intercept - c++ -===
+ *
+ * Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+ * See https://llvm.org/LICENSE.txt for license information.
+ * SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+ *
+ *===---===
+ */
+
+#ifndef __CLANG_OPENMP_STDLIB_H__
+#define __CLANG_OPENMP_STDLIB_H__
+
+#ifndef _OPENMP
+#error "This file is for OpenMP compilation only."
+#endif
+
+#include_next 
+
+#ifdef __AMDGCN__
+#pragma omp begin declare variant match(device = {arch(amdgcn)})
+
+#define __OPENMP_AMDGCN__
+#include <__clang_hip_stdlib.h>
+#undef __OPENMP_AMDGCN__
+
+#pragma omp end declare variant
+#endif
+
+#endif // __CLANG_OPENMP_STDLIB_H__

diff  --git a/clang/test/Headers/Inputs/include/stdlib.h 
b/clang/test/Headers/Inputs/include/stdlib.h
index 47cd80ca84f01..dc1ff225e3af5 100644
--- a/clang/test/Headers/Inputs/include/stdlib.h
+++ b/clang/test/Headers/Inputs/include/stdlib.h
@@ -6,4 +6,6 @@ void free(void*);
 
 #ifndef __cplusplus
 extern int abs(int __x) __attribute__((__const__));
+extern long labs(long __x) __attribute__((__const__));
+extern long long llabs(long long __x) __attribute__((__const__));
 #endif

diff  --git a/clang/test/Headers/amdgcn_openmp_device_math_c.c 
b/clang/test/Headers/amdgcn_openmp_device_math_c.c
new file mode 100644
index 0..2a54e92ffc4fd
--- /dev/null
+++ b/clang/test/Headers/amdgcn_openmp_device_math_c.c
@@ -0,0 +1,131 @@
+// NOTE: Assertions h

[PATCH] D140292: [OpenMP] Migrate OpenMPOffloadMappingFlags from Clang CodeGen to OMPConstants

[Akash Banerjee via Phabricator via cfe-commits]

TIFitis added inline comments.



Comment at: llvm/include/llvm/Frontend/OpenMP/OMPConstants.h:193
-  OMP_TGT_EXEC_MODE_GENERIC | OMP_TGT_EXEC_MODE_SPMD,
-  LLVM_MARK_AS_BITMASK_ENUM(/* LargestValue */ OMP_TGT_EXEC_MODE_GENERIC_SPMD)
 };

I am not sure if this change is safe. It can be avoided by making 
`OpenMPOffloadMappingFlags` an enum class.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140292

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

[PATCH] D140261: [C++20][Modules] Do not allow non-inline external definitions in header units.

[Iain Sandoe via Phabricator via cfe-commits]

iains updated this revision to Diff 483909.
iains added a comment.

rebased, addressed comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140261

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDecl.cpp
  clang/test/CXX/module/module.import/p6.cpp
  clang/test/CodeGenCXX/module-initializer-header.cppm

Index: clang/test/CodeGenCXX/module-initializer-header.cppm
===
--- clang/test/CodeGenCXX/module-initializer-header.cppm
+++ clang/test/CodeGenCXX/module-initializer-header.cppm
@@ -8,24 +8,24 @@
 //
 //--- header.h
 int foo();
-int i = foo();
+static int i = foo();
 
 //--- M.cppm
 module;
 import "header.h";
 export module M;
 
-// CHECK: @i = {{.*}}global i32 0
+// CHECK: @_ZL1i = {{.*}}global i32 0
 // CHECK: void @__cxx_global_var_init()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:  %call = call noundef{{.*}} i32 @_Z3foov()
-// CHECK-NEXT:  store i32 %call, ptr @i  
+// CHECK-NEXT:  store i32 %call, ptr @_ZL1i  
 
 //--- Use.cpp
 import "header.h";
 
-// CHECK: @i = {{.*}}global i32 0
+// CHECK: @_ZL1i = {{.*}}global i32 0
 // CHECK: void @__cxx_global_var_init()
 // CHECK-NEXT: entry:
 // CHECK-NEXT:  %call = call noundef{{.*}} i32 @_Z3foov()
-// CHECK-NEXT:  store i32 %call, ptr @i  
+// CHECK-NEXT:  store i32 %call, ptr @_ZL1i  
Index: clang/test/CXX/module/module.import/p6.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.import/p6.cpp
@@ -0,0 +1,24 @@
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -x c++-header %t/bad-header-unit.h \
+// RUN:  -emit-header-unit -o %t/bad-header-unit.pcm -verify
+
+//--- bad-header-unit.h
+
+inline int ok_foo () { return 0;}
+
+static int ok_bar ();
+
+int ok_decl ();
+
+int bad_def () { return 2;}  // expected-error {{non-inline external definitions are not permitted in C++ header units}}
+
+inline int ok_inline_var = 1;
+
+static int ok_static_var;
+
+int ok_var_decl;
+
+int bad_var_definition = 3;  // expected-error {{non-inline external definitions are not permitted in C++ header units}}
+
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -12952,6 +12952,15 @@
   VDecl->setInvalidDecl();
   }
 
+  // C++ [module.import/6] external definitions are not permitted in header
+  // units.
+  if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
+  VDecl->getFormalLinkage() == Linkage::ExternalLinkage &&
+  !VDecl->isInline()) {
+Diag(VDecl->getLocation(), diag::err_extern_def_in_header_unit);
+VDecl->setInvalidDecl();
+  }
+
   // If adding the initializer will turn this declaration into a definition,
   // and we already have a definition for this variable, diagnose or otherwise
   // handle the situation.
@@ -15100,6 +15109,14 @@
 }
   }
 
+  // C++ [module.import/6] external definitions are not permitted in header
+  // units.
+  if (getLangOpts().CPlusPlusModules && currentModuleIsHeaderUnit() &&
+  FD->getFormalLinkage() == Linkage::ExternalLinkage && !FD->isInlined()) {
+Diag(FD->getLocation(), diag::err_extern_def_in_header_unit);
+FD->setInvalidDecl();
+  }
+
   // Ensure that the function's exception specification is instantiated.
   if (const FunctionProtoType *FPT = FD->getType()->getAs())
 ResolveExceptionSpec(D->getLocation(), FPT);
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2306,6 +2306,12 @@
 return ModuleScopes.empty() ? false : ModuleScopes.back().ModuleInterface;
   }
 
+  /// Is the module scope we are in a C++ Header Unit?
+  bool currentModuleIsHeaderUnit() const {
+return ModuleScopes.empty() ? false
+: ModuleScopes.back().Module->isHeaderUnit();
+  }
+
   /// Get the module owning an entity.
   Module *getOwningModule(const Decl *Entity) {
 return Entity->getOwningModule();
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -11233,6 +11233,8 @@
   "add 'export' here if this is intended to be a module interface unit">;
 def err_invalid_module_name : Error<
   "%0 is %select{an invalid|a reserved}1 name for a module">;
+def err_extern_def_in_header_unit : Error<
+  "non-inline external definitions are not permitted in C++ header units">;
 
 def ext_equivalent_internal_linkage_decl_in_modules : ExtWarn<
   "ambiguous use of internal linkage declaration %0 defined in multiple modules">,

[PATCH] D139723: [OpenMP][AMDGPU] Enable use of abs labs and llabs math functions in C code

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 closed this revision.
doru1004 added a comment.

Commit: 07ff3c5ccce68aed6c1a270b3f89ea14de7aa250 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139723

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

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 483910.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

Files:
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -7,11 +7,17 @@
 //===--===//
 
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -31,7 +37,7 @@
   std::unique_ptr AST;
   FindHeadersTest() {
 Inputs.MakeAction = [this] {
-  struct Hook : public PreprocessOnlyAction {
+  struct Hook : public SyntaxOnlyAction {
   public:
 Hook(PragmaIncludes *Out) : Out(Out) {}
 bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
@@ -153,5 +159,89 @@
physicalHeader("exporter.h")));
 }
 
+struct CustomVisitor : RecursiveASTVisitor {
+  const Decl *Out = nullptr;
+
+  bool VisitCXXRecordDecl(const CXXRecordDecl *ND) {
+if (ND->getName() == "Foo") {
+  EXPECT_TRUE(Out == nullptr);
+  Out = ND;
+}
+return true;
+  }
+
+  bool VisitNamedDecl(const NamedDecl *ND) {
+if (ND->getName() == "FLAG_foo") {
+  EXPECT_TRUE(Out == nullptr);
+  Out = ND;
+}
+return true;
+  }
+};
+
+TEST_F(FindHeadersTest, TargetIsExpandedFromMacroInHeader) {
+  struct {
+llvm::StringRef Main;
+llvm::StringRef DeclareHeader;
+llvm::StringRef MacroHeader;
+  } TestCases[] = {{
+   R"cpp(
+#include "declare.h"
+Foo foo;
+  )cpp",
+   R"cpp(
+#include "macro.h"
+DEFINE_CLASS(Foo)
+  )cpp",
+   R"cpp(
+#define DEFINE_CLASS(name) class name {};
+  )cpp"},
+   {
+   R"cpp(
+#include "declare.h"
+void test(Foo foo);
+  )cpp",
+   R"cpp(
+#include "macro.h"
+DEFINE_Foo
+  )cpp",
+   R"cpp(
+#define DEFINE_Foo class Foo {};
+  )cpp"},
+   {
+   R"cpp(
+#include "declare.h"
+void foo() {
+  FLAG_foo;
+}
+  )cpp",
+   R"cpp(
+#include "macro.h"
+DECLARE_FLAGS(foo);
+  )cpp",
+   R"cpp(
+#define DECLARE_FLAGS(name) extern int FLAG_##name
+  )cpp"}};
+
+  for (const auto &T : TestCases) {
+llvm::Annotations MainFile(T.Main);
+Inputs.Code = MainFile.code();
+Inputs.ExtraFiles["declare.h"] = T.DeclareHeader;
+Inputs.ExtraFiles["macro.h"] = T.MacroHeader;
+buildAST();
+
+CustomVisitor Visitor;
+for (Decl *D : AST->context().getTranslationUnitDecl()->decls()) {
+  Visitor.TraverseDecl(D);
+}
+
+const Decl *Foo = Visitor.Out;
+llvm::SmallVector Headers = clang::include_cleaner::findHeaders(
+Foo->getLocation(), AST->sourceManager(), nullptr);
+EXPECT_EQ(Headers.size(), 1u);
+EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));
+  }
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
===
--- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -18,8 +18,7 @@
   llvm::SmallVector Results;
   switch (Loc.kind()) {
   case SymbolLocation::Physical: {
-// FIXME: Handle macro locations.
-FileID FID = SM.getFileID(Loc.physical());
+FileID FID = SM.getFileID(SM.getExpansionLoc(Loc.physical()));
 const FileEntry *FE = SM.getFileEntryForID(FID);
 if (!PI) {
   return FE ? llvm::SmallVector{Header(FE)}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140261: [C++20][Modules] Do not allow non-inline external definitions in header units.

[Iain Sandoe via Phabricator via cfe-commits]

iains marked 2 inline comments as done.
iains added a comment.

OK so this is what I plan to land assuming testing goes OK.
I suspect that this might cause some user code to flag errors - there are quite 
a number of ODR violations "in the wild".




Comment at: clang/lib/Sema/SemaDecl.cpp:12957-12958
+  // units.
+  if (getLangOpts().CPlusPlus20 && getLangOpts().CPlusPlusModules &&
+  !ModuleScopes.empty() && ModuleScopes.back().Module->isHeaderUnit()) {
+if (VDecl->getFormalLinkage() == Linkage::ExternalLinkage &&

ChuanqiXu wrote:
> iains wrote:
> > ChuanqiXu wrote:
> > > `getLangOpts().CPlusPlus20` is redundant. It is also good to define a 
> > > helper interface `withinHeaderUnit` in Sema (not required).
> > I do not mind making this change - but note that the constraint is specific 
> > to C++20 and there are some people who want to remove it (the constraint).  
> > I guess  you meant `getCurrentModule()` ?
> > I do not mind making this change - but note that the constraint is specific 
> > to C++20 and there are some people who want to remove it (the constraint).
> 
> Got it. Let's make it when it comes true. It won't be a big deal.
> 
> > I guess you meant getCurrentModule() ?
> 
> It looks **a little bit** better to me to not access `ModuleScopes` directly.
revised now (I added the helper).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140261

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

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 483911.
VitaNuo added a comment.

Add guard calls to headers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

Files:
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -7,11 +7,17 @@
 //===--===//
 
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -31,7 +37,7 @@
   std::unique_ptr AST;
   FindHeadersTest() {
 Inputs.MakeAction = [this] {
-  struct Hook : public PreprocessOnlyAction {
+  struct Hook : public SyntaxOnlyAction {
   public:
 Hook(PragmaIncludes *Out) : Out(Out) {}
 bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
@@ -153,5 +159,89 @@
physicalHeader("exporter.h")));
 }
 
+struct CustomVisitor : RecursiveASTVisitor {
+  const Decl *Out = nullptr;
+
+  bool VisitCXXRecordDecl(const CXXRecordDecl *ND) {
+if (ND->getName() == "Foo") {
+  EXPECT_TRUE(Out == nullptr);
+  Out = ND;
+}
+return true;
+  }
+
+  bool VisitNamedDecl(const NamedDecl *ND) {
+if (ND->getName() == "FLAG_foo") {
+  EXPECT_TRUE(Out == nullptr);
+  Out = ND;
+}
+return true;
+  }
+};
+
+TEST_F(FindHeadersTest, TargetIsExpandedFromMacroInHeader) {
+  struct {
+llvm::StringRef Main;
+llvm::StringRef DeclareHeader;
+llvm::StringRef MacroHeader;
+  } TestCases[] = {{
+   R"cpp(
+#include "declare.h"
+Foo foo;
+  )cpp",
+   R"cpp(
+#include "macro.h"
+DEFINE_CLASS(Foo)
+  )cpp",
+   R"cpp(
+#define DEFINE_CLASS(name) class name {};
+  )cpp"},
+   {
+   R"cpp(
+#include "declare.h"
+void test(Foo foo);
+  )cpp",
+   R"cpp(
+#include "macro.h"
+DEFINE_Foo
+  )cpp",
+   R"cpp(
+#define DEFINE_Foo class Foo {};
+  )cpp"},
+   {
+   R"cpp(
+#include "declare.h"
+void foo() {
+  FLAG_foo;
+}
+  )cpp",
+   R"cpp(
+#include "macro.h"
+DECLARE_FLAGS(foo);
+  )cpp",
+   R"cpp(
+#define DECLARE_FLAGS(name) extern int FLAG_##name
+  )cpp"}};
+
+  for (const auto &T : TestCases) {
+llvm::Annotations MainFile(T.Main);
+Inputs.Code = MainFile.code();
+Inputs.ExtraFiles["declare.h"] = guard(T.DeclareHeader);
+Inputs.ExtraFiles["macro.h"] = guard(T.MacroHeader);
+buildAST();
+
+CustomVisitor Visitor;
+for (Decl *D : AST->context().getTranslationUnitDecl()->decls()) {
+  Visitor.TraverseDecl(D);
+}
+
+const Decl *Foo = Visitor.Out;
+llvm::SmallVector Headers = clang::include_cleaner::findHeaders(
+Foo->getLocation(), AST->sourceManager(), nullptr);
+EXPECT_EQ(Headers.size(), 1u);
+EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));
+  }
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
===
--- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -18,8 +18,7 @@
   llvm::SmallVector Results;
   switch (Loc.kind()) {
   case SymbolLocation::Physical: {
-// FIXME: Handle macro locations.
-FileID FID = SM.getFileID(Loc.physical());
+FileID FID = SM.getFileID(SM.getExpansionLoc(Loc.physical()));
 const FileEntry *FE = SM.getFileEntryForID(FID);
 if (!PI) {
   return FE ? llvm::SmallVector{Header(FE)}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:161
 
+TEST_F(FindHeadersTest, TargetIsArgumentExpandedFromMacroInHeader) {
+  llvm::Annotations MainFile(R"cpp(

hokein wrote:
> these 3 newly-added tests are similar, instead of duplicating them, we can 
> use a table-gen style test, see an 
> [example](https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp#L265)
Great idea, thanks!



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:186
+  std::vector Headers;
+  walkUsed(
+  TopLevelDecls, {}, nullptr, AST->sourceManager(),

hokein wrote:
> The intention of the test is to test the function `findHeaders`, we're 
> testing it in an implicit way (`findHeaders` is hidden inside the 
> `walkUsed`), there is no code here explicitly calling this function. 
> 
> We can make it clearer (no need to use walkUsed):
> 1. we get a `Foo` Decl AST node from the testcase.
> 2. with the AST node from step1, we cam call the `findHeaders` directly in 
> the test code.
> 
> for 1, we can just write a simple RecursiveASTVisitor, and capture a 
> NamedDecl which called Foo, something like:
> 
> ```
>  struct Visitor : RecursiveASTVisitor {
>   const NamedDecl *Out = nullptr;
>   bool VisitNamedDecl(const NamedDecl *ND) {
> if (ND->getName() == "Foo") {
>   EXPECT_TRUE(Out == nullptr);
>   Out = ND;
> }
> return true;
>   }
> };
> ```
Ok, I've attempted to follow your advice now, please have a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

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

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added inline comments.



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:193
 
-  if (any_isa(IR)) {
-const Function *F = any_cast(IR);
-return F->getName().str();
+  if (const auto **F = any_cast(&IR)) {
+return (*F)->getName().str();

Redundant braces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[clang-tools-extra] 0e54581 - [include-cleaner] Handle dependent type members in AST.

[Viktoriia Bakalova via cfe-commits]

Author: Viktoriia Bakalova
Date: 2022-12-19T12:45:41Z
New Revision: 0e545816a9e582af29ea4b9441fea8ed376cf52a

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

LOG: [include-cleaner] Handle dependent type members in AST.

Handles dependent type members in AST.

Fix: https://github.com/llvm/llvm-project/issues/59354
Differential Revision: https://reviews.llvm.org/D139409

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/WalkAST.cpp
clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp 
b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index 639add8f5c4be..cf2373d43389d 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -84,6 +84,10 @@ class ASTWalker : public RecursiveASTVisitor {
 report(E->getMemberLoc(), getMemberProvider(Type));
 return true;
   }
+  bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+report(E->getMemberLoc(), getMemberProvider(E->getBaseType()));
+return true;
+  }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
 report(E->getLocation(), E->getConstructor(),

diff  --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index cb41ebb35cd58..2a2fbc438ab9b 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -229,6 +229,13 @@ TEST(WalkAST, MemberExprs) {
   namespace ns { template struct Foo { int a; }; }
   using ns::$explicit^Foo;)cpp",
"void k(Foo b) { b.^a; }");
+  // Test the dependent-type case (CXXDependentScopeMemberExpr)
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base& t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base* t) { t->^method(); }");
 }
 
 TEST(WalkAST, ConstructExprs) {



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

[PATCH] D139409: [include-cleaner] Handle dependent type members in AST

[Viktoriia Bakalova via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0e545816a9e5: [include-cleaner] Handle dependent type 
members in AST. (authored by VitaNuo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139409

Files:
  clang-tools-extra/include-cleaner/lib/WalkAST.cpp
  clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -229,6 +229,13 @@
   namespace ns { template struct Foo { int a; }; }
   using ns::$explicit^Foo;)cpp",
"void k(Foo b) { b.^a; }");
+  // Test the dependent-type case (CXXDependentScopeMemberExpr)
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base& t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base* t) { t->^method(); }");
 }
 
 TEST(WalkAST, ConstructExprs) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -84,6 +84,10 @@
 report(E->getMemberLoc(), getMemberProvider(Type));
 return true;
   }
+  bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+report(E->getMemberLoc(), getMemberProvider(E->getBaseType()));
+return true;
+  }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
 report(E->getLocation(), E->getConstructor(),


Index: clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -229,6 +229,13 @@
   namespace ns { template struct Foo { int a; }; }
   using ns::$explicit^Foo;)cpp",
"void k(Foo b) { b.^a; }");
+  // Test the dependent-type case (CXXDependentScopeMemberExpr)
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base& t) { t.^method(); }");
+  testWalk("template struct $explicit^Base { void method(); };",
+   "template void k(Base* t) { t->^method(); }");
 }
 
 TEST(WalkAST, ConstructExprs) {
Index: clang-tools-extra/include-cleaner/lib/WalkAST.cpp
===
--- clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -84,6 +84,10 @@
 report(E->getMemberLoc(), getMemberProvider(Type));
 return true;
   }
+  bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
+report(E->getMemberLoc(), getMemberProvider(E->getBaseType()));
+return true;
+  }
 
   bool VisitCXXConstructExpr(CXXConstructExpr *E) {
 report(E->getLocation(), E->getConstructor(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112621: [analyzer][solver] Introduce reasoning for not equal to operator

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

steakhal added a comment.

In D112621#4004409 , @steakhal wrote:

> In D112621#4004372 , @uabelho wrote:
>
>> Hi,
>>
>> The following starts crashing with this patch:
>>
>>   clang -cc1 -analyze -analyzer-checker=core bbi-77010.c
>>
>> It crashes with
>>
>>   bbi-77010.c:6:1: warning: non-void function does not return a value 
>> [-Wreturn-type]
>>   }
>>   ^
>>   clang: 
>> ../../clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1622: 
>> clang::ento::RangeSet (anonymous 
>> namespace)::SymbolicRangeInferrer::VisitBinaryOperator(clang::ento::RangeSet,
>>  clang::ento::RangeSet, clang::QualType): Assertion `!LHS.isEmpty() && 
>> !RHS.isEmpty() && "Both ranges should be non-empty"' failed.
>>   PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
>> and include the crash backtrace, preprocessed source, and associated run 
>> script.
>>   Abort (core dumped)
>>
>> F25730184: bbi-77010.c 
>
> Thanks for the report. I'll fix it ASAP.
> I think I'll replace the assertion with an early return.
>
> BTW, was this uncovered by fuzzing? @uabelho

Fixed by f61a08b67f5c8b0202dd30821766ee029e880b7c 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112621

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

[PATCH] D139629: clang: Stop emitting "strictfp"

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2135
-llvm::AttrBuilder FuncAttrs(F->getContext());
-FuncAttrs.addAttribute("strictfp");
-F->addFnAttrs(FuncAttrs);

arsenm wrote:
> andrew.w.kaylor wrote:
> > arsenm wrote:
> > > zahiraam wrote:
> > > > I think it would better to fix this function instead of removing it 
> > > > entirely? The issue here is that there is the "strictfp" attribute and 
> > > > the llvm::Attribute::StrictFP. We could replace  
> > > > FuncAttrs.addAttribute("strictfp"); with
> > > >  FuncAttrs.addAttribute(llvm::Attribute::StrictFP); 
> > > > This function ensures that the function attribute is set when the 
> > > > FunctionDecl attribute is set. I am concerned that when it's removed, 
> > > > we will wind up with cases where the function attribute is missing! The 
> > > > only place where this function attribute is in 
> > > > CodeGenFunction::StartFunction. Is that enough? @andrew.w.kaylor Can 
> > > > you please weigh in on this?
> > > I currently don't have evidence that making this use the correct 
> > > attribute would fix anything. If something was depending on this emission 
> > > in this context, it's already broken
> > It may be that anything depending on this is already broken, but the code 
> > was written for a reason, even if it was always broken. I'm not sure I 
> > understand what that reason was, and unfortunately the person who wrote the 
> > code (@mibintc) is no longer actively contributing to LLVM. It was added 
> > here: https://reviews.llvm.org/D87528
> > 
> > It does seem like the llvm::Attribute::StrictFP is being added any time the 
> > string attribute is added, but they're coming from different places. The 
> > proper attribute seems to be coming from CodeGenFunction::StartFunction() 
> > which is effectively copying it from the function declaration. It's not 
> > clear to me whether there are circumstances where we get to 
> > setLLVMFunctionFEnvAttributes() through EmitGlobalFunctionDefinition() 
> > without ever having gone through CodeGenFunction::StartFunction(). It looks 
> > like maybe there are multiversioning cases that do that, but I couldn't 
> > come up with an example that does. @erichkeane wrote a lot of the 
> > multi-versioning code, so he might know more, but he's on vacation through 
> > the end of the month.
> > 
> > Eliminating this extra string attribute seems obviously good. In this 
> > particular location, though, I'd be inclined to set the enumerated 
> > attribute here, even though that might be redundant in most cases. On the 
> > other hand, if one of our front end experts can look at this code and say 
> > definitively that it's //always// redundant, I'd be fine with this code 
> > being deleted.
> I think code that can be deleted that doesn't manifest in test failures 
> should be immediately deleted. We shouldn't leave things around just in case 
The Verifier changes that would detect the error and fail tests never made it 
into the tree. The lack of failures therefore tells us nothing in this case 
here.


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

https://reviews.llvm.org/D139629

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

[PATCH] D140217: [lit] Script to automate use of %(line-n). Use in CodeComplete tests.

[Sam McCall via Phabricator via cfe-commits]

sammccall marked 6 inline comments as done.
sammccall added inline comments.



Comment at: llvm/utils/relative_lines.py:17
+
+USAGE = """Example usage:
+find clang/test/CodeCompletion | grep -v /Inputs/ | \

hokein wrote:
> This variable is not used in anywhere, and not print out in `--help` mode. I 
> think it is more useful to move it to the above file comment, at least it is 
> shown in `--help` mode.
oops, I meant to add it as the help epilog. done now



Comment at: llvm/utils/relative_lines.py:20
+xargs relative_lines.py --dry-run --verbose --near=100 \
+--pattern='-code-completion-at[ =]%s:(\\d+)' \
+--pattern='requires fix-it: {(\d+):\d+-(\d+):\d+}'

hokein wrote:
> `\\d` should be `\d`?
Indeed - I'm surprised neither python nor shell requires the backslash to be 
escaped...



Comment at: llvm/utils/relative_lines.py:94
+for pattern in args.pattern:
+contents = re.sub(pattern, replace_match, contents)
+if failures > 0 and not args.partial:

hokein wrote:
> looks like we modify the file more eagerly -- if there are no matching 
> pattern, we will replace the content as well (most cases are ok, I saw the 
> CRLF => LF change, I think we'd better only overwrite the content if there 
> are matching patterns). 
Oops, this is processing as text rather than binary. Added ugly bytes 
conversions everywhere.

(Rewriting the file after no changes seems silly, but it should be a no-op and 
it shows up bugs like this)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140217

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

[PATCH] D140294: clang: Replace implementation of __builtin_isnormal

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: bkramer, foad, sepavloff, andrew.w.kaylor, kpn.
Herald added a project: All.
arsenm requested review of this revision.
Herald added a subscriber: wdng.

This was doing an explicit non-canonical isnan check, then two
unordered comparisons. We can fold the ordered check into an ordered
compare. I recently fixed InstCombine to fold this pattern, but might
as well give it less work to do.

  

https://alive2.llvm.org/ce/z/ZWVHhT


https://reviews.llvm.org/D140294

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins.c


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -261,20 +261,16 @@
   // CHECK: fcmp one double {{.*}}, 0x7FF0
 
   res = __builtin_isnormal(*H);
-  // CHECK: fcmp oeq half
-  // CHECK: call half @llvm.fabs.f16(half
-  // CHECK: fcmp ult half {{.*}}, 0xH7C00
-  // CHECK: fcmp uge half {{.*}}, 0xH0400
-  // CHECK: and i1
-  // CHECK: and i1
+  // CHECK: %[[FABS:.*]] = call half @llvm.fabs.f16(half %[[VAL:.*]])
+  // CHECK: %[[ISFINITE:.*]] = fcmp olt half %[[FABS]], 0xH7C00
+  // CHECK: %[[ISNORMAL:.*]] = fcmp uge half %[[FABS]], 0xH0400
+  // CHECK: and i1 %[[ISFINITE]], %[[ISNORMAL]]
 
   res = __builtin_isnormal(F);
-  // CHECK: fcmp oeq float
-  // CHECK: call float @llvm.fabs.f32(float
-  // CHECK: fcmp ult float {{.*}}, 0x7FF0
-  // CHECK: fcmp uge float {{.*}}, 0x3810
-  // CHECK: and i1
-  // CHECK: and i1
+  // CHECK: %[[FABS:.*]] = call float @llvm.fabs.f32(float %[[VAL:.*]])
+  // CHECK: %[[ISFINITE:.*]] = fcmp olt float %[[FABS]], 0x7FF0
+  // CHECK: %[[ISNORMAL:.*]] = fcmp uge float %[[FABS]], 0x3810
+  // CHECK: and i1 %[[ISFINITE]], %[[ISNORMAL]]
 
   res = __builtin_flt_rounds();
   // CHECK: call i32 @llvm.get.rounding(
Index: clang/lib/CodeGen/CGBuiltin.cpp
===
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -3305,22 +3305,18 @@
   }
 
   case Builtin::BI__builtin_isnormal: {
-// isnormal(x) --> x == x && fabsf(x) < infinity && fabsf(x) >= float_min
+// isnormal(x) --> fabs(x) < infinity && !(fabs(x) < float_min)
 CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);
 // FIXME: for strictfp/IEEE-754 we need to not trap on SNaN here.
 Value *V = EmitScalarExpr(E->getArg(0));
-Value *Eq = Builder.CreateFCmpOEQ(V, V, "iseq");
-
 Value *Abs = EmitFAbs(*this, V);
-Value *IsLessThanInf =
-  Builder.CreateFCmpULT(Abs, 
ConstantFP::getInfinity(V->getType()),"isinf");
-APFloat Smallest = APFloat::getSmallestNormalized(
-   
getContext().getFloatTypeSemantics(E->getArg(0)->getType()));
-Value *IsNormal =
-  Builder.CreateFCmpUGE(Abs, ConstantFP::get(V->getContext(), Smallest),
-"isnormal");
-V = Builder.CreateAnd(Eq, IsLessThanInf, "and");
-V = Builder.CreateAnd(V, IsNormal, "and");
+Value *IsFinite = Builder.CreateFCmpOLT(
+Abs, ConstantFP::getInfinity(V->getType()), "isfinite");
+APFloat SmallestNormal = APFloat::getSmallestNormalized(
+getContext().getFloatTypeSemantics(E->getArg(0)->getType()));
+Value *IsNormal = Builder.CreateFCmpUGE(
+Abs, ConstantFP::get(V->getContext(), SmallestNormal), "isnormal");
+V = Builder.CreateAnd(IsFinite, IsNormal, "and");
 return RValue::get(Builder.CreateZExt(V, ConvertType(E->getType(;
   }
 


Index: clang/test/CodeGen/builtins.c
===
--- clang/test/CodeGen/builtins.c
+++ clang/test/CodeGen/builtins.c
@@ -261,20 +261,16 @@
   // CHECK: fcmp one double {{.*}}, 0x7FF0
 
   res = __builtin_isnormal(*H);
-  // CHECK: fcmp oeq half
-  // CHECK: call half @llvm.fabs.f16(half
-  // CHECK: fcmp ult half {{.*}}, 0xH7C00
-  // CHECK: fcmp uge half {{.*}}, 0xH0400
-  // CHECK: and i1
-  // CHECK: and i1
+  // CHECK: %[[FABS:.*]] = call half @llvm.fabs.f16(half %[[VAL:.*]])
+  // CHECK: %[[ISFINITE:.*]] = fcmp olt half %[[FABS]], 0xH7C00
+  // CHECK: %[[ISNORMAL:.*]] = fcmp uge half %[[FABS]], 0xH0400
+  // CHECK: and i1 %[[ISFINITE]], %[[ISNORMAL]]
 
   res = __builtin_isnormal(F);
-  // CHECK: fcmp oeq float
-  // CHECK: call float @llvm.fabs.f32(float
-  // CHECK: fcmp ult float {{.*}}, 0x7FF0
-  // CHECK: fcmp uge float {{.*}}, 0x3810
-  // CHECK: and i1
-  // CHECK: and i1
+  // CHECK: %[[FABS:.*]] = call float @llvm.fabs.f32(float %[[VAL:.*]])
+  // CHECK: %[[ISFINITE:.*]] = fcmp olt float %[[FABS]], 0x7FF0
+  // CHECK: %[[ISNORMAL:.*]] = fcmp uge float %[[FABS]], 0x3810
+  // CHECK: and i1 %[[ISFINITE]], %[[ISNORMAL]]
 
   res = __builtin_flt_rounds();
   // CHECK: call i32 @llvm.get.rounding(
Index: clang/lib/CodeGen/CGBuiltin.cpp
==

[PATCH] D138300: [clangd] Support type hints for `decltype(expr)`

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

@nridge Thank you for reviewing!

If this patch is ready to land, could you please help me commit it? Thanks 
again!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138300

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

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

thanks, the test looks better now, a few more suggestions.




Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:162
 
+struct CustomVisitor : RecursiveASTVisitor {
+  const Decl *Out = nullptr;

Move the definition to the `TEST_F` body.



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:174
+  bool VisitNamedDecl(const NamedDecl *ND) {
+if (ND->getName() == "FLAG_foo") {
+  EXPECT_TRUE(Out == nullptr);

I think `if (ND->getName() == "FLAG_foo" || ND->getName() == "Foo")` should 
just work (without the `VisitCXXRecordDecl` method)?



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:190
+#include "declare.h"
+Foo foo;
+  )cpp",

I think the `Foo foo;` is not really needed for the test (`#include 
"declare.h"` is the key part of the main file) as we only want the location of 
the `CXXRecordDecl`. The same for the following testcases.

So I think we can simplify it further: TestCases has two members 
(`DeclareHeader`, `MacroHeader`). The main file content is always `#include 
"declare.h"`, we can construct the main code in the for-loop body.





Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:227
+  for (const auto &T : TestCases) {
+llvm::Annotations MainFile(T.Main);
+Inputs.Code = MainFile.code();

nit: Annotations is not needed as we don't use any annotation here.



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:233
+
+CustomVisitor Visitor;
+for (Decl *D : AST->context().getTranslationUnitDecl()->decls()) {

Visitor.TraverseDecl(AST->context().getTranslationUnitDecl());



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:241
+Foo->getLocation(), AST->sourceManager(), nullptr);
+EXPECT_EQ(Headers.size(), 1u);
+EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));

this is not needed -- the following `EXPECT_THAT` already covers it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

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

[PATCH] D140217: [lit] Script to automate use of %(line-n). Use in CodeComplete tests.

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
sammccall marked 3 inline comments as done.
Closed by commit rGcf9b25e0adc4: [lit] Script to automate use of %(line-n). Use 
in CodeComplete tests. (authored by sammccall).

Changed prior to commit:
  https://reviews.llvm.org/D140217?vs=483547&id=483922#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140217

Files:
  clang/test/CodeCompletion/PR9728.cpp
  clang/test/CodeCompletion/accessibility-crash.cpp
  clang/test/CodeCompletion/accessibility.cpp
  clang/test/CodeCompletion/after-function-equals.cpp
  clang/test/CodeCompletion/attr.cpp
  clang/test/CodeCompletion/auto.cpp
  clang/test/CodeCompletion/auto_type.c
  clang/test/CodeCompletion/bracket-decl.c
  clang/test/CodeCompletion/call.c
  clang/test/CodeCompletion/call.cpp
  clang/test/CodeCompletion/comments.cpp
  clang/test/CodeCompletion/constexpr.cpp
  clang/test/CodeCompletion/crash-func-decl.cpp
  clang/test/CodeCompletion/crash-func-init.cpp
  clang/test/CodeCompletion/crash-if-directive.cpp
  clang/test/CodeCompletion/crash-null-type.cpp
  clang/test/CodeCompletion/ctor-initializer.cpp
  clang/test/CodeCompletion/ctor-signature.cpp
  clang/test/CodeCompletion/desig-init.cpp
  clang/test/CodeCompletion/deuglify.cpp
  clang/test/CodeCompletion/documentation.cpp
  clang/test/CodeCompletion/documentation.m
  clang/test/CodeCompletion/enable-if-attr-crash.cpp
  clang/test/CodeCompletion/end-of-file.cpp
  clang/test/CodeCompletion/end-of-ident-macro.cpp
  clang/test/CodeCompletion/end-of-ident.cpp
  clang/test/CodeCompletion/enum-preferred-type.cpp
  clang/test/CodeCompletion/enum-switch-case-qualified.cpp
  clang/test/CodeCompletion/enum-switch-case.c
  clang/test/CodeCompletion/enum-switch-case.cpp
  clang/test/CodeCompletion/function-templates.cpp
  clang/test/CodeCompletion/functions.cpp
  clang/test/CodeCompletion/ignore-ns-level-decls.cpp
  clang/test/CodeCompletion/included-symlinks.cpp
  clang/test/CodeCompletion/incomplete-member.cpp
  clang/test/CodeCompletion/incomplete-ret-type.cpp
  clang/test/CodeCompletion/inside-macros.cpp
  clang/test/CodeCompletion/invalid-initialized-class.cpp
  clang/test/CodeCompletion/lambdas.cpp
  clang/test/CodeCompletion/macros-in-modules.c
  clang/test/CodeCompletion/macros-in-modules.m
  clang/test/CodeCompletion/macros.c
  clang/test/CodeCompletion/member-access-qualifiers.cpp
  clang/test/CodeCompletion/member-access.c
  clang/test/CodeCompletion/namespace-alias.cpp
  clang/test/CodeCompletion/namespace.cpp
  clang/test/CodeCompletion/nested-name-specifier.cpp
  clang/test/CodeCompletion/objc-expr.m
  clang/test/CodeCompletion/objc-member-access.m
  clang/test/CodeCompletion/objc-message.m
  clang/test/CodeCompletion/objc-message.mm
  clang/test/CodeCompletion/objc-protocol-member-access.m
  clang/test/CodeCompletion/operator.cpp
  clang/test/CodeCompletion/ordinary-name.c
  clang/test/CodeCompletion/overrides.cpp
  clang/test/CodeCompletion/paren_locs.cpp
  clang/test/CodeCompletion/pragma-macro-token-caching.c
  clang/test/CodeCompletion/preamble.c
  clang/test/CodeCompletion/preferred-type.cpp
  clang/test/CodeCompletion/qualifiers-as-written.cpp
  clang/test/CodeCompletion/self-inits.cpp
  clang/test/CodeCompletion/signatures-crash.cpp
  clang/test/CodeCompletion/tag.c
  clang/test/CodeCompletion/tag.cpp
  clang/test/CodeCompletion/template-signature.cpp
  clang/test/CodeCompletion/templates.cpp
  clang/test/CodeCompletion/this-quals.cpp
  clang/test/CodeCompletion/truncation.c
  clang/test/CodeCompletion/uninstantiated_params.cpp
  clang/test/CodeCompletion/using-enum.cpp
  clang/test/CodeCompletion/using-namespace.cpp
  clang/test/CodeCompletion/using.cpp
  clang/test/CodeCompletion/variadic-template.cpp
  llvm/utils/relative_lines.py

Index: llvm/utils/relative_lines.py
===
--- /dev/null
+++ llvm/utils/relative_lines.py
@@ -0,0 +1,107 @@
+#!/usr/bin/env python3
+
+"""Replaces absolute line numbers in lit-tests with relative line numbers.
+
+Writing line numbers like 152 in 'RUN: or CHECK:' makes tests hard to maintain:
+inserting lines in the middle of the test means updating all the line numbers.
+
+Encoding them relative to the current line helps, and tools support it:
+Lit will substitute %(line+2) with the actual line number
+FileCheck supports [[@LINE+2]]
+
+This tool takes a regex which captures a line number, and a list of test files.
+It searches for line numbers in the files and replaces them with a relative
+line number reference.
+"""
+
+USAGE = """Example usage:
+find -type f clang/test/CodeCompletion | grep -v /Inputs/ | \\
+xargs relative_lines.py --dry-run --verbose --near=100 \\
+--pattern='-code-completion-at[ =]%s:(\d+)' \\
+--pattern='requires fix-it: {(\d+):\d+-(\d+):\d+}'
+"""

[PATCH] D140295: [Fix][OpenMP] Fix commit for nohost variant.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 created this revision.
doru1004 added reviewers: ABataev, carlo.bertolli, ronl.
doru1004 added a project: OpenMP.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
doru1004 requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This fixes the previous commit: 658ed9547cdd6657895339a6c390c31aa77a5698 



Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140295

Files:
  clang/test/OpenMP/declare_target_messages.cpp
  clang/test/OpenMP/declare_target_nohost_variant_messages.cpp


Index: clang/test/OpenMP/declare_target_nohost_variant_messages.cpp
===
--- clang/test/OpenMP/declare_target_nohost_variant_messages.cpp
+++ clang/test/OpenMP/declare_target_nohost_variant_messages.cpp
@@ -1,21 +1,31 @@
+// REQUIRES: amdgpu-registered-target
+
 // RUN: %clang_cc1 -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa 
-fopenmp-version=52 -DVERBOSE_MODE=1 -verify=omp52 -fnoopenmp-use-tls 
-ferror-limit 100 -fopenmp-targets=amdgcn-amd-amdhsa -o - %s
 
 void fun();
+void host_function();
+#pragma omp declare target enter(fun) device_type(nohost)
+#pragma omp declare variant(host_function) match(device={kind(host)})
+void fun() {}
+void host_function() {}
+void call_host_function() { fun(); }
+
+void fun1();
 void not_a_host_function();
-#pragma omp declare target enter(fun) device_type(nohost) // omp52-note 
{{marked as 'device_type(nohost)' here}}
+#pragma omp declare target enter(fun1) device_type(nohost) // omp52-note 
{{marked as 'device_type(nohost)' here}}
 #pragma omp declare variant(not_a_host_function) match(device={kind(host)}) // 
omp52-error {{function with 'device_type(nohost)' is not available on host}}
-void fun() {}
+void fun1() {}
 #pragma omp begin declare target device_type(nohost) // omp52-note {{marked as 
'device_type(nohost)' here}}
 void not_a_host_function() {}
 #pragma omp end declare target
-void failed_call_to_host_function() { fun(); } // omp52-error {{function with 
'device_type(nohost)' is not available on host}}
+void failed_call_to_host_function() { fun1(); } // omp52-error {{function with 
'device_type(nohost)' is not available on host}}
 
 void fun2();
-void host_function();
+void host_function2();
 #pragma omp declare target enter(fun2) device_type(nohost)
-#pragma omp declare variant(host_function) match(device={kind(host)})
+#pragma omp declare variant(host_function2) match(device={kind(host)})
 void fun2() {}
 #pragma omp begin declare target device_type(host)
-void host_function() {}
+void host_function2() {}
 #pragma omp end declare target
 void call_to_host_function() { fun2(); }
Index: clang/test/OpenMP/declare_target_messages.cpp
===
--- clang/test/OpenMP/declare_target_messages.cpp
+++ clang/test/OpenMP/declare_target_messages.cpp
@@ -11,7 +11,7 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp-version=51 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp-version=51 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa 
-fopenmp-version=52 -DVERBOSE_MODE=1 -verify=expected,omp52 -fnoopenmp-use-tls 
-ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp52 
-fopenmp -fopenmp-version=52 -DVERBOSE_MODE=1 -fnoopenmp-use-tls -ferror-limit 
100 -o - %s
 
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5 
-fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 #pragma omp end declare target // expected-error {{unexpected OpenMP directive 
'#pragma omp end declare target'}}
@@ -242,11 +242,3 @@
 // expected-warning@+1 {{expected '#pragma omp end declare target' at end of 
file to match '#pragma omp begin declare target'}}
 #pragma omp begin declare target
 #endif
-
-void fun();
-void host_function();
-#pragma omp declare target enter(fun) device_type(nohost) // omp45-error 
{{unexpected 'enter' clause, use 'to' instead}} omp45-error {{expected at least 
one 'to' or 'link' clause}} omp5-error {{unexpected 'enter' clause, use 'to' 
instead}} omp5-error {{expected at least one 'to' or 'link' clause}} 
omp51-error {{expected at least one 'to', 'link' or 'indirect' clause}} 
omp51-error {{unexpected 'enter' clause, use 'to' instead}}
-#pragma omp declare variant(host_function) match(device={kind(host)})
-void fun() {}
-void host_function() {}
-void call_host_function() { fun(); }


Index: clang/test/OpenMP/declare_target_nohost_variant_messages.cpp
===
--- clang/test/OpenMP/declare_target_nohost_variant_messages.cpp
+++ clang/test/Ope

[PATCH] D140217: [lit] Script to automate use of %(line-n). Use in CodeComplete tests.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

looks like you forgot to upload the new diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140217

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

[PATCH] D140217: [lit] Script to automate use of %(line-n). Use in CodeComplete tests.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D140217#4004657 , @hokein wrote:

> looks like you forgot to upload the new diff.

The changes were trivial/mechanical so I wasn't planning on another round of 
review, but was waiting for the tests to finish before landing.
Sorry for confusion, happy to address any comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140217

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

[PATCH] D140294: clang: Replace implementation of __builtin_isnormal

[Jay Foad via Phabricator via cfe-commits]

foad added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3308
   case Builtin::BI__builtin_isnormal: {
-// isnormal(x) --> x == x && fabsf(x) < infinity && fabsf(x) >= float_min
+// isnormal(x) --> fabs(x) < infinity && !(fabs(x) < float_min)
 CodeGenFunction::CGFPOptionsRAII FPOptsRAII(*this, E);

Why not make both compares ordered and write this as just `fabs(x) < infinity 
&& fabs(x) >= float_min`? That seems conceptually simpler - i.e. it makes the 
comment easier to understand and the IR no worse.


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

https://reviews.llvm.org/D140294

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

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 483926.
VitaNuo marked 7 inline comments as done.
VitaNuo added a comment.

Address most recent review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

Files:
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -7,11 +7,17 @@
 //===--===//
 
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -31,7 +37,7 @@
   std::unique_ptr AST;
   FindHeadersTest() {
 Inputs.MakeAction = [this] {
-  struct Hook : public PreprocessOnlyAction {
+  struct Hook : public SyntaxOnlyAction {
   public:
 Hook(PragmaIncludes *Out) : Out(Out) {}
 bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
@@ -153,5 +159,61 @@
physicalHeader("exporter.h")));
 }
 
+TEST_F(FindHeadersTest, TargetIsExpandedFromMacroInHeader) {
+  struct CustomVisitor : RecursiveASTVisitor {
+const Decl *Out = nullptr;
+bool VisitNamedDecl(const NamedDecl *ND) {
+  if (ND->getName() == "FLAG_foo" || ND->getName() == "Foo") {
+EXPECT_TRUE(Out == nullptr);
+Out = ND;
+  }
+  return true;
+}
+  };
+
+  struct {
+llvm::StringRef DeclareHeader;
+llvm::StringRef MacroHeader;
+  } TestCases[] = {{
+   R"cpp(
+#include "macro.h"
+DEFINE_CLASS(Foo)
+  )cpp",
+   R"cpp(
+#define DEFINE_CLASS(name) class name {};
+  )cpp"},
+   {
+   R"cpp(
+#include "macro.h"
+DEFINE_Foo
+  )cpp",
+   R"cpp(
+#define DEFINE_Foo class Foo {};
+  )cpp"},
+   {
+   R"cpp(
+#include "macro.h"
+DECLARE_FLAGS(foo);
+  )cpp",
+   R"cpp(
+#define DECLARE_FLAGS(name) extern int FLAG_##name
+  )cpp"}};
+
+  for (const auto &T : TestCases) {
+Inputs.Code = R"cpp(#include "declare.h")cpp";
+Inputs.ExtraFiles["declare.h"] = guard(T.DeclareHeader);
+Inputs.ExtraFiles["macro.h"] = guard(T.MacroHeader);
+buildAST();
+
+CustomVisitor Visitor;
+Visitor.TraverseDecl(AST->context().getTranslationUnitDecl());
+
+const Decl *Foo = Visitor.Out;
+llvm::SmallVector Headers = clang::include_cleaner::findHeaders(
+Foo->getLocation(), AST->sourceManager(), nullptr);
+EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));
+  }
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
===
--- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -18,8 +18,7 @@
   llvm::SmallVector Results;
   switch (Loc.kind()) {
   case SymbolLocation::Physical: {
-// FIXME: Handle macro locations.
-FileID FID = SM.getFileID(Loc.physical());
+FileID FID = SM.getFileID(SM.getExpansionLoc(Loc.physical()));
 const FileEntry *FE = SM.getFileEntryForID(FID);
 if (!PI) {
   return FE ? llvm::SmallVector{Header(FE)}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Thanks for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

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

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 483927.
VitaNuo added a comment.

Remove extra variable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

Files:
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -7,11 +7,17 @@
 //===--===//
 
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -31,7 +37,7 @@
   std::unique_ptr AST;
   FindHeadersTest() {
 Inputs.MakeAction = [this] {
-  struct Hook : public PreprocessOnlyAction {
+  struct Hook : public SyntaxOnlyAction {
   public:
 Hook(PragmaIncludes *Out) : Out(Out) {}
 bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
@@ -153,5 +159,60 @@
physicalHeader("exporter.h")));
 }
 
+TEST_F(FindHeadersTest, TargetIsExpandedFromMacroInHeader) {
+  struct CustomVisitor : RecursiveASTVisitor {
+const Decl *Out = nullptr;
+bool VisitNamedDecl(const NamedDecl *ND) {
+  if (ND->getName() == "FLAG_foo" || ND->getName() == "Foo") {
+EXPECT_TRUE(Out == nullptr);
+Out = ND;
+  }
+  return true;
+}
+  };
+
+  struct {
+llvm::StringRef DeclareHeader;
+llvm::StringRef MacroHeader;
+  } TestCases[] = {{
+   R"cpp(
+#include "macro.h"
+DEFINE_CLASS(Foo)
+  )cpp",
+   R"cpp(
+#define DEFINE_CLASS(name) class name {};
+  )cpp"},
+   {
+   R"cpp(
+#include "macro.h"
+DEFINE_Foo
+  )cpp",
+   R"cpp(
+#define DEFINE_Foo class Foo {};
+  )cpp"},
+   {
+   R"cpp(
+#include "macro.h"
+DECLARE_FLAGS(foo);
+  )cpp",
+   R"cpp(
+#define DECLARE_FLAGS(name) extern int FLAG_##name
+  )cpp"}};
+
+  for (const auto &T : TestCases) {
+Inputs.Code = R"cpp(#include "declare.h")cpp";
+Inputs.ExtraFiles["declare.h"] = guard(T.DeclareHeader);
+Inputs.ExtraFiles["macro.h"] = guard(T.MacroHeader);
+buildAST();
+
+CustomVisitor Visitor;
+Visitor.TraverseDecl(AST->context().getTranslationUnitDecl());
+
+llvm::SmallVector Headers = clang::include_cleaner::findHeaders(
+Visitor.Out->getLocation(), AST->sourceManager(), nullptr);
+EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));
+  }
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
===
--- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -18,8 +18,7 @@
   llvm::SmallVector Results;
   switch (Loc.kind()) {
   case SymbolLocation::Physical: {
-// FIXME: Handle macro locations.
-FileID FID = SM.getFileID(Loc.physical());
+FileID FID = SM.getFileID(SM.getExpansionLoc(Loc.physical()));
 const FileEntry *FE = SM.getFileEntryForID(FID);
 if (!PI) {
   return FE ? llvm::SmallVector{Header(FE)}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140281: [X86] Rename CMPCCXADD intrinsics.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

This patch says what it does, but not why it does what it does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140281

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

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:219
+  /// @param State The state of the generated node.
+  /// @param Pred The transition will be generated from the specified Pred node
+  /// to the newly generated node.

What I'm missing here is some guidance. Why would I pick this overload instead 
of the 2-parameter one? Especially for beginners, this is very confusing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137722

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

[clang] b5c809a - Fix tests for commit 658ed9547cdd6657895339a6c390c31aa77a5698.

[Doru Bercea via cfe-commits]

Author: Doru Bercea
Date: 2022-12-19T07:46:34-06:00
New Revision: b5c809acd34c2489679300eb0b8a8b824aeb

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

LOG: Fix tests for commit 658ed9547cdd6657895339a6c390c31aa77a5698.

Added: 


Modified: 
clang/test/OpenMP/declare_target_messages.cpp
clang/test/OpenMP/declare_target_nohost_variant_messages.cpp

Removed: 




diff  --git a/clang/test/OpenMP/declare_target_messages.cpp 
b/clang/test/OpenMP/declare_target_messages.cpp
index bf23813999119..ed011a8c3a593 100644
--- a/clang/test/OpenMP/declare_target_messages.cpp
+++ b/clang/test/OpenMP/declare_target_messages.cpp
@@ -11,7 +11,7 @@
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp-version=51 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp51 
-fopenmp-version=51 -fopenmp-simd -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 
-// RUN: %clang_cc1 -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa 
-fopenmp-version=52 -DVERBOSE_MODE=1 -verify=expected,omp52 -fnoopenmp-use-tls 
-ferror-limit 100 -o - %s
+// RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp52 
-fopenmp -fopenmp-version=52 -DVERBOSE_MODE=1 -fnoopenmp-use-tls -ferror-limit 
100 -o - %s
 
 // RUN: %clang_cc1 -triple x86_64-apple-macos10.7.0 -verify=expected,omp5 
-fopenmp -fnoopenmp-use-tls -ferror-limit 100 -o - %s
 #pragma omp end declare target // expected-error {{unexpected OpenMP directive 
'#pragma omp end declare target'}}
@@ -242,11 +242,3 @@ int MultiDevTy;
 // expected-warning@+1 {{expected '#pragma omp end declare target' at end of 
file to match '#pragma omp begin declare target'}}
 #pragma omp begin declare target
 #endif
-
-void fun();
-void host_function();
-#pragma omp declare target enter(fun) device_type(nohost) // omp45-error 
{{unexpected 'enter' clause, use 'to' instead}} omp45-error {{expected at least 
one 'to' or 'link' clause}} omp5-error {{unexpected 'enter' clause, use 'to' 
instead}} omp5-error {{expected at least one 'to' or 'link' clause}} 
omp51-error {{expected at least one 'to', 'link' or 'indirect' clause}} 
omp51-error {{unexpected 'enter' clause, use 'to' instead}}
-#pragma omp declare variant(host_function) match(device={kind(host)})
-void fun() {}
-void host_function() {}
-void call_host_function() { fun(); }

diff  --git a/clang/test/OpenMP/declare_target_nohost_variant_messages.cpp 
b/clang/test/OpenMP/declare_target_nohost_variant_messages.cpp
index b54f864a926b2..190c1387cb099 100644
--- a/clang/test/OpenMP/declare_target_nohost_variant_messages.cpp
+++ b/clang/test/OpenMP/declare_target_nohost_variant_messages.cpp
@@ -1,21 +1,31 @@
+// REQUIRES: amdgpu-registered-target
+
 // RUN: %clang_cc1 -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa 
-fopenmp-version=52 -DVERBOSE_MODE=1 -verify=omp52 -fnoopenmp-use-tls 
-ferror-limit 100 -fopenmp-targets=amdgcn-amd-amdhsa -o - %s
 
 void fun();
+void host_function();
+#pragma omp declare target enter(fun) device_type(nohost)
+#pragma omp declare variant(host_function) match(device={kind(host)})
+void fun() {}
+void host_function() {}
+void call_host_function() { fun(); }
+
+void fun1();
 void not_a_host_function();
-#pragma omp declare target enter(fun) device_type(nohost) // omp52-note 
{{marked as 'device_type(nohost)' here}}
+#pragma omp declare target enter(fun1) device_type(nohost) // omp52-note 
{{marked as 'device_type(nohost)' here}}
 #pragma omp declare variant(not_a_host_function) match(device={kind(host)}) // 
omp52-error {{function with 'device_type(nohost)' is not available on host}}
-void fun() {}
+void fun1() {}
 #pragma omp begin declare target device_type(nohost) // omp52-note {{marked as 
'device_type(nohost)' here}}
 void not_a_host_function() {}
 #pragma omp end declare target
-void failed_call_to_host_function() { fun(); } // omp52-error {{function with 
'device_type(nohost)' is not available on host}}
+void failed_call_to_host_function() { fun1(); } // omp52-error {{function with 
'device_type(nohost)' is not available on host}}
 
 void fun2();
-void host_function();
+void host_function2();
 #pragma omp declare target enter(fun2) device_type(nohost)
-#pragma omp declare variant(host_function) match(device={kind(host)})
+#pragma omp declare variant(host_function2) match(device={kind(host)})
 void fun2() {}
 #pragma omp begin declare target device_type(host)
-void host_function() {}
+void host_function2() {}
 #pragma omp end declare target
 void call_to_host_function() { fun2(); }



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

[PATCH] D140295: [Fix][OpenMP] Fix commit for nohost variant.

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 closed this revision.
doru1004 added a comment.

Commit b5c809acd34c2489679300eb0b8a8b824aeb 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140295

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

Re: [patch] libcxx win32 support

[admin admin via cfe-commits]

Sent from Mail for Windows

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

Re: [clang-tools-extra] r189354 - cpp11-migrate: Add a class to support include directives modifications

[admin admin via cfe-commits]

Sent from Mail for Windows

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

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks!




Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:176
+llvm::StringRef DeclareHeader;
+llvm::StringRef MacroHeader;
+  } TestCases[] = {{

nit: it feels more natural to declare `MacroHeader` first then `DeclareHeader`, 
as the `DeclareHeader` already includes the macro.h



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:177
+llvm::StringRef MacroHeader;
+  } TestCases[] = {{
+   R"cpp(

The clang-format's indentation seems odd to me, I think if you put a trailing 
`,` on the last element `}, };`, its format is better.





Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:178
+  } TestCases[] = {{
+   R"cpp(
+#include "macro.h"

nit: I'd add `/*DeclareHeader=*/`, `/*MacroHeader=*/`, which improves the code 
readability.



Comment at: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp:212
+llvm::SmallVector Headers = clang::include_cleaner::findHeaders(
+Visitor.Out->getLocation(), AST->sourceManager(), nullptr);
+EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));

nit: /*PragmaIncludes=*/nullptr


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

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

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D137790#3992216 , @balazske wrote:

> On the postgres results, the second is one that can be fixed in the checker 
> (add special cases to `StdLibraryFunctionsChecker` for zero `len` or `size` 
> `fread` and `fwrite` arguments). The others are false positives because the 
> error path is impossible because implicit constraints (what is not known to 
> the analyzer) on variables.

I'd want some more thorough explanations as well. Path events are numbered in 
CodeChecker, you can use them to explain why you think the report is a false 
positive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137790

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

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Here is a postgers result link 
,
 results are from **StdCLibraryFunctionArgs** checker. The second report is in 
file relcache.c (it looks like that `data` pointer is really null but the `len` 
value is 0, and in this special case the data pointer should be ignored 
according to the standards). A FIXME for this case was added to the code in 
D135247 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137790

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

[PATCH] D140300: [lit] Fix a few issues in relative_lines.py

[Haojian Wu via Phabricator via cfe-commits]

hokein created this revision.
hokein added a reviewer: sammccall.
Herald added a project: All.
hokein requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

- fixes error: Line: 91  result += contents[pos:m.start(index)] TypeError: can 
only concatenate str (not "bytes") to str
- fixes the "-code-completion-at=%s:%(lineb'-7')" rewritten results


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140300

Files:
  llvm/utils/relative_lines.py


Index: llvm/utils/relative_lines.py
===
--- llvm/utils/relative_lines.py
+++ llvm/utils/relative_lines.py
@@ -68,23 +68,23 @@
 print(f"{file}:{line}: target line {target} is farther than 
{args.near}", file=sys.stderr)
 return capture
 if target > line:
-delta = b('+' + str(target - line))
+delta = '+' + str(target - line)
 elif target < line:
-delta = b('-' + str(line - target))
+delta = '-' + str(line - target)
 else:
-delta = b''
+delta = ''
 
 prefix = contents[:offset].rsplit(b'\n')[-1]
 is_lit = b'RUN' in prefix or b'DEFINE' in prefix
-text = b(('%(line{0})' if is_lit else '[[@LINE{0}]]').format(delta))
+text = ('%(line{0})' if is_lit else '[[@LINE{0}]]').format(delta)
 if args.verbose:
 print(f"{file}:{line}: {0} ==> {text}")
-return text
+return b(text)
 
 def replace_match(m):
 """Text to replace a whole match, e.g. --at=42:3 => --at=%(line+2):3"""
 line = 1 + contents[:m.start()].count(b'\n')
-result = ''
+result = b''
 pos = m.start()
 for index, capture in enumerate(m.groups()):
 index += 1 # re groups are conventionally 1-indexed


Index: llvm/utils/relative_lines.py
===
--- llvm/utils/relative_lines.py
+++ llvm/utils/relative_lines.py
@@ -68,23 +68,23 @@
 print(f"{file}:{line}: target line {target} is farther than {args.near}", file=sys.stderr)
 return capture
 if target > line:
-delta = b('+' + str(target - line))
+delta = '+' + str(target - line)
 elif target < line:
-delta = b('-' + str(line - target))
+delta = '-' + str(line - target)
 else:
-delta = b''
+delta = ''
 
 prefix = contents[:offset].rsplit(b'\n')[-1]
 is_lit = b'RUN' in prefix or b'DEFINE' in prefix
-text = b(('%(line{0})' if is_lit else '[[@LINE{0}]]').format(delta))
+text = ('%(line{0})' if is_lit else '[[@LINE{0}]]').format(delta)
 if args.verbose:
 print(f"{file}:{line}: {0} ==> {text}")
-return text
+return b(text)
 
 def replace_match(m):
 """Text to replace a whole match, e.g. --at=42:3 => --at=%(line+2):3"""
 line = 1 + contents[:m.start()].count(b'\n')
-result = ''
+result = b''
 pos = m.start()
 for index, capture in enumerate(m.groups()):
 index += 1 # re groups are conventionally 1-indexed
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo updated this revision to Diff 483941.
VitaNuo marked 4 inline comments as done.
VitaNuo added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

Files:
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -7,11 +7,17 @@
 //===--===//
 
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -31,7 +37,7 @@
   std::unique_ptr AST;
   FindHeadersTest() {
 Inputs.MakeAction = [this] {
-  struct Hook : public PreprocessOnlyAction {
+  struct Hook : public SyntaxOnlyAction {
   public:
 Hook(PragmaIncludes *Out) : Out(Out) {}
 bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
@@ -153,5 +159,60 @@
physicalHeader("exporter.h")));
 }
 
+TEST_F(FindHeadersTest, TargetIsExpandedFromMacroInHeader) {
+  struct CustomVisitor : RecursiveASTVisitor {
+const Decl *Out = nullptr;
+bool VisitNamedDecl(const NamedDecl *ND) {
+  if (ND->getName() == "FLAG_foo" || ND->getName() == "Foo") {
+EXPECT_TRUE(Out == nullptr);
+Out = ND;
+  }
+  return true;
+}
+  };
+
+  struct {
+llvm::StringRef MacroHeader;
+llvm::StringRef DeclareHeader;
+  } TestCases[] = {
+  {/*MacroHeader=*/R"cpp(
+#define DEFINE_CLASS(name) class name {};
+  )cpp",
+   /*DeclareHeader=*/R"cpp(
+#include "macro.h"
+DEFINE_CLASS(Foo)
+  )cpp"},
+  {/*MacroHeader=*/R"cpp(
+#define DEFINE_Foo class Foo {};
+  )cpp",
+   /*DeclareHeader=*/R"cpp(
+#include "macro.h"
+DEFINE_Foo
+  )cpp"},
+  {/*MacroHeader=*/R"cpp(
+#define DECLARE_FLAGS(name) extern int FLAG_##name
+  )cpp",
+   /*DeclareHeader=*/R"cpp(
+#include "macro.h"
+DECLARE_FLAGS(foo);
+  )cpp"},
+  };
+
+  for (const auto &T : TestCases) {
+Inputs.Code = R"cpp(#include "declare.h")cpp";
+Inputs.ExtraFiles["declare.h"] = guard(T.DeclareHeader);
+Inputs.ExtraFiles["macro.h"] = guard(T.MacroHeader);
+buildAST();
+
+CustomVisitor Visitor;
+Visitor.TraverseDecl(AST->context().getTranslationUnitDecl());
+
+llvm::SmallVector Headers = clang::include_cleaner::findHeaders(
+Visitor.Out->getLocation(), AST->sourceManager(),
+/*PragmaIncludes=*/nullptr);
+EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));
+  }
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
===
--- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -18,8 +18,7 @@
   llvm::SmallVector Results;
   switch (Loc.kind()) {
   case SymbolLocation::Physical: {
-// FIXME: Handle macro locations.
-FileID FID = SM.getFileID(Loc.physical());
+FileID FID = SM.getFileID(SM.getExpansionLoc(Loc.physical()));
 const FileEntry *FE = SM.getFileEntryForID(FID);
 if (!PI) {
   return FE ? llvm::SmallVector{Header(FE)}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

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

[clang-tools-extra] 81c3739 - [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via cfe-commits]

Author: Viktoriia Bakalova
Date: 2022-12-19T14:18:16Z
New Revision: 81c3739156fe2e98160fb4364ed78edacc293a68

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

LOG: [include-cleaner] Use expansion locations for macros.

Use expansion locations for target symbol decls when finding headers for macros.
Fix: https://github.com/llvm/llvm-project/issues/59392
Differential Revision: https://reviews.llvm.org/D139716

Added: 


Modified: 
clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Removed: 




diff  --git a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp 
b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
index b9e6e29a00667..8f38c95375fc3 100644
--- a/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ b/clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -18,8 +18,7 @@ llvm::SmallVector findHeaders(const SymbolLocation 
&Loc,
   llvm::SmallVector Results;
   switch (Loc.kind()) {
   case SymbolLocation::Physical: {
-// FIXME: Handle macro locations.
-FileID FID = SM.getFileID(Loc.physical());
+FileID FID = SM.getFileID(SM.getExpansionLoc(Loc.physical()));
 const FileEntry *FE = SM.getFileEntryForID(FID);
 if (!PI) {
   return FE ? llvm::SmallVector{Header(FE)}

diff  --git a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
index d1a21ab685346..ad5961699834c 100644
--- a/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -7,11 +7,17 @@
 
//===--===//
 
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -31,7 +37,7 @@ class FindHeadersTest : public testing::Test {
   std::unique_ptr AST;
   FindHeadersTest() {
 Inputs.MakeAction = [this] {
-  struct Hook : public PreprocessOnlyAction {
+  struct Hook : public SyntaxOnlyAction {
   public:
 Hook(PragmaIncludes *Out) : Out(Out) {}
 bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
@@ -153,5 +159,60 @@ TEST_F(FindHeadersTest, NonSelfContainedTraverseExporter) {
physicalHeader("exporter.h")));
 }
 
+TEST_F(FindHeadersTest, TargetIsExpandedFromMacroInHeader) {
+  struct CustomVisitor : RecursiveASTVisitor {
+const Decl *Out = nullptr;
+bool VisitNamedDecl(const NamedDecl *ND) {
+  if (ND->getName() == "FLAG_foo" || ND->getName() == "Foo") {
+EXPECT_TRUE(Out == nullptr);
+Out = ND;
+  }
+  return true;
+}
+  };
+
+  struct {
+llvm::StringRef MacroHeader;
+llvm::StringRef DeclareHeader;
+  } TestCases[] = {
+  {/*MacroHeader=*/R"cpp(
+#define DEFINE_CLASS(name) class name {};
+  )cpp",
+   /*DeclareHeader=*/R"cpp(
+#include "macro.h"
+DEFINE_CLASS(Foo)
+  )cpp"},
+  {/*MacroHeader=*/R"cpp(
+#define DEFINE_Foo class Foo {};
+  )cpp",
+   /*DeclareHeader=*/R"cpp(
+#include "macro.h"
+DEFINE_Foo
+  )cpp"},
+  {/*MacroHeader=*/R"cpp(
+#define DECLARE_FLAGS(name) extern int FLAG_##name
+  )cpp",
+   /*DeclareHeader=*/R"cpp(
+#include "macro.h"
+DECLARE_FLAGS(foo);
+  )cpp"},
+  };
+
+  for (const auto &T : TestCases) {
+Inputs.Code = R"cpp(#include "declare.h")cpp";
+Inputs.ExtraFiles["declare.h"] = guard(T.DeclareHeader);
+Inputs.ExtraFiles["macro.h"] = guard(T.MacroHeader);
+buildAST();
+
+CustomVisitor Visitor;
+Visitor.TraverseDecl(AST->context().getTranslationUnitDecl());
+
+llvm::SmallVector Headers = clang::include_cleaner::findHeaders(
+Visitor.Out->getLocation(), AST->sourceManager(),
+/*PragmaIncludes=*/nullptr);
+EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));
+  }
+}
+
 } // namespace
 } // namespace clang::include_cleaner



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

[PATCH] D139716: [include-cleaner] Use expansion locations for macros.

[Viktoriia Bakalova via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG81c3739156fe: [include-cleaner] Use expansion locations for 
macros. (authored by VitaNuo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139716

Files:
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -7,11 +7,17 @@
 //===--===//
 
 #include "AnalysisInternal.h"
+#include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/FileEntry.h"
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Annotations.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -31,7 +37,7 @@
   std::unique_ptr AST;
   FindHeadersTest() {
 Inputs.MakeAction = [this] {
-  struct Hook : public PreprocessOnlyAction {
+  struct Hook : public SyntaxOnlyAction {
   public:
 Hook(PragmaIncludes *Out) : Out(Out) {}
 bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
@@ -153,5 +159,60 @@
physicalHeader("exporter.h")));
 }
 
+TEST_F(FindHeadersTest, TargetIsExpandedFromMacroInHeader) {
+  struct CustomVisitor : RecursiveASTVisitor {
+const Decl *Out = nullptr;
+bool VisitNamedDecl(const NamedDecl *ND) {
+  if (ND->getName() == "FLAG_foo" || ND->getName() == "Foo") {
+EXPECT_TRUE(Out == nullptr);
+Out = ND;
+  }
+  return true;
+}
+  };
+
+  struct {
+llvm::StringRef MacroHeader;
+llvm::StringRef DeclareHeader;
+  } TestCases[] = {
+  {/*MacroHeader=*/R"cpp(
+#define DEFINE_CLASS(name) class name {};
+  )cpp",
+   /*DeclareHeader=*/R"cpp(
+#include "macro.h"
+DEFINE_CLASS(Foo)
+  )cpp"},
+  {/*MacroHeader=*/R"cpp(
+#define DEFINE_Foo class Foo {};
+  )cpp",
+   /*DeclareHeader=*/R"cpp(
+#include "macro.h"
+DEFINE_Foo
+  )cpp"},
+  {/*MacroHeader=*/R"cpp(
+#define DECLARE_FLAGS(name) extern int FLAG_##name
+  )cpp",
+   /*DeclareHeader=*/R"cpp(
+#include "macro.h"
+DECLARE_FLAGS(foo);
+  )cpp"},
+  };
+
+  for (const auto &T : TestCases) {
+Inputs.Code = R"cpp(#include "declare.h")cpp";
+Inputs.ExtraFiles["declare.h"] = guard(T.DeclareHeader);
+Inputs.ExtraFiles["macro.h"] = guard(T.MacroHeader);
+buildAST();
+
+CustomVisitor Visitor;
+Visitor.TraverseDecl(AST->context().getTranslationUnitDecl());
+
+llvm::SmallVector Headers = clang::include_cleaner::findHeaders(
+Visitor.Out->getLocation(), AST->sourceManager(),
+/*PragmaIncludes=*/nullptr);
+EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));
+  }
+}
+
 } // namespace
 } // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
===
--- clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
+++ clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
@@ -18,8 +18,7 @@
   llvm::SmallVector Results;
   switch (Loc.kind()) {
   case SymbolLocation::Physical: {
-// FIXME: Handle macro locations.
-FileID FID = SM.getFileID(Loc.physical());
+FileID FID = SM.getFileID(SM.getExpansionLoc(Loc.physical()));
 const FileEntry *FE = SM.getFileEntryForID(FID);
 if (!PI) {
   return FE ? llvm::SmallVector{Header(FE)}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

About the first postgres result in pg_backup_tar.c:
At step 3 we see that original value of `len` is 1. The condition in step 5 is 
true, then `th->filelen-th->pos` should be < 1. This is the new value assigned 
to `len`. In step 6 `len` can be only <=0. Still the condition is assumed to be 
false what is not possible. At this point the path becomes already invalid.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137790

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

[PATCH] D137790: [clang][analyzer] Remove report of null stream from StreamChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

I have a fear that we may have too few results as well -- maybe we should 
expand our testing infrastructure with more POSIX API heavy codebases. Looking 
at a few new projects, https://github.com/audacity/audacity looks like a good 
candidate, but of course it often turns out that adding a new project to the 
benchmark is more troublesome than it appears...

In D137790#4004803 , @balazske wrote:

> Here is a postgers result link 
> ,
>  results are from **StdCLibraryFunctionArgs** checker. The second report is 
> in file relcache.c (it looks like that `data` pointer is really null but the 
> `len` value is 0, and in this special case the data pointer should be ignored 
> according to the standards). A FIXME for this case was added to the code in 
> D135247 .

Could you please re-evaluate the patch stack with the new fix?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137790

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

[PATCH] D139629: clang: Stop emitting "strictfp"

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2135
-llvm::AttrBuilder FuncAttrs(F->getContext());
-FuncAttrs.addAttribute("strictfp");
-F->addFnAttrs(FuncAttrs);

kpn wrote:
> arsenm wrote:
> > andrew.w.kaylor wrote:
> > > arsenm wrote:
> > > > zahiraam wrote:
> > > > > I think it would better to fix this function instead of removing it 
> > > > > entirely? The issue here is that there is the "strictfp" attribute 
> > > > > and the llvm::Attribute::StrictFP. We could replace  
> > > > > FuncAttrs.addAttribute("strictfp"); with
> > > > >  FuncAttrs.addAttribute(llvm::Attribute::StrictFP); 
> > > > > This function ensures that the function attribute is set when the 
> > > > > FunctionDecl attribute is set. I am concerned that when it's removed, 
> > > > > we will wind up with cases where the function attribute is missing! 
> > > > > The only place where this function attribute is in 
> > > > > CodeGenFunction::StartFunction. Is that enough? @andrew.w.kaylor Can 
> > > > > you please weigh in on this?
> > > > I currently don't have evidence that making this use the correct 
> > > > attribute would fix anything. If something was depending on this 
> > > > emission in this context, it's already broken
> > > It may be that anything depending on this is already broken, but the code 
> > > was written for a reason, even if it was always broken. I'm not sure I 
> > > understand what that reason was, and unfortunately the person who wrote 
> > > the code (@mibintc) is no longer actively contributing to LLVM. It was 
> > > added here: https://reviews.llvm.org/D87528
> > > 
> > > It does seem like the llvm::Attribute::StrictFP is being added any time 
> > > the string attribute is added, but they're coming from different places. 
> > > The proper attribute seems to be coming from 
> > > CodeGenFunction::StartFunction() which is effectively copying it from the 
> > > function declaration. It's not clear to me whether there are 
> > > circumstances where we get to setLLVMFunctionFEnvAttributes() through 
> > > EmitGlobalFunctionDefinition() without ever having gone through 
> > > CodeGenFunction::StartFunction(). It looks like maybe there are 
> > > multiversioning cases that do that, but I couldn't come up with an 
> > > example that does. @erichkeane wrote a lot of the multi-versioning code, 
> > > so he might know more, but he's on vacation through the end of the month.
> > > 
> > > Eliminating this extra string attribute seems obviously good. In this 
> > > particular location, though, I'd be inclined to set the enumerated 
> > > attribute here, even though that might be redundant in most cases. On the 
> > > other hand, if one of our front end experts can look at this code and say 
> > > definitively that it's //always// redundant, I'd be fine with this code 
> > > being deleted.
> > I think code that can be deleted that doesn't manifest in test failures 
> > should be immediately deleted. We shouldn't leave things around just in 
> > case 
> The Verifier changes that would detect the error and fail tests never made it 
> into the tree. The lack of failures therefore tells us nothing in this case 
> here.
The verifier never would have checked the string form


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

https://reviews.llvm.org/D139629

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

[PATCH] D140300: [lit] Fix a few issues in relative_lines.py

[Sam McCall via Phabricator via cfe-commits]

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

thank you!
I guess I tested this only on files with no matches/bad utf-8 :-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140300

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

[PATCH] D140306: [clang][dataflow] Remove stray lines from `Environment::join`

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added a reviewer: gribozavr2.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

Removes an assertion and a useless line. The assertion seems left over from
earlier debugging and the line that follows is a stray line.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140306

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


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -444,10 +444,6 @@
 
   JoinedEnv.MemberLocToStruct =
   intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
-  assert(JoinedEnv.MemberLocToStruct.size() <=
- std::min(MemberLocToStruct.size(), Other.MemberLocToStruct.size()));
-
-  intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
   if (MemberLocToStruct.size() != JoinedEnv.MemberLocToStruct.size())
 Effect = LatticeJoinEffect::Changed;
 


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -444,10 +444,6 @@
 
   JoinedEnv.MemberLocToStruct =
   intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
-  assert(JoinedEnv.MemberLocToStruct.size() <=
- std::min(MemberLocToStruct.size(), Other.MemberLocToStruct.size()));
-
-  intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
   if (MemberLocToStruct.size() != JoinedEnv.MemberLocToStruct.size())
 Effect = LatticeJoinEffect::Changed;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D138253: [-Wunsafe-buffer-usage] NFC: Implement fix-strategies and variable-use-claiming.

[Mikael Holmén via Phabricator via cfe-commits]

uabelho added a comment.

Hello,

I noticed that the following crashes with this patch:

  clang -Weverything bbi-77071.c

Result:

  clang-16: ../../clang/lib/Analysis/UnsafeBufferUsage.cpp:248: void (anonymous 
namespace)::DeclUseTracker::discoverDecl(const clang::DeclStmt *): Assertion 
`Defs.count(VD) == 0 && "Definition already discovered!"' failed.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: 
/repo/uabelho/main-github/llvm/build-all/bin/clang-16 -cc1 -triple 
x86_64-unknown-linux-gnu -emit-obj -mrelax-all --mrelax-relocations 
-disable-free -clear-ast-before-backend -main-file-name bbi-77071.c 
-mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=all 
-fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases 
-funwind-tables=2 -target-cpu x86-64 -tune-cpu generic -mllvm 
-treat-scalable-fixed-error-as-warning -debugger-tuning=gdb 
-fcoverage-compilation-dir=/repo/uabelho/llvm-dev2/llvm -resource-dir 
/repo/uabelho/main-github/llvm/build-all/lib/clang/16 -internal-isystem 
/repo/uabelho/main-github/llvm/build-all/lib/clang/16/include -internal-isystem 
/usr/local/include -internal-isystem 
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/../../../../x86_64-redhat-linux/include 
-internal-externc-isystem /include -internal-externc-isystem /usr/include 
-Weverything -fdebug-compilation-dir=/repo/uabelho/llvm-dev2/llvm -ferror-limit 
19 -fgnuc-version=4.2.1 -fcolor-diagnostics -faddrsig 
-D__GCC_HAVE_DWARF2_CFI_ASM=1 -o /tmp/bbi-77071-6173ad.o -x c bbi-77071.c
  1. parser at end of file
  2.bbi-77071.c:5:9: parsing function body 'fn1'
   #0 0x02fdf843 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x2fdf843)
   #1 0x02fdd56e llvm::sys::RunSignalHandlers() 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x2fdd56e)
   #2 0x02fdfbc6 SignalHandler(int) Signals.cpp:0:0
   #3 0x7fcfa502a630 __restore_rt sigaction.c:0:0
   #4 0x7fcfa2771387 raise (/lib64/libc.so.6+0x36387)
   #5 0x7fcfa2772a78 abort (/lib64/libc.so.6+0x37a78)
   #6 0x7fcfa276a1a6 __assert_fail_base (/lib64/libc.so.6+0x2f1a6)
   #7 0x7fcfa276a252 (/lib64/libc.so.6+0x2f252)
   #8 0x05520daf findGadgets(clang::Decl 
const*)::GadgetFinderCallback::run(clang::ast_matchers::MatchFinder::MatchResult
 const&) UnsafeBufferUsage.cpp:0:0
   #9 0x0554c862 clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::MatchVisitor::visitMatch(clang::ast_matchers::BoundNodes
 const&) ASTMatchFinder.cpp:0:0
  #10 0x0557d27b 
clang::ast_matchers::internal::BoundNodesTreeBuilder::visitMatches(clang::ast_matchers::internal::BoundNodesTreeBuilder::Visitor*)
 (/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x557d27b)
  #11 0x0554be73 clang::ast_matchers::internal::(anonymous 
namespace)::MatchASTVisitor::matchWithFilter(clang::DynTypedNode const&) 
ASTMatchFinder.cpp:0:0
  #12 0x05524f9c 
clang::ast_matchers::MatchFinder::match(clang::DynTypedNode const&, 
clang::ASTContext&) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x5524f9c)
  #13 0x0551eb53 clang::checkUnsafeBufferUsage(clang::Decl const*, 
clang::UnsafeBufferUsageHandler&) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x551eb53)
  #14 0x053f48e5 
clang::sema::AnalysisBasedWarnings::IssueWarnings(clang::sema::AnalysisBasedWarnings::Policy,
 clang::sema::FunctionScopeInfo*, clang::Decl const*, clang::QualType) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x53f48e5)
  #15 0x04b91a53 
clang::Sema::PopFunctionScopeInfo(clang::sema::AnalysisBasedWarnings::Policy 
const*, clang::Decl const*, clang::QualType) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x4b91a53)
  #16 0x04d14f6f clang::Sema::ActOnFinishFunctionBody(clang::Decl*, 
clang::Stmt*, bool) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x4d14f6f)
  #17 0x04b29135 
clang::Parser::ParseFunctionStatementBody(clang::Decl*, 
clang::Parser::ParseScope&) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x4b29135)
  #18 0x04a50d95 
clang::Parser::ParseFunctionDefinition(clang::ParsingDeclarator&, 
clang::Parser::ParsedTemplateInfo const&, clang::Parser::LateParsedAttrList*) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x4a50d95)
  #19 0x04a6d637 clang::Parser::ParseDeclGroup(clang::ParsingDeclSpec&, 
clang::DeclaratorContext, clang::ParsedAttributes&, clang::SourceLocation*, 
clang::Parser::ForRangeInit*) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16+0x4a6d637)
  #20 0x04a4f9a7 
clang::Parser::ParseDeclOrFunctionDefInternal(clang::ParsedAttributes&, 
clang::ParsedAttributes&, clang::ParsingDeclSpec&, clang::AccessSpecifier) 
(/repo/uabelho/main-github/llvm/build-all/bin/clang-16

[PATCH] D137722: [clang][analyzer] No new nodes when bug is detected in StdLibraryFunctionsChecker.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:219
+  /// @param State The state of the generated node.
+  /// @param Pred The transition will be generated from the specified Pred node
+  /// to the newly generated node.

Szelethus wrote:
> What I'm missing here is some guidance. Why would I pick this overload 
> instead of the 2-parameter one? Especially for beginners, this is very 
> confusing.
The whole documentation contains not enough information, at least not for 
beginners. This overload works the same way as at `generateNonFatalErrorNode` 
(and this documentation is made similar as at that function). `addTransition` 
works similar too, there is a bit more documentation. Probably somebody who 
understands `addTransition` and the `Pred` parameter can understand 
`generateErrorNode` too. If the name would be `addErrorNode` the similarity 
would be even stronger.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137722

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

[clang-tools-extra] bf452f9 - [clang-tidy][NFC] Replace custom isStatic matcher with the existing isStaticStorageClass

[Carlos Galvez via cfe-commits]

Author: Carlos Galvez
Date: 2022-12-19T15:23:50Z
New Revision: bf452f9b347aa0e1f8f368da925b8e1ca27df0c5

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

LOG: [clang-tidy][NFC] Replace custom isStatic matcher with the existing 
isStaticStorageClass

Added: 


Modified: 
clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp 
b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
index b26632d10779..211bd3c73e12 100644
--- a/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UseAnonymousNamespaceCheck.cpp
@@ -16,11 +16,6 @@ namespace clang {
 namespace tidy {
 namespace misc {
 namespace {
-AST_POLYMORPHIC_MATCHER(isStatic, AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
-  VarDecl)) {
-  return Node.getStorageClass() == SC_Static;
-}
-
 AST_POLYMORPHIC_MATCHER_P(isInHeaderFile,
   AST_POLYMORPHIC_SUPPORTED_TYPES(FunctionDecl,
   VarDecl),
@@ -60,13 +55,13 @@ void UseAnonymousNamespaceCheck::storeOptions(
 
 void UseAnonymousNamespaceCheck::registerMatchers(MatchFinder *Finder) {
   Finder->addMatcher(
-  functionDecl(isStatic(),
+  functionDecl(isStaticStorageClass(),
unless(anyOf(isInHeaderFile(HeaderFileExtensions),
 isInAnonymousNamespace(), isMemberFunction(
   .bind("x"),
   this);
   Finder->addMatcher(
-  varDecl(isStatic(),
+  varDecl(isStaticStorageClass(),
   unless(anyOf(isInHeaderFile(HeaderFileExtensions),
isInAnonymousNamespace(), isStaticLocal(),
isStaticDataMember(), hasType(isConstQualified()



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

[PATCH] D139629: clang: Stop emitting "strictfp"

[Kevin P. Neal via Phabricator via cfe-commits]

kpn added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:2135
-llvm::AttrBuilder FuncAttrs(F->getContext());
-FuncAttrs.addAttribute("strictfp");
-F->addFnAttrs(FuncAttrs);

arsenm wrote:
> kpn wrote:
> > arsenm wrote:
> > > andrew.w.kaylor wrote:
> > > > arsenm wrote:
> > > > > zahiraam wrote:
> > > > > > I think it would better to fix this function instead of removing it 
> > > > > > entirely? The issue here is that there is the "strictfp" attribute 
> > > > > > and the llvm::Attribute::StrictFP. We could replace  
> > > > > > FuncAttrs.addAttribute("strictfp"); with
> > > > > >  FuncAttrs.addAttribute(llvm::Attribute::StrictFP); 
> > > > > > This function ensures that the function attribute is set when the 
> > > > > > FunctionDecl attribute is set. I am concerned that when it's 
> > > > > > removed, we will wind up with cases where the function attribute is 
> > > > > > missing! The only place where this function attribute is in 
> > > > > > CodeGenFunction::StartFunction. Is that enough? @andrew.w.kaylor 
> > > > > > Can you please weigh in on this?
> > > > > I currently don't have evidence that making this use the correct 
> > > > > attribute would fix anything. If something was depending on this 
> > > > > emission in this context, it's already broken
> > > > It may be that anything depending on this is already broken, but the 
> > > > code was written for a reason, even if it was always broken. I'm not 
> > > > sure I understand what that reason was, and unfortunately the person 
> > > > who wrote the code (@mibintc) is no longer actively contributing to 
> > > > LLVM. It was added here: https://reviews.llvm.org/D87528
> > > > 
> > > > It does seem like the llvm::Attribute::StrictFP is being added any time 
> > > > the string attribute is added, but they're coming from different 
> > > > places. The proper attribute seems to be coming from 
> > > > CodeGenFunction::StartFunction() which is effectively copying it from 
> > > > the function declaration. It's not clear to me whether there are 
> > > > circumstances where we get to setLLVMFunctionFEnvAttributes() through 
> > > > EmitGlobalFunctionDefinition() without ever having gone through 
> > > > CodeGenFunction::StartFunction(). It looks like maybe there are 
> > > > multiversioning cases that do that, but I couldn't come up with an 
> > > > example that does. @erichkeane wrote a lot of the multi-versioning 
> > > > code, so he might know more, but he's on vacation through the end of 
> > > > the month.
> > > > 
> > > > Eliminating this extra string attribute seems obviously good. In this 
> > > > particular location, though, I'd be inclined to set the enumerated 
> > > > attribute here, even though that might be redundant in most cases. On 
> > > > the other hand, if one of our front end experts can look at this code 
> > > > and say definitively that it's //always// redundant, I'd be fine with 
> > > > this code being deleted.
> > > I think code that can be deleted that doesn't manifest in test failures 
> > > should be immediately deleted. We shouldn't leave things around just in 
> > > case 
> > The Verifier changes that would detect the error and fail tests never made 
> > it into the tree. The lack of failures therefore tells us nothing in this 
> > case here.
> The verifier never would have checked the string form
Ah, true statement.


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

https://reviews.llvm.org/D139629

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

[clang] a18cf8d - [clang][dataflow] Remove stray lines from `Environment::join`

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-12-19T15:28:30Z
New Revision: a18cf8d14f5552c13bd1cef112ba5b88a7fc75ff

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

LOG: [clang][dataflow] Remove stray lines from `Environment::join`

Removes an assertion and a useless line. The assertion seems left over from
earlier debugging and the line that follows is a stray line.

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

Added: 


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

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 8df542410e7c..597e39e27170 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -444,10 +444,6 @@ LatticeJoinEffect Environment::join(const Environment 
&Other,
 
   JoinedEnv.MemberLocToStruct =
   intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
-  assert(JoinedEnv.MemberLocToStruct.size() <=
- std::min(MemberLocToStruct.size(), Other.MemberLocToStruct.size()));
-
-  intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
   if (MemberLocToStruct.size() != JoinedEnv.MemberLocToStruct.size())
 Effect = LatticeJoinEffect::Changed;
 



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

[PATCH] D140306: [clang][dataflow] Remove stray lines from `Environment::join`

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa18cf8d14f55: [clang][dataflow] Remove stray lines from 
`Environment::join` (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140306

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


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -444,10 +444,6 @@
 
   JoinedEnv.MemberLocToStruct =
   intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
-  assert(JoinedEnv.MemberLocToStruct.size() <=
- std::min(MemberLocToStruct.size(), Other.MemberLocToStruct.size()));
-
-  intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
   if (MemberLocToStruct.size() != JoinedEnv.MemberLocToStruct.size())
 Effect = LatticeJoinEffect::Changed;
 


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -444,10 +444,6 @@
 
   JoinedEnv.MemberLocToStruct =
   intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
-  assert(JoinedEnv.MemberLocToStruct.size() <=
- std::min(MemberLocToStruct.size(), Other.MemberLocToStruct.size()));
-
-  intersectDenseMaps(MemberLocToStruct, Other.MemberLocToStruct);
   if (MemberLocToStruct.size() != JoinedEnv.MemberLocToStruct.size())
 Effect = LatticeJoinEffect::Changed;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140307: Match derived types in in modernize-loop-convert

[Chris Cotter via Phabricator via cfe-commits]

ccotter created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a reviewer: njames93.
Herald added a project: All.
ccotter requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

This patch allows clang-tidy to replace traditional for-loops where the
container type inherits its `begin`/`end` methods from a base class.

Test plan: Added unit test cases that confirm the tool replaces the new
pattern.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140307

Files:
  clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
  clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -575,6 +575,7 @@
 const dependent *Pconstv;
 
 transparent> Cv;
+dependent_derived VD;
 
 void f() {
   int Sum = 0;
@@ -653,6 +654,15 @@
   // CHECK-FIXES: for (int I : V)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
   // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = VD.size(); E != I; ++I) {
+printf("Fibonacci number is %d\n", VD[I]);
+Sum += VD[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : VD)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
 }
 
 // Ensure that 'const auto &' is used with containers of non-trivial types.
Index: 
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
===
--- 
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
+++ 
clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
@@ -126,6 +126,10 @@
   void constFoo() const;
 };
 
+template
+class dependent_derived : public dependent {
+};
+
 template
 class doublyDependent{
  public:
Index: clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -251,17 +251,18 @@
   // functions called begin() and end() taking the container as an argument
   // are also allowed.
   TypeMatcher RecordWithBeginEnd = qualType(anyOf(
-  qualType(
-  isConstQualified(),
-  hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
-  hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
-  hasMethod(cxxMethodDecl(hasName("end"),
-  isConst()   // hasDeclaration
- ))), // qualType
+  qualType(isConstQualified(),
+   hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+   cxxRecordDecl(isSameOrDerivedFrom(cxxRecordDecl(
+   hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
+   hasMethod(cxxMethodDecl(hasName("end"),
+   isConst())) // 
hasDeclaration
+  ))), // qualType
   qualType(unless(isConstQualified()),
hasUnqualifiedDesugaredType(recordType(hasDeclaration(
-   cxxRecordDecl(hasMethod(hasName("begin")),
- hasMethod(hasName("end"))) // qualType
+   cxxRecordDecl(isSameOrDerivedFrom(cxxRecordDecl(
+   hasMethod(hasName("begin")),
+   hasMethod(hasName("end") // qualType
   ));
 
   StatementMatcher SizeCallMatcher = cxxMemberCallExpr(


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -575,6 +575,7 @@
 const dependent *Pconstv;
 
 transparent> Cv;
+dependent_derived VD;
 
 void f() {
   int Sum = 0;
@@ -653,6 +654,15 @@
   // CHECK-FIXES: for (int I : V)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
   // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = VD.size(); E != I; ++I) {
+printf("Fibonacci number is %d\n", VD[I]);
+Sum += VD[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : VD)
+  // CHECK-FIXES-NEXT:

[PATCH] D139564: clang: Don't emit "frame-pointer"="none"

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 483964.
arsenm added a comment.

Add unreachable


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

https://reviews.llvm.org/D139564

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/lib/CodeGen/CGCall.cpp
  clang/test/CodeGen/libcalls.c
  clang/test/CodeGen/long-call-attr.c
  clang/test/CodeGen/regcall2.c
  clang/test/CodeGen/xcore-abi.c
  clang/test/CodeGen/xcore-abi.cpp
  clang/test/OpenMP/amdgcn-attributes.cpp
  clang/test/OpenMP/irbuilder_safelen.cpp
  clang/test/OpenMP/irbuilder_safelen_order_concurrent.cpp
  clang/test/OpenMP/irbuilder_simd_aligned.cpp
  clang/test/OpenMP/irbuilder_simdlen.cpp
  clang/test/OpenMP/irbuilder_simdlen_safelen.cpp

Index: clang/test/OpenMP/irbuilder_simdlen_safelen.cpp
===
--- clang/test/OpenMP/irbuilder_simdlen_safelen.cpp
+++ clang/test/OpenMP/irbuilder_simdlen_safelen.cpp
@@ -123,8 +123,8 @@
   }
 }
 //.
-// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
-// CHECK: attributes #1 = { noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #1 = { noinline nounwind optnone "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
 //.
 // CHECK: !0 = !{i32 1, !"wchar_size", i32 4}
 // CHECK: !1 = !{i32 7, !"openmp", i32 45}
Index: clang/test/OpenMP/irbuilder_simdlen.cpp
===
--- clang/test/OpenMP/irbuilder_simdlen.cpp
+++ clang/test/OpenMP/irbuilder_simdlen.cpp
@@ -123,8 +123,8 @@
   }
 }
 //.
-// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
-// CHECK: attributes #1 = { noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #1 = { noinline nounwind optnone "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
 //.
 // CHECK: !0 = !{i32 1, !"wchar_size", i32 4}
 // CHECK: !1 = !{i32 7, !"openmp", i32 45}
Index: clang/test/OpenMP/irbuilder_simd_aligned.cpp
===
--- clang/test/OpenMP/irbuilder_simd_aligned.cpp
+++ clang/test/OpenMP/irbuilder_simd_aligned.cpp
@@ -162,8 +162,8 @@
   }
 }
 //.
-// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
-// CHECK: attributes #1 = { noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
+// CHECK: attributes #1 = { noinline nounwind optnone "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
 // CHECK: attributes #2 = { nocallback nofree nosync nounwind willreturn memory(inaccessiblemem: readwrite) }
 //.
 // CHECK: !0 = !{i32 1, !"wchar_size", i32 4}
Index: clang/test/OpenMP/irbuilder_safelen_order_concurrent.cpp
===
--- clang/test/OpenMP/irbuilder_safelen_order_concurrent.cpp
+++ clang/test/OpenMP/irbuilder_safelen_order_concurrent.cpp
@@ -123,8 +123,8 @@
   }
 }
 //.
-// CHECK: attributes #0 = { mustprogress noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-features"="+cx8,+mmx,+sse,+sse2,+x87" }
-// CHECK: attributes #1 = { noinline nounwind optnone "frame-pointer"="none" "min-legal-vector-width"="0" "no-trapping-math"="true" "sta

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp:46
+: ClangTidyCheck(Name, Context),
+  RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+  "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {

Look like this setting should be global for Clang-tidy, because it already used 
in other checks. It also accompanied by other option.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:123
+  ` check.
+
 - New :doc:`misc-use-anonymous-namespace

Please add first sentence from documentation.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst:39
+   A semicolon-separated list of filename extensions of header files (the 
filename
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an

Please replace quotes with single back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140290

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

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp:46
+: ClangTidyCheck(Name, Context),
+  RawStringHeaderFileExtensions(Options.getLocalOrGlobal(
+  "HeaderFileExtensions", utils::defaultHeaderFileExtensions())) {

Eugene.Zelenko wrote:
> Look like this setting should be global for Clang-tidy, because it already 
> used in other checks. It also accompanied by other option.
Fully agree, I can put up a separate patch for that. I suppose we need to wait 
2 releases to be able to remove the existing duplicated options from the 
checks? 

@njames93 Do you agree?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140290

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

[PATCH] D140308: [clang][dataflow][NFC] Fix comments related to widening.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: gribozavr2, xazax.hun.
Herald added subscribers: martong, rnkovacs.
Herald added a reviewer: NoQ.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

The comments describing the API for analysis `widen` and the environment `widen`
were overly strict in the preconditions they assumed for the operation. In
particular, both assumed that the previous value preceded the current value in
the relevant ordering. However, that's not generally how widen operators work
and widening itself can violate this property. That is, when the previous value
is the result of a widening, it can easily be "greater" than the current value.

This patch updates the comments to accurately reflect the expectations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140308

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


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -370,15 +370,18 @@
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
-  // By the API, `PrevEnv` <= `*this`, meaning `join(PrevEnv, *this) =
-  // *this`. That guarantees that these maps are subsets of the maps in
-  // `PrevEnv`. So, we don't need change their current values to widen (in
-  // contrast to `join`).
+  // By the API, `PrevEnv` is a previous version of the environment for the 
same
+  // block, so we have some guarantees about its shape. In particular, it will
+  // be the result of a join or widen operation on previous values for this
+  // block. For `DeclToLoc` and `ExprToLoc`, join guarantees that these maps 
are
+  // subsets of the maps in `PrevEnv`. So, as long as we maintain this property
+  // here, we don't need change their current values to widen.
   //
-  // FIXME: The above is violated for `MemberLocToStruct`, because `join` can
-  // cause the map size to increase (when we add fresh data in places of
-  // conflict). Once this issue with join is resolved, re-enable the assertion
-  // below or replace with something that captures the desired invariant.
+  // FIXME: `MemberLocToStruct` does not share the above property, because
+  // `join` can cause the map size to increase (when we add fresh data in 
places
+  // of conflict). Once this issue with join is resolved, re-enable the
+  // assertion below or replace with something that captures the desired
+  // invariant.
   assert(DeclToLoc.size() <= PrevEnv.DeclToLoc.size());
   assert(ExprToLoc.size() <= PrevEnv.ExprToLoc.size());
   // assert(MemberLocToStruct.size() <= PrevEnv.MemberLocToStruct.size());
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -137,10 +137,6 @@
 ///
 /// Requirements:
 ///
-///  `Prev` must precede `Current` in the value ordering. Widening is *not*
-///  called when the current value is already equivalent to the previous
-///  value.
-///
 ///  `Prev` and `Current` must model values of type `Type`.
 ///
 ///  `Prev` and `Current` must be assigned to the same storage location in
@@ -229,7 +225,7 @@
   ///
   /// Requirements:
   ///
-  ///  `PrevEnv` must precede `this` in the environment ordering.
+  ///  `PrevEnv` must be the immediate previous version of the environment.
   ///  `PrevEnv` and `this` must use the same `DataflowAnalysisContext`.
   LatticeJoinEffect widen(const Environment &PrevEnv,
   Environment::ValueModel &Model);


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -370,15 +370,18 @@
 
   auto Effect = LatticeJoinEffect::Unchanged;
 
-  // By the API, `PrevEnv` <= `*this`, meaning `join(PrevEnv, *this) =
-  // *this`. That guarantees that these maps are subsets of the maps in
-  // `PrevEnv`. So, we don't need change their current values to widen (in
-  // contrast to `join`).
+  // By the API, `PrevEnv` is a previous version of the environment for the same
+  // block, so we have some guarantees about its shape. In particular, it will
+  // be the result of a join or widen operation on previous values for this
+  // block. For `DeclToLoc` and `ExprToLoc`, join guarantees that these maps are
+  // subsets of the maps in `PrevEnv`. So, as long as we maintain this property
+  // here, we don't need change their current values to widen.
   //

[PATCH] D139705: [clang] fix zero-initialization fix-it for variable template

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Ping?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

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

[PATCH] D136639: [CodeGen][ObjC] Fix a memory leak that occurs when a non-trivial C struct property is set using dot notation

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 483973.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Check that the destructor is called conditionally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136639

Files:
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/test/CodeGenObjC/nontrivial-c-struct-property.m


Index: clang/test/CodeGenObjC/nontrivial-c-struct-property.m
===
--- clang/test/CodeGenObjC/nontrivial-c-struct-property.m
+++ clang/test/CodeGenObjC/nontrivial-c-struct-property.m
@@ -68,3 +68,39 @@
 // CHECK: call void @objc_copyCppObjectAtomic({{.*}}, {{.*}}, ptr noundef 
@__move_assignment_8_8_s0)
 // CHECK-NOT: call
 // CHECK: ret void
+
+// CHECK: define void @test0(ptr noundef %[[C:.*]], ptr noundef %[[A:.*]])
+// CHECK: %[[C_ADDR:.*]] = alloca ptr, align 8
+// CHECK: %[[A_ADDR:.*]] = alloca ptr, align 8
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_S0]], align 8
+// CHECK: %[[AGG_TMP:.*]] = alloca %[[STRUCT_S0]], align 8
+// CHECK: store ptr null, ptr %[[C_ADDR]], align 8
+// CHECK: call void @llvm.objc.storeStrong(ptr %[[C_ADDR]], ptr %[[C]])
+// CHECK: store ptr %[[A]], ptr %[[A_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load ptr, ptr %[[A_ADDR]], align 8
+// CHECK: call void @__copy_constructor_8_8_s0(ptr %[[AGG_TMP_ENSURED]], ptr 
%[[V0]])
+// CHECK: %[[V1:.*]] = load ptr, ptr %[[C_ADDR]], align 8
+// CHECK: call void @__copy_constructor_8_8_s0(ptr %[[AGG_TMP]], ptr 
%[[AGG_TMP_ENSURED]])
+// CHECK: %[[V2:.*]] = icmp eq ptr %[[V1]], null
+// CHECK: br i1 %[[V2]], label %[[MSGSEND_NULL:.*]], label %[[MSGSEND_CALL:.*]]
+
+// CHECK: [[MSGSEND_CALL]]:
+// CHECK: %[[V3:.*]] = load ptr, ptr @OBJC_SELECTOR_REFERENCES_, align 8
+// CHECK: %[[COERCE_DIVE:.*]] = getelementptr inbounds %[[STRUCT_S0]], ptr 
%[[AGG_TMP]], i32 0, i32 0
+// CHECK: %[[V4:.*]] = load ptr, ptr %[[COERCE_DIVE]], align 8
+// CHECK: %[[COERCE_VAL_PI:.*]] = ptrtoint ptr %[[V4]] to i64
+// CHECK: call void @objc_msgSend(ptr noundef %[[V1]], ptr noundef %[[V3]], 
i64 %[[COERCE_VAL_PI]])
+// CHECK: br label %[[MSGSEND_CONT:.*]]
+
+// CHECK: [[MSGSEND_NULL]]:
+// CHECK: call void @__destructor_8_s0(ptr %[[AGG_TMP]])
+// CHECK: br label %[[MSGSEND_CONT]]
+
+// CHECK: [[MSGSEND_CONT]]:
+// CHECK: call void @__destructor_8_s0(ptr %[[AGG_TMP_ENSURED]]
+// CHECK: call void @llvm.objc.storeStrong(ptr %[[C_ADDR]], ptr null)
+// CHECK: ret void
+
+void test0(C *c, S0 *a) {
+  c.atomic0 = *a;
+}
Index: clang/lib/CodeGen/CGExprAgg.cpp
===
--- clang/lib/CodeGen/CGExprAgg.cpp
+++ clang/lib/CodeGen/CGExprAgg.cpp
@@ -205,7 +205,16 @@
   return EmitFinalDestCopy(E->getType(), LV);
 }
 
-CGF.EmitPseudoObjectRValue(E, EnsureSlot(E->getType()));
+AggValueSlot Slot = EnsureSlot(E->getType());
+bool NeedsDestruction =
+!Slot.isExternallyDestructed() &&
+E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct;
+if (NeedsDestruction)
+  Slot.setExternallyDestructed();
+CGF.EmitPseudoObjectRValue(E, Slot);
+if (NeedsDestruction)
+  CGF.pushDestroy(QualType::DK_nontrivial_c_struct, Slot.getAddress(),
+  E->getType());
   }
 
   void VisitVAArgExpr(VAArgExpr *E);


Index: clang/test/CodeGenObjC/nontrivial-c-struct-property.m
===
--- clang/test/CodeGenObjC/nontrivial-c-struct-property.m
+++ clang/test/CodeGenObjC/nontrivial-c-struct-property.m
@@ -68,3 +68,39 @@
 // CHECK: call void @objc_copyCppObjectAtomic({{.*}}, {{.*}}, ptr noundef @__move_assignment_8_8_s0)
 // CHECK-NOT: call
 // CHECK: ret void
+
+// CHECK: define void @test0(ptr noundef %[[C:.*]], ptr noundef %[[A:.*]])
+// CHECK: %[[C_ADDR:.*]] = alloca ptr, align 8
+// CHECK: %[[A_ADDR:.*]] = alloca ptr, align 8
+// CHECK: %[[AGG_TMP_ENSURED:.*]] = alloca %[[STRUCT_S0]], align 8
+// CHECK: %[[AGG_TMP:.*]] = alloca %[[STRUCT_S0]], align 8
+// CHECK: store ptr null, ptr %[[C_ADDR]], align 8
+// CHECK: call void @llvm.objc.storeStrong(ptr %[[C_ADDR]], ptr %[[C]])
+// CHECK: store ptr %[[A]], ptr %[[A_ADDR]], align 8
+// CHECK: %[[V0:.*]] = load ptr, ptr %[[A_ADDR]], align 8
+// CHECK: call void @__copy_constructor_8_8_s0(ptr %[[AGG_TMP_ENSURED]], ptr %[[V0]])
+// CHECK: %[[V1:.*]] = load ptr, ptr %[[C_ADDR]], align 8
+// CHECK: call void @__copy_constructor_8_8_s0(ptr %[[AGG_TMP]], ptr %[[AGG_TMP_ENSURED]])
+// CHECK: %[[V2:.*]] = icmp eq ptr %[[V1]], null
+// CHECK: br i1 %[[V2]], label %[[MSGSEND_NULL:.*]], label %[[MSGSEND_CALL:.*]]
+
+// CHECK: [[MSGSEND_CALL]]:
+// CHECK: %[[V3:.*]] = load ptr, ptr @OBJC_SELECTOR_REFERENCES_, align 8
+// CHECK: %[[COERCE_DIVE:.*]] = getelementptr inbounds %[[STRUCT_S0]], ptr %[[AGG_TMP]], i32 0, i32 0
+// CHECK: %[[V4:.*]] = load ptr, ptr %[[COERCE_DIVE]], align 8
+// CHECK: %[[COERCE_V

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

[Shangwu Yao via Phabricator via cfe-commits]

shangwuyao added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1198
 
-def CUDAGlobal : InheritableAttr {
-  let Spellings = [GNU<"global">, Declspec<"__global__">];
+def CUDAGlobal : InheritableAttr, TargetSpecificAttr {
+  let Spellings = [GNU<"global">, Declspec<"__global__">, 
Clang<"nvptx_kernel">];

jhuber6 wrote:
> tra wrote:
> > Nice.
> > 
> > This reminded me that we have a project compiling CUDA, but targeting 
> > SPIR-V instead of NVPTX. It looks like this will likely break them. The 
> > project is out-of-tree, but I'd still need to figure out how to keep them 
> > working.  I guess it would be easy enough to expand TargetNVPTX to 
> > TargetNVPTXOrSpirV. I'm mostly concerned about logistics of making it 
> > happen without disruption.
> > 
> > 
> This might've broken more stuff after looking into it, I forgot that `AMDGPU` 
> still uses the same CUDA attributes, and the host portion of CUDA also checks 
> these. It would be nice if there was a way to say "CUDA" or "NVPTX", 
> wondering if that's possible in the tablegen here.
What's the plan here for keeping the SPIR-V and AMDGPU working? Would it work 
if we simply get rid of the `TargetSpecificAttr`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140226

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

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sergei Barannikov via Phabricator via cfe-commits]

barannikov88 added a comment.

Related: https://reviews.llvm.org/D139974


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[PATCH] D136639: [CodeGen][ObjC] Fix a memory leak that occurs when a non-trivial C struct property is set using dot notation

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: clang/test/CodeGenObjC/nontrivial-c-struct-property.m:89
+
+// CHECK: call void @__destructor_8_s0(ptr %[[AGG_TMP_ENSURED]])
+

rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > It looks like we're copying `a` into a temporary and then copying out of 
> > > that temporary into the argument slot.  I don't see any reason the extra 
> > > copy is required.
> > > 
> > > Also, aren't non-trivial structs consumed as arguments?  If so, this 
> > > would be an over-release.
> > It isn't an over-release because, if the receiver isn't nil, temporary 
> > `AGG_TMP` is destructed in the callee and, if the receiver isn't nil, the 
> > temporary is destructed in the caller (by the call to 
> > `@__destructor_8_s0(ptr %[[AGG_TMP]])`).
> > 
> > As you pointed out, the copy isn't needed if the result of the assignment 
> > isn't used.
> Oh, is this second destructor call in conditionally-executed code?  Can we 
> test for that, at least?
The first copy constructor call is emitted when `emitPseudoObjectExpr` 
evaluates the `OpaqueValueExpr` that is the result expression. The second copy 
constructor call is emitted to copy the argument `*a` to the argument temporary.

Since the result expression isn't used in this case, there isn't any reason to 
emit the first copy constructor call. If Sema marks the `OpaqueValueExpr` for 
the result expression as unique when the result is unused, CodeGen avoids 
emitting the first copy constructor.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136639

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

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne updated this revision to Diff 483975.
sebastian-ne marked 2 inline comments as done.
sebastian-ne added a comment.

> It is surprising to me that std::any can work without RTTI. Never thought it 
> could be implemented.

It seems like libstdc++ and libc++ both implement it the way llvm::Any is 
implemented when RTTI is not available (actually, I think llvm::Any was copied 
from libstdc++ at some point).

> This "compare function pointer" trick is awesome, but it might not always 
> work as explained in this commit.
> This question however, suggest that any_cast doesn't always work with RTTI 
> either, which is weird.
> I don't know of any other potential issues. std::visitor can't be used with 
> std::any in absense of std::any::type, but that's minor, I think.

As you noticed, it is very difficult to implement Any correctly on every 
platform under any circumstances and compiler flags.
That is exactly what this patch aims at: Moving the responsibility to implement 
Any correctly to the standard library.

Unfortunatly, we can’t replace llvm::Any with std::any just yet, because of 
bugs in msvc that were only fixed recently.

> llvm::any_isa<> is kind of convenient and is aligned with llvm::isa<>. Same 
> for llvm::any_cast.

It is, however, there is no std::any_isa or std::any_cast_or_null, so the 
question gets wether we want to keep llvm::Any around as a wrapper of std::any 
once we can use it (in this case this patch would be obsolete)
or if we we want to remove llvm::Any and use std::any only.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

Files:
  clang/unittests/Tooling/RefactoringTest.cpp
  clang/unittests/Tooling/TransformerTest.cpp
  lldb/include/lldb/Core/RichManglingContext.h
  llvm/include/llvm/ADT/Any.h
  llvm/lib/CodeGen/MachinePassManager.cpp
  llvm/lib/IR/LLVMContextImpl.h
  llvm/lib/Passes/StandardInstrumentations.cpp
  llvm/lib/Transforms/IPO/SampleProfileProbe.cpp
  llvm/lib/Transforms/Scalar/LoopPassManager.cpp
  llvm/lib/Transforms/Utils/Debugify.cpp
  llvm/unittests/ADT/AnyTest.cpp
  llvm/unittests/IR/PassBuilderCallbacksTest.cpp

Index: llvm/unittests/IR/PassBuilderCallbacksTest.cpp
===
--- llvm/unittests/IR/PassBuilderCallbacksTest.cpp
+++ llvm/unittests/IR/PassBuilderCallbacksTest.cpp
@@ -290,17 +290,17 @@
   return std::string(name);
 }
 
-template <> std::string getName(const llvm::Any &WrappedIR) {
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName().str();
-  if (any_isa(WrappedIR))
-return any_cast(WrappedIR)->getName();
+template <> std::string getName(const Any &WrappedIR) {
+  if (const auto *const *M = any_cast(&WrappedIR))
+return (*M)->getName().str();
+  if (const auto *const *F = any_cast(&WrappedIR))
+return (*F)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *L = any_cast(&WrappedIR))
+return (*L)->getName().str();
+  if (const auto *const *C = any_cast(&WrappedIR))
+return (*C)->getName();
   return "";
 }
 /// Define a custom matcher for objects which support a 'getName' method.
Index: llvm/unittests/ADT/AnyTest.cpp
===
--- llvm/unittests/ADT/AnyTest.cpp
+++ llvm/unittests/ADT/AnyTest.cpp
@@ -24,55 +24,55 @@
 
   // An empty Any is not anything.
   EXPECT_FALSE(A.has_value());
-  EXPECT_FALSE(any_isa(A));
+  EXPECT_FALSE(any_cast(&A));
 
   // An int is an int but not something else.
   EXPECT_TRUE(B.has_value());
-  EXPECT_TRUE(any_isa(B));
-  EXPECT_FALSE(any_isa(B));
+  EXPECT_TRUE(any_cast(&B));
+  EXPECT_FALSE(any_cast(&B));
 
   EXPECT_TRUE(C.has_value());
-  EXPECT_TRUE(any_isa(C));
+  EXPECT_TRUE(any_cast(&C));
 
   // A const char * is a const char * but not an int.
   EXPECT_TRUE(D.has_value());
-  EXPECT_TRUE(any_isa(D));
-  EXPECT_FALSE(any_isa(D));
+  EXPECT_TRUE(any_cast(&D));
+  EXPECT_FALSE(any_cast(&D));
 
   // A double is a double but not a float.
   EXPECT_TRUE(E.has_value());
-  EXPECT_TRUE(any_isa(E));
-  EXPECT_FALSE(any_isa(E));
+  EXPECT_TRUE(any_cast(&E));
+  EXPECT_FALSE(any_cast(&E));
 
   // After copy constructing from an int, the new item and old item are both
   // ints.
   llvm::Any F(B);
   EXPECT_TRUE(B.has_value());
   EXPECT_TRUE(F.has_value());
-  EXPECT_TRUE(any_isa(F));
-  EXPECT_TRUE(any_isa(B));
+  EXPECT_TRUE(any_cast(&F));
+  EXPECT_TRUE(any_cast(&B));
 
   // After move constructing from an int, the new item is an int and the old one
   // isn't.
   llvm::Any G(std::move(C));
   EXPECT_FALSE(C.has_value());
   EXPECT_TRUE(G.has_value());
-  EXPE

[PATCH] D139973: [llvm] Make llvm::Any similar to std::any

[Sebastian Neubauer via Phabricator via cfe-commits]

sebastian-ne added inline comments.



Comment at: lldb/include/lldb/Core/RichManglingContext.h:90-91
 assert(parser.has_value());
-assert(llvm::any_isa(parser));
+assert(llvm::any_cast(&parser));
 return llvm::any_cast(parser);
   }

barannikov88 wrote:
> This is not intuitively clear. In assert you pass an address, but in 'return' 
> below the object is passed by reference.
> 
I changed it to use an address + explicit dereference in the return. I hope 
that makes it clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139973

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

[PATCH] D140226: [NVPTX] Introduce attribute to mark kernels without a language mode

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added inline comments.



Comment at: clang/include/clang/Basic/Attr.td:1198
 
-def CUDAGlobal : InheritableAttr {
-  let Spellings = [GNU<"global">, Declspec<"__global__">];
+def CUDAGlobal : InheritableAttr, TargetSpecificAttr {
+  let Spellings = [GNU<"global">, Declspec<"__global__">, 
Clang<"nvptx_kernel">];

shangwuyao wrote:
> jhuber6 wrote:
> > tra wrote:
> > > Nice.
> > > 
> > > This reminded me that we have a project compiling CUDA, but targeting 
> > > SPIR-V instead of NVPTX. It looks like this will likely break them. The 
> > > project is out-of-tree, but I'd still need to figure out how to keep them 
> > > working.  I guess it would be easy enough to expand TargetNVPTX to 
> > > TargetNVPTXOrSpirV. I'm mostly concerned about logistics of making it 
> > > happen without disruption.
> > > 
> > > 
> > This might've broken more stuff after looking into it, I forgot that 
> > `AMDGPU` still uses the same CUDA attributes, and the host portion of CUDA 
> > also checks these. It would be nice if there was a way to say "CUDA" or 
> > "NVPTX", wondering if that's possible in the tablegen here.
> What's the plan here for keeping the SPIR-V and AMDGPU working? Would it work 
> if we simply get rid of the `TargetSpecificAttr`?
Yeah, it would I'll need to update the patch. The best solution would be if 
there were a way to say "TargetNVPTX or LangOpts.CUDA". Not sure if that's 
possible in Tablegen. The previous diff I had worked fine, but we should 
definitely try to avoid rework.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140226

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

[PATCH] D140290: [clang-tidy] Add misc-static-declaration-in-header check

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 483981.
carlosgalvezp marked an inline comment as done.
carlosgalvezp added a comment.

Add description in release notes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140290

Files:
  clang-tools-extra/clang-tidy/misc/CMakeLists.txt
  clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
  clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.cpp
  clang-tools-extra/clang-tidy/misc/StaticDeclarationInHeaderCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst
  
clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp

Index: clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc/static-declaration-in-header.hpp
@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy %s misc-static-declaration-in-header %t
+
+static int v1 = 123;
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: static declaration in header; remove 'static' [misc-static-declaration-in-header]
+// CHECK-FIXES: {{^}}int v1 = 123;
+
+static const int c1 = 123;
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: static declaration in header; remove 'static'
+// CHECK-FIXES: {{^}}const int c1 = 123;
+
+static constexpr int c2 = 123;
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: static declaration in header; remove 'static'
+// CHECK-FIXES: {{^}}constexpr int c2 = 123;
+
+static void f1(){}
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: static declaration in header; remove 'static'
+// CHECK-FIXES: {{^}}void f1(){}
+
+namespace foo {
+  static int v2 = 123;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static'
+  // CHECK-FIXES: {{^}}  int v2 = 123;
+
+  static int f2(){}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static'
+  // CHECK-FIXES: {{^}}  int f2(){}
+}
+
+namespace {
+  static int v3 = 123;
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static'
+  // CHECK-FIXES: {{^}}  int v3 = 123;
+
+  static int f3(){}
+  // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: static declaration in header; remove 'static'
+  // CHECK-FIXES: {{^}}  int f3(){}
+}
+
+// OK
+struct Foo {
+  static const int v4 = 123;
+  static void f4(){}
+};
+
+// OK
+void f5() {
+  static int v5;
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc/static-declaration-in-header.rst
@@ -0,0 +1,41 @@
+.. title:: clang-tidy - misc-static-declaration-in-header
+
+misc-static-declaration-in-header
+=
+
+Warns about ``static`` variable and function declarations at file or namespace
+scope in header files. Provides fix-it hint to remove ``static``.
+
+Header files are used to share code among different translation units. ``static``,
+on the other hand, can be used to express that a given declaration has internal
+linkage. Using ``static`` in header files leads to each translation unit creating
+their own internal copy of the function or variable at hand, which is problematic.
+This can cause unexpected results, bloat the resulting executable or even trigger
+one-definition-rule (ODR) violations.
+
+This problem is similar to having anonymous namespaces in header files, already
+covered by :doc:`google-build-namespaces`.
+
+Example of problematic code:
+
+.. code-block:: c++
+
+  // foo.h
+
+  // Bad
+  static int x = 123;
+  static void f() {}
+
+  // Good
+  int x = 123;
+  void f() {}
+
+Options
+---
+
+.. option:: HeaderFileExtensions
+
+   A semicolon-separated list of filename extensions of header files (the filename
+   extensions should not include "." prefix). Default is ";h;hh;hpp;hxx".
+   For extension-less header files, using an empty string or leaving an
+   empty string between ";" if there are other filename extensions.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -254,6 +254,7 @@
`misc-non-private-member-variables-in-classes `_,
`misc-redundant-expression `_, "Yes"
`misc-static-assert `_, "Yes"
+   `misc-static-declaration-in-header `_, "Yes"
`misc-throw-by-value-catch-by-reference `_,
`misc-unconventional-assign-operator `_,
`misc-uniqueptr-reset-release `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNote