[PATCH] D145793: [clang][AST] Improve diagnostic for `nullptr` constexpr function pointer call

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:7673
+Info.FFDiag(Callee, diag::note_constexpr_null_callee)
+<< const_cast(Callee);
+return false;

tbaeder wrote:
> Is the `const_cast` really necessary?
> Is the `const_cast` really necessary?
Without `const_cast`, it did not compile.
I searched the existing codebase to find this line 
https://github.com/llvm/llvm-project/blob/151d3b607e1e3256ed901e02b48b92e79a77021d/clang/lib/Sema/SemaConcept.cpp#L300
 and I did the same here.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145793

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

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Please keep in mind that I don't have a lot of expertise in modules, but this 
sure seems like a module merging bug from what I can tell, and it might even 
relate to work happening in D145737  (CC 
@rsmith). That patch does not seem to address this issue, but what I'm seeing 
when I try to debug this is a bit confusing to me:

In the reproducer from Sam, there are three lambdas: 
`create::Create([] {});` in `main.cpp`, but this does not participate in AST 
merging because it's within the main source file
`BuildWidget([dummy](Widget*) {});` in `create.h`
`create::Create([] {});` in `instantiate_create.h`
(While `wrap_create.h` includes `create.h`, it never instantiates the template 
and so should not produce an additional lambda)

So I would expect `ASTDeclReader::ReadCXXDefinitionData()` to find three 
lambdas, but it finds *four*. The first lambda it reads has no captures, so 
this is presumably the one in `instantiate_create.h`. The next three all have 
captures. So where's that fourth lambda coming from?

The reason I think this relates is because 
`LocalInstantiationScope::findInstantiationOf()` seems to think there's one 
declaration in the local scope, but the argument passed to the function is not 
the one found, which suggest to me there's a module merging issue and we wind 
up hitting our favorite assert.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D145770: [clang-offload-bundler] Standardize TargetID field for bundler

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

The description needs fix. "ABI field" should be "environment component".

Also, we need a clang-offload-bundler test which bundles with a non-canonical 
triple and unbundles with a canonical triple and vice versa.




Comment at: clang/lib/Driver/OffloadBundler.cpp:76-79
+// Enforce optional ABI field to standardize bundles
+llvm::Triple t = llvm::Triple(KindTriple.second);
+this->Triple = llvm::Triple(t.getArchName(), t.getVendorName(),
+t.getOSName(), t.getEnvironmentName());

can we use Triple::normalize to normalize KindTriple.second, then use the 
returned string to create this->Triple? This is the conventional way to 
normalize the triple.

same as below


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145770

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

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin added a comment.

@kadircet thank you for the review! Please take another look.




Comment at: clang-tools-extra/clangd/CompileCommands.cpp:199
+  // use/change working directory, which ExpandResponseFiles doesn't).
+  FS = llvm::vfs::getRealFileSystem();
+}

kadircet wrote:
> getting real filesystem is cheap, no need to make this part of the state, 
> just inline it into `tooling::addExpandedResponseFiles` call.
We need version of `tooling::addExpandedResponseFiles` with FS as an argument 
for testing at least, see 
https://github.com/llvm/llvm-project/blob/main/clang/unittests/Tooling/CompilationDatabaseTest.cpp#L968
 switching it to using real filesystem IMHO makes things worse. So moved this 
code to the call site.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143436

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

[PATCH] D143436: [clangd] Move standard options adaptor to CommandMangler

[Dmitry Polukhin via Phabricator via cfe-commits]

DmitryPolukhin updated this revision to Diff 504156.
DmitryPolukhin marked 4 inline comments as done.
DmitryPolukhin added a comment.

Comments resolved


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143436

Files:
  clang-tools-extra/clangd/CompileCommands.cpp
  clang-tools-extra/clangd/CompileCommands.h
  clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
  clang-tools-extra/clangd/test/did-change-configuration-params.test
  clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
  clang/include/clang/Tooling/Tooling.h
  clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
  clang/lib/Tooling/Tooling.cpp

Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -43,6 +43,7 @@
 #include "llvm/Option/OptTable.h"
 #include "llvm/Option/Option.h"
 #include "llvm/Support/Casting.h"
+#include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FileSystem.h"
@@ -299,6 +300,31 @@
   }
 }
 
+void addExpandedResponseFiles(std::vector ,
+  llvm::StringRef WorkingDir,
+  llvm::cl::TokenizerCallback Tokenizer,
+  llvm::vfs::FileSystem ) {
+  bool SeenRSPFile = false;
+  llvm::SmallVector Argv;
+  Argv.reserve(CommandLine.size());
+  for (auto  : CommandLine) {
+Argv.push_back(Arg.c_str());
+if (!Arg.empty())
+  SeenRSPFile |= Arg.front() == '@';
+  }
+  if (!SeenRSPFile)
+return;
+  llvm::BumpPtrAllocator Alloc;
+  llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
+  llvm::Error Err =
+  ECtx.setVFS().setCurrentDir(WorkingDir).expandResponseFiles(Argv);
+  if (Err)
+llvm::errs() << Err;
+  // Don't assign directly, Argv aliases CommandLine.
+  std::vector ExpandedArgv(Argv.begin(), Argv.end());
+  CommandLine = std::move(ExpandedArgv);
+}
+
 } // namespace tooling
 } // namespace clang
 
@@ -684,7 +710,7 @@
 
   if (!Invocation.run())
 return nullptr;
- 
+
   assert(ASTs.size() == 1);
   return std::move(ASTs[0]);
 }
Index: clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
===
--- clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
+++ clang/lib/Tooling/ExpandResponseFilesCompilationDatabase.cpp
@@ -7,6 +7,7 @@
 //===--===//
 
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/ConvertUTF.h"
@@ -48,28 +49,9 @@
 
 private:
   std::vector expand(std::vector Cmds) const {
-for (auto  : Cmds) {
-  bool SeenRSPFile = false;
-  llvm::SmallVector Argv;
-  Argv.reserve(Cmd.CommandLine.size());
-  for (auto  : Cmd.CommandLine) {
-Argv.push_back(Arg.c_str());
-if (!Arg.empty())
-  SeenRSPFile |= Arg.front() == '@';
-  }
-  if (!SeenRSPFile)
-continue;
-  llvm::BumpPtrAllocator Alloc;
-  llvm::cl::ExpansionContext ECtx(Alloc, Tokenizer);
-  llvm::Error Err = ECtx.setVFS(FS.get())
-.setCurrentDir(Cmd.Directory)
-.expandResponseFiles(Argv);
-  if (Err)
-llvm::errs() << Err;
-  // Don't assign directly, Argv aliases CommandLine.
-  std::vector ExpandedArgv(Argv.begin(), Argv.end());
-  Cmd.CommandLine = std::move(ExpandedArgv);
-}
+for (auto  : Cmds)
+  tooling::addExpandedResponseFiles(Cmd.CommandLine, Cmd.Directory,
+Tokenizer, *FS);
 return Cmds;
   }
 
Index: clang/include/clang/Tooling/Tooling.h
===
--- clang/include/clang/Tooling/Tooling.h
+++ clang/include/clang/Tooling/Tooling.h
@@ -506,6 +506,12 @@
 void addTargetAndModeForProgramName(std::vector ,
 StringRef InvokedAs);
 
+/// Helper function that expands response files in command line.
+void addExpandedResponseFiles(std::vector ,
+  llvm::StringRef WorkingDir,
+  llvm::cl::TokenizerCallback Tokenizer,
+  llvm::vfs::FileSystem );
+
 /// Creates a \c CompilerInvocation.
 CompilerInvocation *newInvocation(DiagnosticsEngine *Diagnostics,
   ArrayRef CC1Args,
Index: clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
===
--- clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
+++ clang-tools-extra/clangd/unittests/CompileCommandsTests.cpp
@@ -20,6 +20,7 @@
 #include "llvm/Support/FileSystem.h"
 #include 

[PATCH] D144405: [clang][pp] Handle attributes defined by plugin in __has_attribute

[Erich Keane via Phabricator via cfe-commits]

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

So here's a potential idea for future development:  It isn't uncommon/untypical 
for an attribute to want to return something other than '1', for 'version' 
(usually an integral value representing a date).  The standard attributes all 
do this.  It might be worth looking into some infrastructure to do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144405

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

[PATCH] D145769: [clang] Extract ParsedAttrInfo::hasSpelling method (NFC)

[Anders Waldenborg via Phabricator via cfe-commits]

wanders edited the summary of this revision.
wanders updated this revision to Diff 504150.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145769

Files:
  clang/include/clang/Basic/ParsedAttrInfo.h
  clang/lib/Sema/ParsedAttr.cpp


Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -126,9 +126,8 @@
 SyntaxUsed = AttributeCommonInfo::AS_Keyword;
 
   for (auto  : getAttributePluginInstances())
-for (auto  : Ptr->Spellings)
-  if (S.Syntax == SyntaxUsed && S.NormalizedFullName == FullName)
-return *Ptr;
+if (Ptr->hasSpelling(SyntaxUsed, FullName))
+  return *Ptr;
 
   // If we failed to find a match then return a default ParsedAttrInfo.
   static const ParsedAttrInfo DefaultParsedAttrInfo(
Index: clang/include/clang/Basic/ParsedAttrInfo.h
===
--- clang/include/clang/Basic/ParsedAttrInfo.h
+++ clang/include/clang/Basic/ParsedAttrInfo.h
@@ -88,6 +88,13 @@
 public:
   virtual ~ParsedAttrInfo() = default;
 
+  /// Check if this attribute has specified spelling.
+  bool hasSpelling(AttributeCommonInfo::Syntax Syntax, StringRef Name) const {
+return llvm::any_of(Spellings, [&](const Spelling ) {
+  return (S.Syntax == Syntax && S.NormalizedFullName == Name);
+});
+  }
+
   /// Check if this attribute appertains to D, and issue a diagnostic if not.
   virtual bool diagAppertainsToDecl(Sema , const ParsedAttr ,
 const Decl *D) const {


Index: clang/lib/Sema/ParsedAttr.cpp
===
--- clang/lib/Sema/ParsedAttr.cpp
+++ clang/lib/Sema/ParsedAttr.cpp
@@ -126,9 +126,8 @@
 SyntaxUsed = AttributeCommonInfo::AS_Keyword;
 
   for (auto  : getAttributePluginInstances())
-for (auto  : Ptr->Spellings)
-  if (S.Syntax == SyntaxUsed && S.NormalizedFullName == FullName)
-return *Ptr;
+if (Ptr->hasSpelling(SyntaxUsed, FullName))
+  return *Ptr;
 
   // If we failed to find a match then return a default ParsedAttrInfo.
   static const ParsedAttrInfo DefaultParsedAttrInfo(
Index: clang/include/clang/Basic/ParsedAttrInfo.h
===
--- clang/include/clang/Basic/ParsedAttrInfo.h
+++ clang/include/clang/Basic/ParsedAttrInfo.h
@@ -88,6 +88,13 @@
 public:
   virtual ~ParsedAttrInfo() = default;
 
+  /// Check if this attribute has specified spelling.
+  bool hasSpelling(AttributeCommonInfo::Syntax Syntax, StringRef Name) const {
+return llvm::any_of(Spellings, [&](const Spelling ) {
+  return (S.Syntax == Syntax && S.NormalizedFullName == Name);
+});
+  }
+
   /// Check if this attribute appertains to D, and issue a diagnostic if not.
   virtual bool diagAppertainsToDecl(Sema , const ParsedAttr ,
 const Decl *D) const {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144405: [clang][pp] Handle attributes defined by plugin in __has_attribute

[Anders Waldenborg via Phabricator via cfe-commits]

wanders updated this revision to Diff 504152.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144405

Files:
  clang/docs/ReleaseNotes.rst
  clang/examples/Attribute/Attribute.cpp
  clang/lib/Basic/Attributes.cpp
  clang/test/Frontend/plugin-attribute-pp.cpp


Index: clang/test/Frontend/plugin-attribute-pp.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-attribute-pp.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %s | FileCheck %s
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %s -x c | 
FileCheck %s
+// REQUIRES: plugins, examples
+
+#ifdef __cplusplus
+# define HAS_ATTR(a) __has_cpp_attribute (a)
+#else
+# define HAS_ATTR(a) __has_c_attribute (a)
+#endif
+
+#if __has_attribute(example)
+// CHECK: has_attribute(example) was true
+has_attribute(example) was true
+#endif
+#if HAS_ATTR(example)
+// CHECK: has_$LANG_attribute(example) was true
+has_$LANG_attribute(example) was true
+#endif
+
+#if __has_attribute(doesnt_exist)
+// CHECK-NOT: has_attribute(doesnt_exist) unexpectedly was true
+has_attribute(doesnt_exist) unexpectedly was true
+#endif
+
+#if HAS_ATTR(doesnt_exist)
+// CHECK-NOT: has_$LANG_attribute(doesnt_exist) unexpectedly was true
+has_$LANG_attribute(doesnt_exist) unexpectedly was true
+#endif
Index: clang/lib/Basic/Attributes.cpp
===
--- clang/lib/Basic/Attributes.cpp
+++ clang/lib/Basic/Attributes.cpp
@@ -2,6 +2,7 @@
 #include "clang/Basic/AttrSubjectMatchRules.h"
 #include "clang/Basic/AttributeCommonInfo.h"
 #include "clang/Basic/IdentifierTable.h"
+#include "clang/Basic/ParsedAttrInfo.h"
 using namespace clang;
 
 static int hasAttributeImpl(AttributeCommonInfo::Syntax Syntax, StringRef Name,
@@ -40,6 +41,11 @@
   if (res)
 return res;
 
+  // Check if any plugin provides this attribute.
+  for (auto  : getAttributePluginInstances())
+if (Ptr->hasSpelling(Syntax, Name))
+  return 1;
+
   return 0;
 }
 
Index: clang/examples/Attribute/Attribute.cpp
===
--- clang/examples/Attribute/Attribute.cpp
+++ clang/examples/Attribute/Attribute.cpp
@@ -9,6 +9,8 @@
 // Example clang plugin which adds an an annotation to file-scope declarations
 // with the 'example' attribute.
 //
+// This plugin is used by clang/test/Frontend/plugin-attribute tests.
+//
 
//===--===//
 
 #include "clang/AST/ASTContext.h"
@@ -27,9 +29,10 @@
 // number of arguments. This just illustrates how many arguments a
 // `ParsedAttrInfo` can hold, we will not use that much in this example.
 OptArgs = 15;
-// GNU-style __attribute__(("example")) and C++-style [[example]] and
+// GNU-style __attribute__(("example")) and C++/C2x-style [[example]] and
 // [[plugin::example]] supported.
 static constexpr Spelling S[] = {{ParsedAttr::AS_GNU, "example"},
+ {ParsedAttr::AS_C2x, "example"},
  {ParsedAttr::AS_CXX11, "example"},
  {ParsedAttr::AS_CXX11, 
"plugin::example"}};
 Spellings = S;
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -146,6 +146,8 @@
   uses the optional USR value when indexing Clang's AST. This value is expected
   to be generated by an external compiler when generating C++ bindings during
   the compilation of the foreign language sources (e.g. Swift).
+- The ``__has_attribute``, ``__has_c_attribute`` and ``__has_cpp_attribute``
+  preprocessor operators now return 1 also for attributes defined by plugins.
 
 Improvements to Clang's diagnostics
 ---


Index: clang/test/Frontend/plugin-attribute-pp.cpp
===
--- /dev/null
+++ clang/test/Frontend/plugin-attribute-pp.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %s | FileCheck %s
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %s -x c | FileCheck %s
+// REQUIRES: plugins, examples
+
+#ifdef __cplusplus
+# define HAS_ATTR(a) __has_cpp_attribute (a)
+#else
+# define HAS_ATTR(a) __has_c_attribute (a)
+#endif
+
+#if __has_attribute(example)
+// CHECK: has_attribute(example) was true
+has_attribute(example) was true
+#endif
+#if HAS_ATTR(example)
+// CHECK: has_$LANG_attribute(example) was true
+has_$LANG_attribute(example) was true
+#endif
+
+#if __has_attribute(doesnt_exist)
+// CHECK-NOT: has_attribute(doesnt_exist) unexpectedly was true
+has_attribute(doesnt_exist) unexpectedly was true
+#endif
+
+#if HAS_ATTR(doesnt_exist)
+// CHECK-NOT: 

[PATCH] D144404: [clang] Extract function for generated part of clang::hasAttribute (NFC)

[Anders Waldenborg via Phabricator via cfe-commits]

wanders updated this revision to Diff 504151.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144404

Files:
  clang/lib/Basic/Attributes.cpp


Index: clang/lib/Basic/Attributes.cpp
===
--- clang/lib/Basic/Attributes.cpp
+++ clang/lib/Basic/Attributes.cpp
@@ -4,6 +4,15 @@
 #include "clang/Basic/IdentifierTable.h"
 using namespace clang;
 
+static int hasAttributeImpl(AttributeCommonInfo::Syntax Syntax, StringRef Name,
+StringRef ScopeName, const TargetInfo ,
+const LangOptions ) {
+
+#include "clang/Basic/AttrHasAttributeImpl.inc"
+
+  return 0;
+}
+
 int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
 const IdentifierInfo *Scope, const IdentifierInfo 
*Attr,
 const TargetInfo , const LangOptions ) 
{
@@ -27,7 +36,9 @@
   ScopeName == "omp")
 return (Name == "directive" || Name == "sequence") ? 1 : 0;
 
-#include "clang/Basic/AttrHasAttributeImpl.inc"
+  int res = hasAttributeImpl(Syntax, Name, ScopeName, Target, LangOpts);
+  if (res)
+return res;
 
   return 0;
 }


Index: clang/lib/Basic/Attributes.cpp
===
--- clang/lib/Basic/Attributes.cpp
+++ clang/lib/Basic/Attributes.cpp
@@ -4,6 +4,15 @@
 #include "clang/Basic/IdentifierTable.h"
 using namespace clang;
 
+static int hasAttributeImpl(AttributeCommonInfo::Syntax Syntax, StringRef Name,
+StringRef ScopeName, const TargetInfo ,
+const LangOptions ) {
+
+#include "clang/Basic/AttrHasAttributeImpl.inc"
+
+  return 0;
+}
+
 int clang::hasAttribute(AttributeCommonInfo::Syntax Syntax,
 const IdentifierInfo *Scope, const IdentifierInfo *Attr,
 const TargetInfo , const LangOptions ) {
@@ -27,7 +36,9 @@
   ScopeName == "omp")
 return (Name == "directive" || Name == "sequence") ? 1 : 0;
 
-#include "clang/Basic/AttrHasAttributeImpl.inc"
+  int res = hasAttributeImpl(Syntax, Name, ScopeName, Target, LangOpts);
+  if (res)
+return res;
 
   return 0;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145793: [clang][AST] Improve diagnostic for `nullptr` constexpr function pointer call

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:7673
+Info.FFDiag(Callee, diag::note_constexpr_null_callee)
+<< const_cast(Callee);
+return false;

Is the `const_cast` really necessary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145793

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

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: urnathan, iains.
aaron.ballman added a comment.

In D124351#4179510 , @cor3ntin wrote:

> In D124351#4178334 , @sammccall 
> wrote:
>
>> Sorry for the delay, extracting the repro from the build system seems about 
>> as much work as minimizing it :-)
>> F26749550: modrepo2.zip  - running 
>> repro.sh hits an assert for me on the main.cpp compile.
>>
>> (I suspect something much smaller is breaking the expected invariant, I just 
>> can't get a smaller crasher)
>
> @ChuanqiXu @erichkeane I started to look at this repro but I might need help, 
> it seems extremely weird (I failed to reduce it much further), and I don't 
> have a good grasp on how modules affect things

I'm trying to help out here as well, but in addition to @ChuanqiXu and 
@erichkeane, I wonder if @iains or @urnathan have ideas as well (they've both 
done modules work before).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124351

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

[PATCH] D83906: [CodeGen] Emit a call instruction instead of an invoke if the called llvm function is marked nounwind

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added a comment.

In D83906#4183453 , @hoy wrote:

> Wondering if we can come up with a way to tell the optimizer about that, 
> e.g., through a new module flag. When it comes to LTO, the selection of 
> linkonce_odr symbols should already been done and the optimizer may be able 
> to recompute the attributes based on pre-LTO attributes, or at least we can 
> allow IPO to one module only, which should still do a better job than FE does?

I don't think there's much point in passing anything to LTO. There are very few 
`linkonce_odr` symbols in LTO, since LTO has the advantage of an export list 
from the link. Symbols not on the export list are internalized (they're given 
local linkage).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83906

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

[PATCH] D143466: [clang][Interp] Fix initializing base class members

[Timm Bäder via Phabricator via cfe-commits]

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



Comment at: clang/test/AST/Interp/records.cpp:271
+  static_assert(d.B::a == 12);
+  static_assert(d.C::a == 0);
 };

These lines are added by https://reviews.llvm.org/D143480 now, but the tests 
are left commented since they require this patch as well.


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

https://reviews.llvm.org/D143466

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

[PATCH] D143466: [clang][Interp] Fix initializing base class members

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 504146.

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

https://reviews.llvm.org/D143466

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Record.cpp
  clang/lib/AST/Interp/Record.h
  clang/test/AST/Interp/cxx20.cpp
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -252,16 +252,23 @@
 
 #if __cplusplus >= 201703L
 namespace BaseInit {
+  class _A {public: int a;};
+  class _B : public _A {};
+  class _C : public _B {};
+
+  constexpr _C c{12};
+  constexpr const _B  = c;
+  static_assert(b.a == 12);
+
   class A {public: int a;};
   class B : public A {};
   class C : public A {};
   class D : public B, public C {};
 
-  // FIXME: Enable this once we support the initialization.
   // This initializes D::B::A::a and not D::C::A::a.
-  //constexpr D d{12};
-  //static_assert(d.B::a == 12);
-  //static_assert(d.C::a == 0);
+  constexpr D d{12};
+  static_assert(d.B::a == 12);
+  static_assert(d.C::a == 0);
 };
 #endif
 
Index: clang/test/AST/Interp/cxx20.cpp
===
--- clang/test/AST/Interp/cxx20.cpp
+++ clang/test/AST/Interp/cxx20.cpp
@@ -272,6 +272,69 @@
// ref-note {{in call to}}
 };
 
+namespace BaseInit {
+  struct Base {
+int a;
+  };
+
+  struct Intermediate : Base {
+int b;
+  };
+
+  struct Final : Intermediate {
+int c;
+
+constexpr Final(int a, int b, int c) : c(c) {}
+  };
+
+  static_assert(Final{1, 2, 3}.c == 3, ""); // OK
+  static_assert(Final{1, 2, 3}.a == 0, ""); // expected-error {{not an integral constant expression}} \
+// expected-note {{read of object outside its lifetime}} \
+// ref-error {{not an integral constant expression}} \
+// ref-note {{read of uninitialized object}}
+
+
+  struct Mixin  {
+int b;
+
+constexpr Mixin() = default;
+constexpr Mixin(int b) : b(b) {}
+  };
+
+  struct Final2 : Base, Mixin {
+int c;
+
+constexpr Final2(int a, int b, int c) : Mixin(b), c(c) {}
+constexpr Final2(int a, int b, int c, bool) : c(c) {}
+  };
+
+  static_assert(Final2{1, 2, 3}.c == 3, ""); // OK
+  static_assert(Final2{1, 2, 3}.b == 2, ""); // OK
+  static_assert(Final2{1, 2, 3}.a == 0, ""); // expected-error {{not an integral constant expression}} \
+ // expected-note {{read of object outside its lifetime}} \
+ // ref-error {{not an integral constant expression}} \
+ // ref-note {{read of uninitialized object}}
+
+
+  struct Mixin3  {
+int b;
+  };
+
+  struct Final3 : Base, Mixin3 {
+int c;
+
+constexpr Final3(int a, int b, int c) : c(c) { this->b = b; }
+constexpr Final3(int a, int b, int c, bool) : c(c) {}
+  };
+
+  static_assert(Final3{1, 2, 3}.c == 3, ""); // OK
+  static_assert(Final3{1, 2, 3}.b == 2, ""); // OK
+  static_assert(Final3{1, 2, 3}.a == 0, ""); // expected-error {{not an integral constant expression}} \
+ // expected-note {{read of object outside its lifetime}} \
+ // ref-error {{not an integral constant expression}} \
+ // ref-note {{read of uninitialized object}}
+};
+
 namespace Destructors {
 
   class Inc final {
Index: clang/lib/AST/Interp/Record.h
===
--- clang/lib/AST/Interp/Record.h
+++ clang/lib/AST/Interp/Record.h
@@ -61,6 +61,8 @@
   const Field *getField(const FieldDecl *FD) const;
   /// Returns a base descriptor.
   const Base *getBase(const RecordDecl *FD) const;
+  /// Returns a base descriptor.
+  const Base *getBase(QualType T) const;
   /// Returns a virtual base descriptor.
   const Base *getVirtualBase(const RecordDecl *RD) const;
   // Returns the destructor of the record, if any.
Index: clang/lib/AST/Interp/Record.cpp
===
--- clang/lib/AST/Interp/Record.cpp
+++ clang/lib/AST/Interp/Record.cpp
@@ -39,6 +39,16 @@
   return It->second;
 }
 
+const Record::Base *Record::getBase(QualType T) const {
+  if (!T->isRecordType())
+return nullptr;
+
+  const RecordDecl *RD = T->getAs()->getDecl();
+  if (auto It = BaseMap.find(RD); It != BaseMap.end())
+return It->second;
+  return nullptr;
+}
+
 const Record::Base *Record::getVirtualBase(const RecordDecl *FD) const {
   auto It = VirtualBaseMap.find(FD);
   assert(It != VirtualBaseMap.end() && "Missing virtual base");
Index: 

[PATCH] D144457: [clang][Interp] Handle global composite temporaries

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144457

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

[PATCH] D144878: __builtin_FILE_NAME()

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

> In terms of the encoding question I was asking, that's information we'll have 
> to figure out (CC @tahonermann and @cor3ntin for text encoding question). My 
> guess (which needs verification) is that we convert the file name from the 
> system encoding to UTF-8 internally, and this builtin will likely return 
> UTF-8 as a result. If that's correct, I think that behavior is reasonable, 
> but I've CCed some experts who can tell me all the things I forgot to 
> consider.

These things are bag of bytes - we likely can't promise _anything_ because 
there is no requirements of filenames to be encodable in _any_ encoding on some 
systems. In practice we can assume it's UTF-8 everywhere and convert to UTF-8 
on windows because that's the only string literal encoding clang supports for 
now. If we did support other literal encodings, we would need to answer that 
question. IE, we could try to convert, but the conversion might not work.




Comment at: clang/include/clang/Lex/Preprocessor.h:2859
+  template 
+  static void processPathToFilename(SmallString ,
+const PresumedLoc ,

`processPathToFileName` would be better for consistency



Comment at: clang/lib/Lex/PPMacroExpansion.cpp:1993-1994
+
+template 
+void Preprocessor::processPathToFilename(SmallString ,
+ const PresumedLoc ,

we probably want to pass a `SmallVectorImpl` here, instead of a 
`SmallString`, to avoid making that function a template
(look at `processPathForFileMacro` definition for example)



Comment at: clang/lib/Lex/PPMacroExpansion.cpp:2000-2001
+  // presumed location.
+  StringRef PLFileName = llvm::sys::path::filename(PLoc.getFilename());
+  if (PLFileName != "")
+FileName += PLFileName;




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

https://reviews.llvm.org/D144878

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

[PATCH] D145794: [clang-format] Recognize Verilog always blocks

[sstwcw via Phabricator via cfe-commits]

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

The small `Coverage` test was added because we added the space rule
about 2 at signs along with the rule about onely 1 of it.  We have not
fully covered covergroup yet.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145794

Files:
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTestVerilog.cpp

Index: clang/unittests/Format/FormatTestVerilog.cpp
===
--- clang/unittests/Format/FormatTestVerilog.cpp
+++ clang/unittests/Format/FormatTestVerilog.cpp
@@ -252,6 +252,12 @@
Style);
 }
 
+TEST_F(FormatTestVerilog, Coverage) {
+  verifyFormat("covergroup x\n"
+   "@@(begin x);\n"
+   "endgroup");
+}
+
 TEST_F(FormatTestVerilog, Declaration) {
   verifyFormat("wire mynet;");
   verifyFormat("wire mynet, mynet1;");
@@ -809,5 +815,50 @@
"  endtable\n"
"endprimitive");
 }
+
+TEST_F(FormatTestVerilog, StructuredProcedure) {
+  // Blocks should be indented correctly.
+  verifyFormat("initial begin\n"
+   "end");
+  verifyFormat("initial begin\n"
+   "  x <= x;\n"
+   "  x <= x;\n"
+   "end");
+  verifyFormat("initial\n"
+   "  x <= x;\n"
+   "x <= x;");
+  verifyFormat("always @(x) begin\n"
+   "end");
+  verifyFormat("always @(x) begin\n"
+   "  x <= x;\n"
+   "  x <= x;\n"
+   "end");
+  verifyFormat("always @(x)\n"
+   "  x <= x;\n"
+   "x <= x;");
+  // Various keywords.
+  verifyFormat("always @(x)\n"
+   "  x <= x;");
+  verifyFormat("always @(posedge x)\n"
+   "  x <= x;");
+  verifyFormat("always\n"
+   "  x <= x;");
+  verifyFormat("always @*\n"
+   "  x <= x;");
+  verifyFormat("always @(*)\n"
+   "  x <= x;");
+  verifyFormat("always_comb\n"
+   "  x <= x;");
+  verifyFormat("always_latch @(x)\n"
+   "  x <= x;");
+  verifyFormat("always_ff @(posedge x)\n"
+   "  x <= x;");
+  verifyFormat("initial\n"
+   "  x <= x;");
+  verifyFormat("final\n"
+   "  x <= x;");
+  verifyFormat("forever\n"
+   "  x <= x;");
+}
 } // namespace format
 } // end namespace clang
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -166,7 +166,7 @@
   FormatToken *parseIfThenElse(IfStmtKind *IfKind, bool KeepBraces = false);
   void parseTryCatch();
   void parseLoopBody(bool KeepBraces, bool WrapRightBrace);
-  void parseForOrWhileLoop();
+  void parseForOrWhileLoop(bool HasParens = true);
   void parseDoWhile();
   void parseLabel(bool LeftAlignLabel = false);
   void parseCaseLabel();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1389,6 +1389,11 @@
   }
 
   if (Style.isVerilog()) {
+if (Keywords.isVerilogStructuredProcedure(*FormatTok)) {
+  parseForOrWhileLoop(/*HasParens=*/false);
+  return;
+}
+
 // Skip things that can exist before keywords like 'if' and 'case'.
 while (true) {
   if (FormatTok->isOneOf(Keywords.kw_priority, Keywords.kw_unique,
@@ -2963,8 +2968,14 @@
 NestedTooDeep.pop_back();
 }
 
-void UnwrappedLineParser::parseForOrWhileLoop() {
-  assert(FormatTok->isOneOf(tok::kw_for, tok::kw_while, TT_ForEachMacro) &&
+void UnwrappedLineParser::parseForOrWhileLoop(bool HasParens) {
+  assert((FormatTok->isOneOf(tok::kw_for, tok::kw_while, TT_ForEachMacro) ||
+  (Style.isVerilog() &&
+   FormatTok->isOneOf(Keywords.kw_always, Keywords.kw_always_comb,
+  Keywords.kw_always_ff, Keywords.kw_always_latch,
+  Keywords.kw_final, Keywords.kw_initial,
+  Keywords.kw_foreach, Keywords.kw_forever,
+  Keywords.kw_repeat))) &&
  "'for', 'while' or foreach macro expected");
   const bool KeepBraces = !Style.RemoveBracesLLVM ||
   !FormatTok->isOneOf(tok::kw_for, tok::kw_while);
@@ -2975,8 +2986,11 @@
 nextToken();
   if (Style.isCpp() && FormatTok->is(tok::kw_co_await))
 nextToken();
-  if (FormatTok->is(tok::l_paren))
+  if (HasParens && FormatTok->is(tok::l_paren))
 

[PATCH] D144402: [clang] Move ParsedAttrInfo from Sema to Basic (NFC)

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144402

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

[PATCH] D145793: [clang][AST] Improve diagnostic for `nullptr` constexpr function pointer call

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet created this revision.
hazohelet added reviewers: tbaeder, cjdb, aaron.ballman.
Herald added a project: All.
hazohelet requested review of this revision.
Herald added a project: clang.

This patch improves diagnostic for clang constexpr evaluator by adding a check 
for `nullptr` in function pointer call evaluations.
This fixes https://github.com/llvm/llvm-project/issues/59872

ex.

  constexpr int foo(int (*bla)(void)) {
return bla();
  }
  
  static_assert(foo(nullptr) == 1);

BEFORE this patch, clang generates the following diagnostic for the code above

  :5:15: error: static assertion expression is not an integral constant 
expression
  static_assert(foo(nullptr) == 1);
^
  :2:10: note: subexpression not valid in a constant expression
return bla();
   ^
  :5:15: note: in call to 'foo(nullptr)'
  static_assert(foo(nullptr) == 1);
^
  1 error generated.

AFTER this patch, `subexpression not valid in a constant expression` note is 
replaced with `function pointer 'bla' evaluates to a null pointer`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145793

Files:
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/lib/AST/ExprConstant.cpp
  clang/test/SemaCXX/constant-expression-cxx11.cpp


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -279,7 +279,7 @@
   constexpr auto Select(int n) -> int (*)(int) {
 return n == 2 ?  : n == 3 ?  : n == 4 ?  : 0;
   }
-  constexpr int Apply(int (*F)(int), int n) { return F(n); } // expected-note 
{{subexpression}}
+  constexpr int Apply(int (*F)(int), int n) { return F(n); } // expected-note 
{{function pointer 'F' evaluates to a null pointer}}
 
   static_assert(1 + Apply(Select(4), 5) + Apply(Select(3), 7) == 42, "");
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7668,6 +7668,11 @@
 
   if (!CalleeLV.getLValueOffset().isZero())
 return Error(Callee);
+  if (CalleeLV.isNullPointer()) {
+Info.FFDiag(Callee, diag::note_constexpr_null_callee)
+<< const_cast(Callee);
+return false;
+  }
   FD = dyn_cast_or_null(
   CalleeLV.getLValueBase().dyn_cast());
   if (!FD)
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -127,6 +127,8 @@
   "access array element of|perform pointer arithmetic on|"
   "access real component of|"
   "access imaginary component of}0 null pointer">;
+def note_constexpr_null_callee : Note<
+  "function pointer '%0' evaluates to a null pointer">;
 def note_constexpr_function_param_value_unknown : Note<
   "function parameter %0 with unknown value cannot be used in a constant "
   "expression">;


Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -279,7 +279,7 @@
   constexpr auto Select(int n) -> int (*)(int) {
 return n == 2 ?  : n == 3 ?  : n == 4 ?  : 0;
   }
-  constexpr int Apply(int (*F)(int), int n) { return F(n); } // expected-note {{subexpression}}
+  constexpr int Apply(int (*F)(int), int n) { return F(n); } // expected-note {{function pointer 'F' evaluates to a null pointer}}
 
   static_assert(1 + Apply(Select(4), 5) + Apply(Select(3), 7) == 42, "");
 
Index: clang/lib/AST/ExprConstant.cpp
===
--- clang/lib/AST/ExprConstant.cpp
+++ clang/lib/AST/ExprConstant.cpp
@@ -7668,6 +7668,11 @@
 
   if (!CalleeLV.getLValueOffset().isZero())
 return Error(Callee);
+  if (CalleeLV.isNullPointer()) {
+Info.FFDiag(Callee, diag::note_constexpr_null_callee)
+<< const_cast(Callee);
+return false;
+  }
   FD = dyn_cast_or_null(
   CalleeLV.getLValueBase().dyn_cast());
   if (!FD)
Index: clang/include/clang/Basic/DiagnosticASTKinds.td
===
--- clang/include/clang/Basic/DiagnosticASTKinds.td
+++ clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -127,6 +127,8 @@
   "access array element of|perform pointer arithmetic on|"
   "access real component of|"
   "access imaginary component of}0 null pointer">;
+def note_constexpr_null_callee : Note<
+  "function pointer '%0' evaluates to a null pointer">;
 def note_constexpr_function_param_value_unknown : Note<
   "function parameter %0 with unknown value cannot be used in a constant "
   

[PATCH] D142420: [Flang] Add support to use LTO specific pipelines

[Tom Eccles via Phabricator via cfe-commits]

tblah added a comment.

In D142420#4080619 , @mnadeem wrote:

> We have tried full LTO on Aarch64 (without this patch) and have seen a few 
> 3-8% improvements in SPEC and a 100+% improvement in leslie3d.
> There were a couple of additional failures in SPEC that we have yet to 
> inspect but AFAICT no issue with respect to bitcode/assembly changes.

Hi @mnadeem, thanks for this patch. I'm having some trouble reproducing the 
improvement on leslie3d. Can you share more information about your 
configuration? Which flags did you use?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142420

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

[PATCH] D144402: [clang] Move ParsedAttrInfo from Sema to Basic (NFC)

[Anders Waldenborg via Phabricator via cfe-commits]

wanders added a comment.

In D144402#4184749 , @erichkeane 
wrote:

> I want to wait for @aaron.ballman to answer, but i think this is generally 
> OK.  Note the motivation is the ability to check this from 
> __has_cpp_attribute/__has_c_attribute/etc.

I did discuss this with @aaron.ballman in his office hours 13th Feb, so he 
should be aware of context.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144402

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

[PATCH] D144405: [clang][pp] Handle attributes defined by plugin in __has_attribute

[Anders Waldenborg via Phabricator via cfe-commits]

wanders added a comment.

In D144405#4184726 , @erichkeane 
wrote:

> Looks like this doesn't compile pre-commit, though no idea if that is a 
> patch-stack issue.  Other than test, patch looks fine.

Yeah, when uploading the new commit the stack was disorganized. So the build 
probably started before I got around organizing the stack in correct order.

So hopefully clears up when I fix the test  (clang-format and const thing)




Comment at: clang/test/Frontend/plugin-attribute-pp.cpp:1
+// RUN: split-file %s %t
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %t/has_attr.cpp | 
FileCheck %t/has_attr.cpp

erichkeane wrote:
> This split-file thing is weird, don't do that.  create 2 tests instead, OR 
> Just use -x c and -x c++ to change languages (you can use a separate macro to 
> get __has_c and __has_cpp_attribute right).
Right. That should be simpler. Will fix.

(it ended up this way because I first added a split to the existing 
`plugin-attribute.cpp` test with splits)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144405

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

[PATCH] D144878: __builtin_FILE_NAME()

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added subscribers: cor3ntin, tahonermann.
aaron.ballman added a comment.

In D144878#4183893 , @karapsinie 
wrote:

> In D144878#4172639 , @aaron.ballman 
> wrote:
>
>> In D144878#4171234 , @karapsinie 
>> wrote:
>>
>>> PTAL.
>>
>> Have you seen the comments on the GCC issue that @MaskRay filed? Is that 
>> something we should do as well? (It doesn't have to be part of this patch, 
>> but it'd be good to ensure we're collaborating with GCC so we get the same 
>> builtin functionality in this space.)
>>
>> There should be a release note and documentation for the new functionality. 
>> One thing to consider for the docs is explaining what's going on with text 
>> encodings (and perhaps that text applies to this entire class of 
>> file-related builtins). e.g., if the system code page is Shift-JIS does this 
>> builtin return the text in Shift-JIS or UTF-8 or something else?
>
> This is my first open-source commit.

Oh, awesome! Welcome!

> I don't know where or what to write.
> Maybe you will do it or tell me how to do it?

Happily!

Our release notes live in `clang/docs/ReleaseNotes.rst` and I'd probably add it 
under this heading specifically: 
https://github.com/llvm/llvm-project/blob/main/clang/docs/ReleaseNotes.rst#non-comprehensive-list-of-changes-in-this-release
Our documentation for builtins live in `clang/docs/LanguageExtensions.rst` and 
I'd probably add documentation under this heading specifically: 
https://github.com/llvm/llvm-project/blob/main/clang/docs/LanguageExtensions.rst#source-location-builtins

Both our documentation and our release notes are written in Sphinx (RST) which 
is a markdown format, more details of which can be found at: 
https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html

In terms of the encoding question I was asking, that's information we'll have 
to figure out (CC @tahonermann and @cor3ntin for text encoding question). My 
guess (which needs verification) is that we convert the file name from the 
system encoding to UTF-8 internally, and this builtin will likely return UTF-8 
as a result. If that's correct, I think that behavior is reasonable, but I've 
CCed some experts who can tell me all the things I forgot to consider.

As for speaking with GCC devs, I think the issue that @MaskRay filed 
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108978) got the discussion 
started. They asked if we should add `__builtin_BASE_FILE` at the same time -- 
is that something you think we should add and would be willing to work on as 
well? If so, then you should probably add a comment to that bug saying that's 
something we'd be fine adding so the GCC folks know the scope of the request. 
And if that's not something you think should be added, you should probably add 
a comment to that bug saying so and why. FWIW, you can see the implementation 
for `__BASE_FILE__` and `__FILE_NAME__` here: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Lex/PPMacroExpansion.cpp#L1537.


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

https://reviews.llvm.org/D144878

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

[PATCH] D144164: [clang][Interp] Handle PtrMemOps

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144164

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

[PATCH] D141472: [clang][Interp] Add function pointers

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D141472

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

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang-c/Index.h:365
+   */
+  int ExcludeDeclarationsFromPCH : 1;
+  /**

vedgy wrote:
> aaron.ballman wrote:
> > vedgy wrote:
> > > vedgy wrote:
> > > > aaron.ballman wrote:
> > > > > vedgy wrote:
> > > > > > Assigning `true` to `int : 1` bit-fields in C++ code produces a GCC 
> > > > > > warning:
> > > > > > ```
> > > > > > warning: overflow in conversion from ‘int’ to ‘signed char:1’ 
> > > > > > changes value from ‘1’ to ‘-1’ [-Woverflow]
> > > > > > ```
> > > > > > 
> > > > > > Following a suggestion in a comment to 
> > > > > > https://github.com/llvm/llvm-project/issues/53253, I replaced this 
> > > > > > `int` with `unsigned` and the warning disappeared. Same for `int 
> > > > > > DisplayDiagnostics : 1`. Should this type change be included in the 
> > > > > > upcoming `StorePreamblesInMemory` revision?
> > > > > > Assigning true to int : 1 bit-fields in C++ code produces a GCC 
> > > > > > warning:
> > > > > >
> > > > > > `warning: overflow in conversion from ‘int’ to ‘signed char:1’ 
> > > > > > changes value from ‘1’ to ‘-1’ [-Woverflow]`
> > > > > 
> > > > > Ugh, I forgot that the C standard allows that. (C2x 6.7.2.1p12: "A 
> > > > > bit-field member is interpreted as having a signed or unsigned 
> > > > > integer type consisting of the specified number of bits" -- GCC 
> > > > > decided to turn our `int` into `signed char` which is nice for 
> > > > > packing data together, but not as nice when it comes to boolean-like 
> > > > > bit-fields.)
> > > > > 
> > > > > > Should this type change be included in the upcoming 
> > > > > > StorePreamblesInMemory revision?
> > > > > 
> > > > > It'd probably be the cleanest to fix that separately. Given that it's 
> > > > > NFC and you don't have commit privileges, I can make the change on 
> > > > > your behalf and land it today if that's what you'd like.
> > > > Or should this change be done in a separate revision, on which the 
> > > > `StorePreamblesInMemory` would be based?
> > > > 
> > > > I also implemented two other changes to the `struct CXIndexOptions` 
> > > > (mostly documentation/comments). Should these all be in separate 
> > > > revisions or combined into one?
> > > Yes, I agree that such changes should be in separate commits. But I don't 
> > > want to burden you with committing them all separately. So if 4 is too 
> > > much, I can request the commit access for myself. If this burden is not 
> > > too heavy, I am fine with you making the change on my behalf.
> > No worries, this was a trivial one -- I landed it in 
> > dbde7cc17c3a5b6a35e5ec598ba7eaba6f75d90b, so you should be able to fetch 
> > and rebase on top of that.
> > I also implemented two other changes to the struct CXIndexOptions (mostly 
> > documentation/comments).
> Here they are: D145775, D145783.
Thank you, I've landed both of them.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[PATCH] D144402: [clang] Move ParsedAttrInfo from Sema to Basic (NFC)

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I want to wait for @aaron.ballman to answer, but i think this is generally OK.  
Note the motivation is the ability to check this from 
__has_cpp_attribute/__has_c_attribute/etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144402

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

[PATCH] D145783: Reserve unused bits in struct CXIndexOptions; NFC

[Aaron Ballman 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 rGa28e4821829e: Reserve unused bits in struct CXIndexOptions; 
NFC (authored by vedgy, committed by aaron.ballman).

Changed prior to commit:
  https://reviews.llvm.org/D145783?vs=504090=504129#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145783

Files:
  clang/include/clang-c/Index.h
  clang/tools/libclang/CIndex.cpp


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3724,6 +3724,13 @@
   // if (options->Size >= offsetof(CXIndexOptions, RecentlyAddedMember))
   //   do_something(options->RecentlyAddedMember);
 
+  // An exception: if a new option is small enough, it can be squeezed into the
+  // /*Reserved*/ bits in CXIndexOptions. Since the default value of each 
option
+  // is guaranteed to be 0 and the callers are advised to zero out the struct,
+  // programs built against older libclang versions would implicitly set the 
new
+  // options to default values, which should keep the behavior of previous
+  // libclang versions and thus be backward-compatible.
+
   // If options->Size > sizeof(CXIndexOptions), the user may have set an option
   // we can't handle, in which case we return nullptr to report failure.
   // Replace `!=` with `>` here to support older struct versions. `!=` has the
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -372,6 +372,8 @@
* \see clang_createIndex()
*/
   unsigned DisplayDiagnostics : 1;
+  unsigned /*Reserved*/ : 14;
+
   /**
* The path to a directory, in which to store temporary PCH files. If null or
* empty, the default system temporary directory is used. These PCH files are


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3724,6 +3724,13 @@
   // if (options->Size >= offsetof(CXIndexOptions, RecentlyAddedMember))
   //   do_something(options->RecentlyAddedMember);
 
+  // An exception: if a new option is small enough, it can be squeezed into the
+  // /*Reserved*/ bits in CXIndexOptions. Since the default value of each option
+  // is guaranteed to be 0 and the callers are advised to zero out the struct,
+  // programs built against older libclang versions would implicitly set the new
+  // options to default values, which should keep the behavior of previous
+  // libclang versions and thus be backward-compatible.
+
   // If options->Size > sizeof(CXIndexOptions), the user may have set an option
   // we can't handle, in which case we return nullptr to report failure.
   // Replace `!=` with `>` here to support older struct versions. `!=` has the
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -372,6 +372,8 @@
* \see clang_createIndex()
*/
   unsigned DisplayDiagnostics : 1;
+  unsigned /*Reserved*/ : 14;
+
   /**
* The path to a directory, in which to store temporary PCH files. If null or
* empty, the default system temporary directory is used. These PCH files are
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a28e482 - Reserve unused bits in struct CXIndexOptions; NFC

[Aaron Ballman via cfe-commits]

Author: Igor Kushnir
Date: 2023-03-10T09:26:10-05:00
New Revision: a28e4821829e2d5c68fc652f1db5db6e02284c3d

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

LOG: Reserve unused bits in struct CXIndexOptions; NFC

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

Added: 


Modified: 
clang/include/clang-c/Index.h
clang/tools/libclang/CIndex.cpp

Removed: 




diff  --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index 7c662c002c26b..7cd35e9b2a7d4 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -372,6 +372,8 @@ typedef struct CXIndexOptions {
* \see clang_createIndex()
*/
   unsigned DisplayDiagnostics : 1;
+  unsigned /*Reserved*/ : 14;
+
   /**
* The path to a directory, in which to store temporary PCH files. If null or
* empty, the default system temporary directory is used. These PCH files are

diff  --git a/clang/tools/libclang/CIndex.cpp b/clang/tools/libclang/CIndex.cpp
index 959646707d8f4..8a23679beeb6d 100644
--- a/clang/tools/libclang/CIndex.cpp
+++ b/clang/tools/libclang/CIndex.cpp
@@ -3724,6 +3724,13 @@ CXIndex clang_createIndexWithOptions(const 
CXIndexOptions *options) {
   // if (options->Size >= offsetof(CXIndexOptions, RecentlyAddedMember))
   //   do_something(options->RecentlyAddedMember);
 
+  // An exception: if a new option is small enough, it can be squeezed into the
+  // /*Reserved*/ bits in CXIndexOptions. Since the default value of each 
option
+  // is guaranteed to be 0 and the callers are advised to zero out the struct,
+  // programs built against older libclang versions would implicitly set the 
new
+  // options to default values, which should keep the behavior of previous
+  // libclang versions and thus be backward-compatible.
+
   // If options->Size > sizeof(CXIndexOptions), the user may have set an option
   // we can't handle, in which case we return nullptr to report failure.
   // Replace `!=` with `>` here to support older struct versions. `!=` has the



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

[PATCH] D145783: Reserve unused bits in struct CXIndexOptions; NFC

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145783

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

[PATCH] D145775: Improve documentation of CXIndexOptions; NFC

[Aaron Ballman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG00e52588f02e: Improve documentation of CXIndexOptions; NFC 
(authored by vedgy, committed by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145775

Files:
  clang/include/clang-c/Index.h


Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -329,8 +329,8 @@
  * Index initialization options.
  *
  * 0 is the default value of each member of this struct except for Size.
- * Initialize the struct in one of the following two ways to avoid adapting 
code
- * each time a new member is added to it:
+ * Initialize the struct in one of the following three ways to avoid adapting
+ * code each time a new member is added to it:
  * \code
  * CXIndexOptions Opts;
  * memset(, 0, sizeof(Opts));
@@ -340,6 +340,11 @@
  * \code
  * CXIndexOptions Opts = { sizeof(CXIndexOptions) };
  * \endcode
+ * or to prevent the -Wmissing-field-initializers warning for the above 
version:
+ * \code
+ * CXIndexOptions Opts{};
+ * Opts.Size = sizeof(CXIndexOptions);
+ * \endcode
  */
 typedef struct CXIndexOptions {
   /**


Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -329,8 +329,8 @@
  * Index initialization options.
  *
  * 0 is the default value of each member of this struct except for Size.
- * Initialize the struct in one of the following two ways to avoid adapting code
- * each time a new member is added to it:
+ * Initialize the struct in one of the following three ways to avoid adapting
+ * code each time a new member is added to it:
  * \code
  * CXIndexOptions Opts;
  * memset(, 0, sizeof(Opts));
@@ -340,6 +340,11 @@
  * \code
  * CXIndexOptions Opts = { sizeof(CXIndexOptions) };
  * \endcode
+ * or to prevent the -Wmissing-field-initializers warning for the above version:
+ * \code
+ * CXIndexOptions Opts{};
+ * Opts.Size = sizeof(CXIndexOptions);
+ * \endcode
  */
 typedef struct CXIndexOptions {
   /**
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 00e5258 - Improve documentation of CXIndexOptions; NFC

[Aaron Ballman via cfe-commits]

Author: Igor Kushnir
Date: 2023-03-10T09:22:09-05:00
New Revision: 00e52588f02e2d9c7fcd1cfb4703deaca1b32a8d

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

LOG: Improve documentation of CXIndexOptions; NFC

Document one more alternative way to initialize struct CXIndexOptions,
which is used in LibclangSetPreambleStoragePathTest since
df8f8f76207df40dca11c9c0c2328d6b3dfba9ca because the previous
initialization approach causes compiler warnings.

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

Added: 


Modified: 
clang/include/clang-c/Index.h

Removed: 




diff  --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index b986d29235903..7c662c002c26b 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -329,8 +329,8 @@ typedef enum {
  * Index initialization options.
  *
  * 0 is the default value of each member of this struct except for Size.
- * Initialize the struct in one of the following two ways to avoid adapting 
code
- * each time a new member is added to it:
+ * Initialize the struct in one of the following three ways to avoid adapting
+ * code each time a new member is added to it:
  * \code
  * CXIndexOptions Opts;
  * memset(, 0, sizeof(Opts));
@@ -340,6 +340,11 @@ typedef enum {
  * \code
  * CXIndexOptions Opts = { sizeof(CXIndexOptions) };
  * \endcode
+ * or to prevent the -Wmissing-field-initializers warning for the above 
version:
+ * \code
+ * CXIndexOptions Opts{};
+ * Opts.Size = sizeof(CXIndexOptions);
+ * \endcode
  */
 typedef struct CXIndexOptions {
   /**



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

[PATCH] D144405: [clang][pp] Handle attributes defined by plugin in __has_attribute

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

Looks like this doesn't compile pre-commit, though no idea if that is a 
patch-stack issue.  Other than test, patch looks fine.




Comment at: clang/test/Frontend/plugin-attribute-pp.cpp:1
+// RUN: split-file %s %t
+// RUN: %clang -fplugin=%llvmshlibdir/Attribute%pluginext -E %t/has_attr.cpp | 
FileCheck %t/has_attr.cpp

This split-file thing is weird, don't do that.  create 2 tests instead, OR Just 
use -x c and -x c++ to change languages (you can use a separate macro to get 
__has_c and __has_cpp_attribute right).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144405

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

[clang] ecf6508 - [Clang] Convert some tests to opaque pointers (NFC)

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2023-03-10T15:17:28+01:00
New Revision: ecf65087eee7529936c9c93d76e02b06816ba930

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

LOG: [Clang] Convert some tests to opaque pointers (NFC)

Added: 


Modified: 
clang/test/Modules/codegen-opt.test
clang/test/Modules/codegen.test
clang/test/Modules/initializers.cpp
clang/test/Modules/objc-initializer.m

Removed: 




diff  --git a/clang/test/Modules/codegen-opt.test 
b/clang/test/Modules/codegen-opt.test
index 6536611bd307..eafd47ae1ee3 100644
--- a/clang/test/Modules/codegen-opt.test
+++ b/clang/test/Modules/codegen-opt.test
@@ -1,16 +1,16 @@
 RUN: rm -rf %t
 REQUIRES: x86-registered-target
 
-RUN: %clang_cc1 -no-opaque-pointers -triple=x86_64-linux-gnu -fmodules-codegen 
-x c++ -fmodules -emit-module -fmodule-name=foo 
%S/Inputs/codegen-opt/foo.modulemap -o %t/foo.pcm
-RUN: %clang_cc1 -no-opaque-pointers -triple=x86_64-linux-gnu -fmodules-codegen 
-x c++ -fmodules -emit-module -fmodule-name=bar 
%S/Inputs/codegen-opt/bar.modulemap -o %t/bar.pcm -fmodule-file=%t/foo.pcm
+RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++ -fmodules 
-emit-module -fmodule-name=foo %S/Inputs/codegen-opt/foo.modulemap -o %t/foo.pcm
+RUN: %clang_cc1 -triple=x86_64-linux-gnu -fmodules-codegen -x c++ -fmodules 
-emit-module -fmodule-name=bar %S/Inputs/codegen-opt/bar.modulemap -o 
%t/bar.pcm -fmodule-file=%t/foo.pcm
 
-RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm -o - 
%t/foo.pcm | FileCheck --check-prefix=FOO %s
-RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm -o - 
%t/bar.pcm -fmodule-file=%t/foo.pcm | FileCheck --check-prefix=BAR-CMN 
--check-prefix=BAR %s
-RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm -o - 
-fmodules -fmodule-file=%t/foo.pcm -fmodule-file=%t/bar.pcm 
%S/Inputs/codegen-opt/use.cpp | FileCheck --check-prefix=USE-CMN 
--check-prefix=USE %s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %t/foo.pcm | 
FileCheck --check-prefix=FOO %s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - %t/bar.pcm 
-fmodule-file=%t/foo.pcm | FileCheck --check-prefix=BAR-CMN --check-prefix=BAR 
%s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - -fmodules 
-fmodule-file=%t/foo.pcm -fmodule-file=%t/bar.pcm %S/Inputs/codegen-opt/use.cpp 
| FileCheck --check-prefix=USE-CMN --check-prefix=USE %s
 
-RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm -o - 
-O2 -disable-llvm-passes %t/foo.pcm | FileCheck --check-prefix=FOO %s
-RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm -o - 
-O2 -disable-llvm-passes %t/bar.pcm -fmodule-file=%t/foo.pcm | FileCheck 
--check-prefix=BAR-CMN --check-prefix=BAR-OPT %s
-RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-linux-gnu -emit-llvm -o - 
-O2 -disable-llvm-passes -fmodules -fmodule-file=%t/foo.pcm 
-fmodule-file=%t/bar.pcm %S/Inputs/codegen-opt/use.cpp | FileCheck 
--check-prefix=USE-CMN --check-prefix=USE-OPT %s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - -O2 
-disable-llvm-passes %t/foo.pcm | FileCheck --check-prefix=FOO %s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - -O2 
-disable-llvm-passes %t/bar.pcm -fmodule-file=%t/foo.pcm | FileCheck 
--check-prefix=BAR-CMN --check-prefix=BAR-OPT %s
+RUN: %clang_cc1 -triple x86_64-linux-gnu -emit-llvm -o - -O2 
-disable-llvm-passes -fmodules -fmodule-file=%t/foo.pcm 
-fmodule-file=%t/bar.pcm %S/Inputs/codegen-opt/use.cpp | FileCheck 
--check-prefix=USE-CMN --check-prefix=USE-OPT %s
 
 FOO-NOT: comdat
 FOO: $_Z3foov = comdat any
@@ -22,7 +22,7 @@ FOO: define{{.*}} void @_Z7foo_extv()
 FOO-NOT: {{define|declare}}
 FOO: define weak_odr void @_Z3foov() #{{[0-9]+}} comdat
 FOO-NOT: {{define|declare}}
-FOO: declare void @_Z2f1Ri(i32*
+FOO: declare void @_Z2f1Ri(ptr
 FOO-NOT: {{define|declare}}
 
 Internal functions are not modularly code generated - they are
@@ -49,7 +49,7 @@ BAR: declare void @_Z3foov()
 Include all the available_externally definitions required for bar (foo -> f2)
 BAR-OPT: define available_externally void @_Z3foov()
 BAR-CMN-NOT: {{define|declare}}
-BAR-OPT: declare void @_Z2f1Ri(i32*
+BAR-OPT: declare void @_Z2f1Ri(ptr
 BAR-OPT-NOT: {{define|declare}}
 BAR-OPT: define internal void @_ZL2f2v()
 BAR-OPT-NOT: {{define|declare}}
@@ -65,7 +65,7 @@ USE-OPT: define available_externally void @_Z3barv()
 USE-CMN-NOT: {{define|declare}}
 USE-OPT: define available_externally void @_Z3foov()
 USE-OPT-NOT: {{define|declare}}
-USE-OPT: declare void @_Z2f1Ri(i32*
+USE-OPT: declare void @_Z2f1Ri(ptr
 USE-OPT-NOT: {{define|declare}}
 USE-OPT: define internal void @_ZL2f2v()
 USE-OPT-NOT: {{define|declare}}

diff  --git a/clang/test/Modules/codegen.test 

[PATCH] D145769: [clang] Extract ParsedAttrInfo::hasSpelling method (NFC)

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.



Comment at: clang/include/clang/Basic/ParsedAttrInfo.h:92
+  /// Check if this attribute has specified spelling.
+  bool hasSpelling(AttributeCommonInfo::Syntax Syntax, StringRef Name) {
+return llvm::any_of(Spellings, [&] (const Spelling ) {




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145769

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

[PATCH] D145769: [clang] Extract ParsedAttrInfo::hasSpelling method (NFC)

[Erich Keane via Phabricator via cfe-commits]

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

Looks like you have a clang-format fix to do (see pre-commit CI), else, LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145769

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

[PATCH] D145720: [clang-tidy] Finish cppcoreguidelines-avoid-capturing-lambda-coroutines check

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG17d403f66443: [clang-tidy] Finish 
cppcoreguidelines-avoid-capturing-lambda-coroutines check (authored by 
PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145720

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/Inputs/system/coroutines.h
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++20-or-later %s cppcoreguidelines-avoid-capturing-lambda-coroutines %t -- -- \
-// RUN:   -isystem %S/../readability/Inputs/identifier-naming/system
+// RUN: %check_clang_tidy -std=c++20-or-later %s cppcoreguidelines-avoid-capturing-lambda-coroutines %t -- -- -isystem %S/Inputs/system
 
 #include 
 
@@ -7,23 +6,23 @@
 int v;
 
 [&] () -> task { int y = v; co_return; };
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
 [=] () -> task { int y = v; co_return; };
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
 [v] () -> task { co_return; };
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
 [] () -> task { co_return; };
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
 [y=v] () -> task { co_return; };
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
 [y{v}] () -> task { co_return; };
-// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
 }
 
 struct S {
 void m() {
 [this] () -> task { co_return; };
-// CHECK-MESSAGES: [[@LINE-1]]:9: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
 }
 };
 
@@ -32,4 +31,6 @@
 [] () -> task { co_return; };
 [&] () -> task { co_return; };
 [=] () -> task { co_return; };
+
+[]{++v;}();
 }
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/Inputs/system/coroutines.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/Inputs/system/coroutines.h
@@ -0,0 +1,32 @@
+#pragma once
+
+namespace std {
+
+template 
+struct coroutine_traits {
+  using 

[PATCH] D137514: [clang-tidy] add check for capturing lambda coroutines

[Piotr Zegar via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG54178fc6161a: [clang-tidy] add check for capturing lambda 
coroutines (authored by dotnwat, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137514

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy -std=c++20-or-later %s cppcoreguidelines-avoid-capturing-lambda-coroutines %t -- -- \
+// RUN:   -isystem %S/../readability/Inputs/identifier-naming/system
+
+#include 
+
+void Caught() {
+int v;
+
+[&] () -> task { int y = v; co_return; };
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+[=] () -> task { int y = v; co_return; };
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+[v] () -> task { co_return; };
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+[] () -> task { co_return; };
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+[y=v] () -> task { co_return; };
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+[y{v}] () -> task { co_return; };
+// CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+}
+
+struct S {
+void m() {
+[this] () -> task { co_return; };
+// CHECK-MESSAGES: [[@LINE-1]]:9: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+}
+};
+
+void Safe() {
+int v;
+[] () -> task { co_return; };
+[&] () -> task { co_return; };
+[=] () -> task { co_return; };
+}
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
@@ -180,6 +180,7 @@
`concurrency-mt-unsafe `_,
`concurrency-thread-canceltype-asynchronous `_,
`cppcoreguidelines-avoid-capture-default-when-capturing-this `_,
+   `cppcoreguidelines-avoid-capturing-lambda-coroutines `_, "Yes"
`cppcoreguidelines-avoid-const-or-ref-data-members `_,
`cppcoreguidelines-avoid-do-while `_,
`cppcoreguidelines-avoid-goto `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst
@@ -0,0 +1,31 @@
+.. title:: clang-tidy - cppcoreguidelines-avoid-capturing-lambda-coroutines
+
+cppcoreguidelines-avoid-capturing-lambda-coroutines
+===
+
+Warns if a capturing lambda is a coroutine. For example:
+
+.. code-block:: c++
+
+   int c;
+
+   [c] () -> task { co_return; };
+   [&] () -> task { int y = c; co_return; };
+   [=] () -> task { int y = c; co_return; };
+
+   struct s {
+   void m() {
+   [this] () -> task { co_return; };
+   }
+   };
+
+All of the cases above will trigger the warning. However, implicit captures
+do not trigger the warning unless the body of the lambda uses the capture.
+For example, the following do not trigger the warning.
+
+.. code-block:: c++
+
+   int c;
+
+   [&] () -> task { co_return; };
+   [=] () -> task { co_return; };
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -120,6 +120,13 @@
   Checks that 

[clang-tools-extra] 17d403f - [clang-tidy] Finish cppcoreguidelines-avoid-capturing-lambda-coroutines check

[Piotr Zegar via cfe-commits]

Author: Piotr Zegar
Date: 2023-03-10T14:06:50Z
New Revision: 17d403f6644337e3e099e6dcb7b057fce11e65a5

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

LOG: [clang-tidy] Finish cppcoreguidelines-avoid-capturing-lambda-coroutines 
check

This commit implements check for CP.51 C++ Core Guidelines

Depends on D137514

Reviewed By: ChuanqiXu

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

Added: 

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/Inputs/system/coroutines.h

Modified: 

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
index de9c9eaa7f407..3c99831f9d640 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
@@ -12,34 +12,34 @@
 
 using namespace clang::ast_matchers;
 
-namespace clang {
-namespace tidy {
-namespace cppcoreguidelines {
+namespace clang::tidy::cppcoreguidelines {
+
+namespace {
+AST_MATCHER(LambdaExpr, hasCoroutineBody) {
+  const Stmt *Body = Node.getBody();
+  return Body != nullptr && CoroutineBodyStmt::classof(Body);
+}
+
+AST_MATCHER(LambdaExpr, hasCaptures) { return Node.capture_size() != 0U; }
+} // namespace
 
 void AvoidCapturingLambdaCoroutinesCheck::registerMatchers(
 MatchFinder *Finder) {
-  Finder->addMatcher(lambdaExpr().bind("lambda"), this);
+  Finder->addMatcher(
+  lambdaExpr(hasCaptures(), hasCoroutineBody()).bind("lambda"), this);
+}
+
+bool AvoidCapturingLambdaCoroutinesCheck::isLanguageVersionSupported(
+const LangOptions ) const {
+  return LangOpts.CPlusPlus20;
 }
 
 void AvoidCapturingLambdaCoroutinesCheck::check(
 const MatchFinder::MatchResult ) {
-  const auto *Lambda = Result.Nodes.getNodeAs("lambda");
-  if (!Lambda) {
-return;
-  }
-
-  const auto *Body = dyn_cast(Lambda->getBody());
-  if (!Body) {
-return;
-  }
-
-  if (Lambda->captures().empty()) {
-return;
-  }
-
-  diag(Lambda->getBeginLoc(), "found capturing coroutine lambda");
+  const auto *MatchedLambda = Result.Nodes.getNodeAs("lambda");
+  diag(MatchedLambda->getExprLoc(),
+   "coroutine lambda may cause use-after-free, avoid captures or ensure "
+   "lambda closure object has guaranteed lifetime");
 }
 
-} // namespace cppcoreguidelines
-} // namespace tidy
-} // namespace clang
+} // namespace clang::tidy::cppcoreguidelines

diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
index 63895f3354414..b32e2662b5fba 100644
--- 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
@@ -11,26 +11,23 @@
 
 #include "../ClangTidyCheck.h"
 
-namespace clang {
-namespace tidy {
-namespace cppcoreguidelines {
+namespace clang::tidy::cppcoreguidelines {
 
-/// The normal usage of captures in lambdas are problematic when the lambda is 
a
-/// coroutine because the captures are destroyed after the first suspension
-/// point. Using the captures after this point is a use-after-free issue.
+/// Flags C++20 coroutine lambdas with non-empty capture lists that may cause
+/// use-after-free errors and suggests avoiding captures or ensuring the lambda
+/// closure object has a guaranteed lifetime.
 ///
 /// For the user-facing documentation see:
-/// 
http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-capturing-lambda-coroutines.html
+/// 
https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.html
 class AvoidCapturingLambdaCoroutinesCheck : public ClangTidyCheck {
 public:
   AvoidCapturingLambdaCoroutinesCheck(StringRef Name, ClangTidyContext 
*Context)
   : ClangTidyCheck(Name, Context) {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult ) override;
+  bool isLanguageVersionSupported(const LangOptions ) const override;
 };
 
-} // namespace 

[clang-tools-extra] 54178fc - [clang-tidy] add check for capturing lambda coroutines

[Piotr Zegar via cfe-commits]

Author: Noah Watkins
Date: 2023-03-10T14:06:50Z
New Revision: 54178fc6161a5856bea608dce0d726787a3b71c3

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

LOG: [clang-tidy] add check for capturing lambda coroutines

Signed-off-by: Noah Watkins 

Reviewed By: ChuanqiXu

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

Added: 

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h

clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp

Modified: 
clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt

clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
new file mode 100644
index 0..de9c9eaa7f407
--- /dev/null
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
@@ -0,0 +1,45 @@
+//===--- AvoidCapturingLambdaCoroutinesCheck.cpp - clang-tidy 
-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AvoidCapturingLambdaCoroutinesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+void AvoidCapturingLambdaCoroutinesCheck::registerMatchers(
+MatchFinder *Finder) {
+  Finder->addMatcher(lambdaExpr().bind("lambda"), this);
+}
+
+void AvoidCapturingLambdaCoroutinesCheck::check(
+const MatchFinder::MatchResult ) {
+  const auto *Lambda = Result.Nodes.getNodeAs("lambda");
+  if (!Lambda) {
+return;
+  }
+
+  const auto *Body = dyn_cast(Lambda->getBody());
+  if (!Body) {
+return;
+  }
+
+  if (Lambda->captures().empty()) {
+return;
+  }
+
+  diag(Lambda->getBeginLoc(), "found capturing coroutine lambda");
+}
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang

diff  --git 
a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
new file mode 100644
index 0..63895f3354414
--- /dev/null
+++ 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
@@ -0,0 +1,36 @@
+//===--- AvoidCapturingLambdaCoroutinesCheck.h - clang-tidy -*- 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 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H
+#define 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace cppcoreguidelines {
+
+/// The normal usage of captures in lambdas are problematic when the lambda is 
a
+/// coroutine because the captures are destroyed after the first suspension
+/// point. Using the captures after this point is a use-after-free issue.
+///
+/// For the user-facing documentation see:
+/// 
http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-capturing-lambda-coroutines.html
+class AvoidCapturingLambdaCoroutinesCheck : public ClangTidyCheck {
+public:
+  AvoidCapturingLambdaCoroutinesCheck(StringRef Name, ClangTidyContext 
*Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult ) override;
+};
+
+} // namespace cppcoreguidelines
+} // namespace tidy
+} // namespace clang
+
+#endif // 
LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H

diff  --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt 
b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
index 

[PATCH] D145730: [clang-tidy] Fix readability-redundant-string-cstr for smart pointer #576705

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

Thanks, and sorry about the erroneous `[PATCH]` in the summary. :(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145730

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

[PATCH] D145765: Add __builtin_set_flt_rounds

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for this! You should also add a release note and some documentation 
for the new builtin.




Comment at: clang/test/CodeGen/builtin_set_flt_rounds.c:1-4
+// RUN: %clang_cc1  -triple x86_64-gnu-linux %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1  -triple x86_64-windows-msvc %s -emit-llvm -o - | FileCheck 
%s
+// RUN: %clang_cc1  -triple aarch64-gnu-linux %s -emit-llvm -o - | FileCheck %s
+// RUN: %clang_cc1  -triple aarch64-windows-msvc %s -emit-llvm -o - | 
FileCheck %s

Minor cleanup.



Comment at: clang/test/CodeGen/builtin_set_flt_rounds.c:5-7
+void test_builtin_set_flt_rounds() {
+  __builtin_set_flt_rounds(1);
+  // CHECK: call void @llvm.set.rounding(i32 1)

I think you should also add some Sema tests for correct and misuse of the 
builtin:
```
__builtin_set_flt_rounds(1); // OK

struct S { int a; } s;
__builtin_set_flt_rounds(s); // This should diagnose, right?

__builtin_set_flt_rounds(1.0f); // Should this implicitly convert or is this an 
error?
```
and that sema test can additionally test what happens when you call the builtin 
on an unsupported architecture.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145765

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

[clang-tools-extra] d4b45b9 - [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Piotr Zegar via cfe-commits]

Author: Florian Humblot
Date: 2023-03-10T13:56:28Z
New Revision: d4b45b93fa56622ee5f935c066f43b15b856f3b8

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

LOG: [clang-tidy] Change readability-magic-numbers to allow numbers in type 
aliases.

This commit introduces an option to the readability-magic-values checker.
The option defaults to false so that the behavior of the checker doesn't change 
unless specifically enabled.
These commits are supposed to fix 
https://github.com/llvm/llvm-project/issues/61259

Reviewed By: PiotrZSL

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
index 64940ede8e1c7..ca81b6e3c6e61 100644
--- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
@@ -14,6 +14,8 @@
 #include "MagicNumbersCheck.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTTypeTraits.h"
+#include "clang/AST/Type.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/STLExtras.h"
 #include 
@@ -42,6 +44,18 @@ static bool isUsedToInitializeAConstant(const 
MatchFinder::MatchResult ,
   });
 }
 
+static bool isUsedToDefineATypeAlias(const MatchFinder::MatchResult ,
+ const DynTypedNode ) {
+
+  if (Node.get() || Node.get())
+return true;
+
+  return llvm::any_of(Result.Context->getParents(Node),
+  [](const DynTypedNode ) {
+return isUsedToDefineATypeAlias(Result, Parent);
+  });
+}
+
 static bool isUsedToDefineABitField(const MatchFinder::MatchResult ,
 const DynTypedNode ) {
   const auto *AsFieldDecl = Node.get();
@@ -66,6 +80,7 @@ MagicNumbersCheck::MagicNumbersCheck(StringRef Name, 
ClangTidyContext *Context)
   IgnoreBitFieldsWidths(Options.get("IgnoreBitFieldsWidths", true)),
   IgnorePowersOf2IntegerValues(
   Options.get("IgnorePowersOf2IntegerValues", false)),
+  IgnoreTypeAliases(Options.get("IgnoreTypeAliases", false)),
   RawIgnoredIntegerValues(
   Options.get("IgnoredIntegerValues", DefaultIgnoredIntegerValues)),
   RawIgnoredFloatingPointValues(Options.get(
@@ -114,6 +129,7 @@ void 
MagicNumbersCheck::storeOptions(ClangTidyOptions::OptionMap ) {
   Options.store(Opts, "IgnoreBitFieldsWidths", IgnoreBitFieldsWidths);
   Options.store(Opts, "IgnorePowersOf2IntegerValues",
 IgnorePowersOf2IntegerValues);
+  Options.store(Opts, "IgnoreTypeAliases", IgnoreTypeAliases);
   Options.store(Opts, "IgnoredIntegerValues", RawIgnoredIntegerValues);
   Options.store(Opts, "IgnoredFloatingPointValues",
 RawIgnoredFloatingPointValues);
@@ -137,10 +153,13 @@ bool MagicNumbersCheck::isConstant(const 
MatchFinder::MatchResult ,
const Expr ) const {
   return llvm::any_of(
   Result.Context->getParents(ExprResult),
-  [](const DynTypedNode ) {
+  [this, ](const DynTypedNode ) {
 if (isUsedToInitializeAConstant(Result, Parent))
   return true;
 
+if (IgnoreTypeAliases && isUsedToDefineATypeAlias(Result, Parent))
+  return true;
+
 // Ignore this instance, because this matches an
 // expanded class enumeration value.
 if (Parent.get() &&

diff  --git a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h 
b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
index ba39f17f4d500..39e9baea3adc8 100644
--- a/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
@@ -84,6 +84,7 @@ class MagicNumbersCheck : public ClangTidyCheck {
   const bool IgnoreAllFloatingPointValues;
   const bool IgnoreBitFieldsWidths;
   const bool IgnorePowersOf2IntegerValues;
+  const bool IgnoreTypeAliases;
   const StringRef RawIgnoredIntegerValues;
   const StringRef RawIgnoredFloatingPointValues;
 

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 426b92ab950cc..e8a74f8402460 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,11 @@ Changes 

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4b45b93fa56: [clang-tidy] Change readability-magic-numbers 
to allow numbers in type aliases. (authored by florianhumblot, committed by 
PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145778

Files:
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
@@ -3,7 +3,8 @@
 // RUN:  [{key: readability-magic-numbers.IgnoredIntegerValues, value: "0;1;2;10;100;"}, \
 // RUN:   {key: readability-magic-numbers.IgnoredFloatingPointValues, value: "3.14;2.71828;9.81;1.0;101.0;0x1.2p3"}, \
 // RUN:   {key: readability-magic-numbers.IgnoreBitFieldsWidths, value: false}, \
-// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}]}' \
+// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}, \
+// RUN:   {key: readability-magic-numbers.IgnoreTypeAliases, value: false}]}' \
 // RUN: --
 
 template 
@@ -96,6 +97,10 @@
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 };
 
+using NumberInTypeAlias = ValueBucket;
+// CHECK-MESSAGES: :[[@LINE-1]]:44: warning: 25 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+typedef ValueBucket NumberInTypedef;
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 243 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 
 /*
  * Clean code
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -24,6 +24,16 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   struct OtherType {
+  CustomType container;
+   }
+   CustomType values;
+
double circleArea = 3.1415926535 * radius * radius;
 
double totalCharge = 1.08 * itemPrice;
@@ -40,6 +50,19 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   const size_t NUMBER_OF_ELEMENTS = 30;
+   using containerType = CustomType;
+   
+   struct OtherType {
+  containerType container;
+   }
+   containerType values;
+
double circleArea = M_PI * radius * radius;
 
const double TAX_RATE = 0.08;  // or make it variable and read from a file
@@ -116,3 +139,8 @@
Boolean value indicating whether to accept magic numbers as bit field widths
without warning. This is useful for example for register definitions which
are generated from hardware specifications. Default value is `true`.
+
+.. option:: IgnoreTypeAliases
+
+   Boolean value indicating whether to accept magic numbers in ``typedef`` or
+   ``using`` declarations. Default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,11 @@
   ` check. Basic support
   for bit-field and integer members as a loop variable or upper limit were added.
 
+- Improved :doc:`readability-magic-numbers 
+  ` check, now allows for
+  magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
+  the new ``IgnoreTypeAliases`` option is set to true.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
===
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
@@ -84,6 +84,7 @@
   const bool IgnoreAllFloatingPointValues;
   const bool IgnoreBitFieldsWidths;
   const bool IgnorePowersOf2IntegerValues;
+  const bool IgnoreTypeAliases;
   const StringRef RawIgnoredIntegerValues;
   const StringRef RawIgnoredFloatingPointValues;
 
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
@@ -14,6 +14,8 @@
 #include 

[PATCH] D145730: [clang-tidy] Fix readability-redundant-string-cstr for smart pointer #576705

[Piotr Zegar via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9c8bc090a0c2: [clang-tidy] Fix 
readability-redundant-string-cstr for smart pointer #576705 (authored by 
mikecrowe, committed by PiotrZSL).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145730

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -1,6 +1,12 @@
 // RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- 
-isystem %clang_tidy_headers
 #include 
 
+template 
+struct iterator {
+  T *operator->();
+  T *();
+};
+
 namespace llvm {
 struct StringRef {
   StringRef(const char *p);
@@ -202,6 +208,31 @@
   m1p2(s.c_str());  
 }
 
+// Test for overloaded operator->
+void it(iterator i)
+{
+  std::string tmp;
+  tmp = i->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i;{{$}}
+
+  // An unlikely situation and the outcome is not ideal, but at least the fix 
doesn't generate broken code.
+  tmp = i.operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i.operator->();{{$}}
+
+  // The fix contains an unnecessary set of parentheses, but these have no 
effect.
+  iterator *pi = 
+  tmp = (*pi)->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *(*pi);{{$}}
+
+  // An unlikely situation, but at least the fix doesn't generate broken code.
+  tmp = pi->operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *pi->operator->();{{$}}
+}
+
 namespace PR45286 {
 struct Foo {
   void func(const std::string &) {}
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -52,6 +52,10 @@
 
   if (Text.empty())
 return std::string();
+
+  // Remove remaining '->' from overloaded operator call
+  Text.consume_back("->");
+
   // Add leading '*'.
   if (needParensAfterUnaryOperator(ExprNode)) {
 return (llvm::Twine("*(") + Text + ")").str();


Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -1,6 +1,12 @@
 // RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- -isystem %clang_tidy_headers
 #include 
 
+template 
+struct iterator {
+  T *operator->();
+  T *();
+};
+
 namespace llvm {
 struct StringRef {
   StringRef(const char *p);
@@ -202,6 +208,31 @@
   m1p2(s.c_str());  
 }
 
+// Test for overloaded operator->
+void it(iterator i)
+{
+  std::string tmp;
+  tmp = i->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i;{{$}}
+
+  // An unlikely situation and the outcome is not ideal, but at least the fix doesn't generate broken code.
+  tmp = i.operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i.operator->();{{$}}
+
+  // The fix contains an unnecessary set of parentheses, but these have no effect.
+  iterator *pi = 
+  tmp = (*pi)->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *(*pi);{{$}}
+
+  // An unlikely situation, but at least the fix doesn't generate broken code.
+  tmp = pi->operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *pi->operator->();{{$}}
+}
+
 namespace PR45286 {
 struct Foo {
   void func(const std::string &) {}
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===
--- 

[clang-tools-extra] 9c8bc09 - [clang-tidy] Fix readability-redundant-string-cstr for smart pointer #576705

[Piotr Zegar via cfe-commits]

Author: Mike Crowe
Date: 2023-03-10T13:52:49Z
New Revision: 9c8bc090a0c291efb41cc4b5c51ea0c340961ad1

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

LOG: [clang-tidy] Fix readability-redundant-string-cstr for smart pointer 
#576705

Fix the readability-redundant-string-cstr check to correctly replace
calls to c_str() via an overloaded operator-> (such as from an
iterator.)

Previously, the fix for `i->c_str()` would be `*i->`. Using consume_back
to remove any trailing `->` results in the correct `*i`.

Add some lit check test cases too.

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

Reviewed By: PiotrZSL

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp

clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp 
b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
index 960d5abcc912e..1b21f70f0bf64 100644
--- a/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -52,6 +52,10 @@ formatDereference(const 
ast_matchers::MatchFinder::MatchResult ,
 
   if (Text.empty())
 return std::string();
+
+  // Remove remaining '->' from overloaded operator call
+  Text.consume_back("->");
+
   // Add leading '*'.
   if (needParensAfterUnaryOperator(ExprNode)) {
 return (llvm::Twine("*(") + Text + ")").str();

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
index 2b6d290a4aff6..5dc138e81eeec 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -1,6 +1,12 @@
 // RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- 
-isystem %clang_tidy_headers
 #include 
 
+template 
+struct iterator {
+  T *operator->();
+  T *();
+};
+
 namespace llvm {
 struct StringRef {
   StringRef(const char *p);
@@ -202,6 +208,31 @@ void m1(std::string&&) {
   m1p2(s.c_str());  
 }
 
+// Test for overloaded operator->
+void it(iterator i)
+{
+  std::string tmp;
+  tmp = i->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i;{{$}}
+
+  // An unlikely situation and the outcome is not ideal, but at least the fix 
doesn't generate broken code.
+  tmp = i.operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i.operator->();{{$}}
+
+  // The fix contains an unnecessary set of parentheses, but these have no 
effect.
+  iterator *pi = 
+  tmp = (*pi)->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *(*pi);{{$}}
+
+  // An unlikely situation, but at least the fix doesn't generate broken code.
+  tmp = pi->operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *pi->operator->();{{$}}
+}
+
 namespace PR45286 {
 struct Foo {
   void func(const std::string &) {}



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

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Florian Humblot via Phabricator via cfe-commits]

florianhumblot added a comment.

In D145778#4184611 , @PiotrZSL wrote:

> What should I put as commit author ? 
> I'm asking because this is diff, and commit author looks to be attached to it.

"Florian Humblot "  should be good :)


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

https://reviews.llvm.org/D145778

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

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

What should I put as commit author ? 
I'm asking because this is diff, and commit author looks to be attached to it.


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

https://reviews.llvm.org/D145778

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added a comment.

I have no objections about doing this revert.


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

https://reviews.llvm.org/D145704

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D145704#4184584 , @aaron.ballman 
wrote:

> LGTM! If you don't hear otherwise from @kito-cheng, @bjope, @Wilco1 in the 
> next ~4 hours, feel free to land.

Great! Thanks @aaron.ballman  and @tstellar.


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

https://reviews.llvm.org/D145704

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

[PATCH] D143480: [clang][Interp] Fix derived-to-base casts for >1 levels

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D143480

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

[PATCH] D145704: Revert "Set FLT_EVAL_METHOD to -1 when fast-math is enabled."

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM! If you don't hear otherwise from @kito-cheng, @bjope, @Wilco1 in the next 
~4 hours, feel free to land.


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

https://reviews.llvm.org/D145704

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

[PATCH] D145730: [PATCH] [clang-tidy] Fix readability-redundant-string-cstr for smart pointer #576705

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

Thanks for the speedy re-review. I don't have commit permission, so please can 
you land this if you are happy to do so?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145730

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

[PATCH] D130303: Fix include order in CXType.cpp

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130303#4183081 , @collinbaker 
wrote:

> All done. Again, I don't have commit access so someone else will need to land 
> this.

I can do that for you -- what name and email address would you like me to use 
for patch attribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130303

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

[PATCH] D145730: [clang-tidy] readability-redundant-string-cstr for smart pointer #576705

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe updated this revision to Diff 504108.
mikecrowe marked an inline comment as done.
mikecrowe added a comment.

Fix comment in code as suggested by PiotrZSL. Improve lit check tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145730

Files:
  clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -1,6 +1,12 @@
 // RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- 
-isystem %clang_tidy_headers
 #include 
 
+template 
+struct iterator {
+  T *operator->();
+  T *();
+};
+
 namespace llvm {
 struct StringRef {
   StringRef(const char *p);
@@ -202,6 +208,31 @@
   m1p2(s.c_str());  
 }
 
+// Test for overloaded operator->
+void it(iterator i)
+{
+  std::string tmp;
+  tmp = i->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i;{{$}}
+
+  // An unlikely situation and the outcome is not ideal, but at least the fix 
doesn't generate broken code.
+  tmp = i.operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i.operator->();{{$}}
+
+  // The fix contains an unnecessary set of parentheses, but these have no 
effect.
+  iterator *pi = 
+  tmp = (*pi)->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *(*pi);{{$}}
+
+  // An unlikely situation, but at least the fix doesn't generate broken code.
+  tmp = pi->operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' 
[readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *pi->operator->();{{$}}
+}
+
 namespace PR45286 {
 struct Foo {
   void func(const std::string &) {}
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
@@ -52,6 +52,10 @@
 
   if (Text.empty())
 return std::string();
+
+  // Remove remaining '->' from overloaded operator call
+  Text.consume_back("->");
+
   // Add leading '*'.
   if (needParensAfterUnaryOperator(ExprNode)) {
 return (llvm::Twine("*(") + Text + ")").str();


Index: clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/redundant-string-cstr.cpp
@@ -1,6 +1,12 @@
 // RUN: %check_clang_tidy %s readability-redundant-string-cstr %t -- -- -isystem %clang_tidy_headers
 #include 
 
+template 
+struct iterator {
+  T *operator->();
+  T *();
+};
+
 namespace llvm {
 struct StringRef {
   StringRef(const char *p);
@@ -202,6 +208,31 @@
   m1p2(s.c_str());  
 }
 
+// Test for overloaded operator->
+void it(iterator i)
+{
+  std::string tmp;
+  tmp = i->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i;{{$}}
+
+  // An unlikely situation and the outcome is not ideal, but at least the fix doesn't generate broken code.
+  tmp = i.operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *i.operator->();{{$}}
+
+  // The fix contains an unnecessary set of parentheses, but these have no effect.
+  iterator *pi = 
+  tmp = (*pi)->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *(*pi);{{$}}
+
+  // An unlikely situation, but at least the fix doesn't generate broken code.
+  tmp = pi->operator->()->c_str();
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: redundant call to 'c_str' [readability-redundant-string-cstr]
+  // CHECK-FIXES: {{^  }}tmp = *pi->operator->();{{$}}
+}
+
 namespace PR45286 {
 struct Foo {
   void func(const std::string &) {}
Index: clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp
+++ 

[PATCH] D145730: [clang-tidy] readability-redundant-string-cstr for smart pointer #576705

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Code is correct, simply this check uses more manual string manipulation.
In theory there could be some more issues if `operator ->` wouldn't return 
pointer, but a class with additional `operator ->`, but that's legacy 
independent issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145730

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

[clang] 1e07091 - Revert "[Modules] Remove unnecessary check when generating name lookup table in ASTWriter"

[Krasimir Georgiev via cfe-commits]

Author: Krasimir Georgiev
Date: 2023-03-10T14:14:55+01:00
New Revision: 1e0709167f5edd330889f51bb203c458bdb5e359

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

LOG: Revert "[Modules] Remove unnecessary check when generating name lookup 
table in ASTWriter"

This reverts commit db987b9589be1eb604fcb74c85b410469e31485f.

We're seeing failures in modules-enabled builds from within stdlib after
this commit. Errors look like:

In module '...stl_cc_library':
...optional:560:38: error: 'std::__optional_copy_assign_base::__optional_copy_assign_base' from module '...optional' is not
present in definition of 'std::__optional_copy_assign_base' in module '...optional'
using __optional_move_base<_Tp>::__optional_move_base;

In module '...stl_cc_library':
...optional:771:11: error: no matching constructor for initialization of 
'__optional_move_assign_base'
: __base(in_place, _VSTD::forward<_Up>(__v)) {}
  ^  ~~
llvm-project/clang/include/clang/Basic/CodeGenOptions.h:448:57: note: in 
instantiation of function template specialization 'std::optional::optional' requested here
  std::optional DiagnosticsHotnessThreshold = 0;

I don't have a self-contained reproducer at this point. I'm hoping that
we may be able to share more information about these issues later, if
necessary.

Added: 


Modified: 
clang/include/clang/Serialization/ASTWriter.h
clang/lib/Serialization/ASTWriter.cpp

Removed: 
clang/test/Modules/pr61065.cppm



diff  --git a/clang/include/clang/Serialization/ASTWriter.h 
b/clang/include/clang/Serialization/ASTWriter.h
index d31fa38b93825..09ee1744e8945 100644
--- a/clang/include/clang/Serialization/ASTWriter.h
+++ b/clang/include/clang/Serialization/ASTWriter.h
@@ -514,6 +514,7 @@ class ASTWriter : public ASTDeserializationListener,
   void WriteTypeAbbrevs();
   void WriteType(QualType T);
 
+  bool isLookupResultExternal(StoredDeclsList , DeclContext *DC);
   bool isLookupResultEntirelyExternal(StoredDeclsList , DeclContext 
*DC);
 
   void GenerateNameLookupTable(const DeclContext *DC,

diff  --git a/clang/lib/Serialization/ASTWriter.cpp 
b/clang/lib/Serialization/ASTWriter.cpp
index 5f8f5d38932a5..c1afdeb6007db 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3849,6 +3849,12 @@ class ASTDeclContextNameLookupTrait {
 
 } // namespace
 
+bool ASTWriter::isLookupResultExternal(StoredDeclsList ,
+   DeclContext *DC) {
+  return Result.hasExternalDecls() &&
+ DC->hasNeedToReconcileExternalVisibleStorage();
+}
+
 bool ASTWriter::isLookupResultEntirelyExternal(StoredDeclsList ,
DeclContext *DC) {
   for (auto *D : Result.getLookupResult())
@@ -3891,7 +3897,8 @@ ASTWriter::GenerateNameLookupTable(const DeclContext 
*ConstDC,
 // don't need to write an entry for the name at all. If we can't
 // write out a lookup set without performing more deserialization,
 // just skip this entry.
-if (isLookupResultEntirelyExternal(Result, DC))
+if (isLookupResultExternal(Result, DC) &&
+isLookupResultEntirelyExternal(Result, DC))
   continue;
 
 // We also skip empty results. If any of the results could be external and

diff  --git a/clang/test/Modules/pr61065.cppm b/clang/test/Modules/pr61065.cppm
deleted file mode 100644
index 44fa3679974ad..0
--- a/clang/test/Modules/pr61065.cppm
+++ /dev/null
@@ -1,55 +0,0 @@
-// From https://github.com/llvm/llvm-project/issues/61065
-// RUN: rm -rf %t
-// RUN: mkdir -p %t
-// RUN: split-file %s %t
-//
-// RUN: %clang_cc1 -std=c++20 %t/a.cppm -emit-module-interface -o %t/a.pcm
-// RUN: %clang_cc1 -std=c++20 %t/b.cppm -emit-module-interface -o %t/b.pcm \
-// RUN: -fprebuilt-module-path=%t
-// RUN: %clang_cc1 -std=c++20 %t/c.cppm -emit-module-interface -o %t/c.pcm \
-// RUN: -fprebuilt-module-path=%t
-// RUN: %clang_cc1 -std=c++20 %t/d.cpp -fsyntax-only -verify 
-fprebuilt-module-path=%t
-
-//--- a.cppm
-export module a;
-
-struct base {
-   base(int) {}
-};
-
-export struct a : base {
-   using base::base;
-};
-
-//--- b.cppm
-export module b;
-
-import a;
-
-a b() {
-   return a(1);
-}
-
-//--- c.cppm
-export module c;
-
-import a;
-import b;
-
-struct noncopyable {
-   noncopyable(noncopyable const &) = delete;
-noncopyable() = default;
-};
-
-export struct c {
-   noncopyable c0;
-   a c1 = 43;
-c() = default;
-};
-
-//--- d.cpp
-// expected-no-diagnostics
-import c;
-void d() {
-c _;
-}



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

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Florian Humblot via Phabricator via cfe-commits]

florianhumblot added a comment.

In D145778#4184530 , @PiotrZSL wrote:

> LGTM

Thanks for the reviews @PiotrZSL ! 
Is one review enough or does the second reviewer also need to approve these 
changes?

If this one is enough, could I ask you (or someone else) to commit the change 
since I don't have commit access? :)


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

https://reviews.llvm.org/D145778

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

[PATCH] D145730: [clang-tidy] readability-redundant-string-cstr for smart pointer #576705

[Mike Crowe via Phabricator via cfe-commits]

mikecrowe added a comment.

In D145730#4182949 , @PiotrZSL wrote:

> Also consider reducing commit message, instead just copying issue description.
> Simple description about issue would be sufficient.

TBH, I wasn't expecting this change to be accepted. I was merely hoping that it 
would cause someone to give me a hint as to how to fix it properly. That's why 
I included so much information in the commit message. I will shorten the commit 
message.

> No functional issues, so LGTM.

Thanks for the review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145730

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

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Piotr Zegar via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D145778

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

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Florian Humblot via Phabricator via cfe-commits]

florianhumblot updated this revision to Diff 504097.
florianhumblot marked 2 inline comments as done.
florianhumblot added a comment.

Generate diff with full context and updated the example to use the default 
settings of the readabiliy-magic-numbers checker


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

https://reviews.llvm.org/D145778

Files:
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
@@ -3,7 +3,8 @@
 // RUN:  [{key: readability-magic-numbers.IgnoredIntegerValues, value: "0;1;2;10;100;"}, \
 // RUN:   {key: readability-magic-numbers.IgnoredFloatingPointValues, value: "3.14;2.71828;9.81;1.0;101.0;0x1.2p3"}, \
 // RUN:   {key: readability-magic-numbers.IgnoreBitFieldsWidths, value: false}, \
-// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}]}' \
+// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}, \
+// RUN:   {key: readability-magic-numbers.IgnoreTypeAliases, value: false}]}' \
 // RUN: --
 
 template 
@@ -96,6 +97,10 @@
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 };
 
+using NumberInTypeAlias = ValueBucket;
+// CHECK-MESSAGES: :[[@LINE-1]]:44: warning: 25 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+typedef ValueBucket NumberInTypedef;
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 243 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 
 /*
  * Clean code
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -24,6 +24,16 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   struct OtherType {
+  CustomType container;
+   }
+   CustomType values;
+
double circleArea = 3.1415926535 * radius * radius;
 
double totalCharge = 1.08 * itemPrice;
@@ -40,6 +50,19 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   const size_t NUMBER_OF_ELEMENTS = 30;
+   using containerType = CustomType;
+   
+   struct OtherType {
+  containerType container;
+   }
+   containerType values;
+
double circleArea = M_PI * radius * radius;
 
const double TAX_RATE = 0.08;  // or make it variable and read from a file
@@ -116,3 +139,8 @@
Boolean value indicating whether to accept magic numbers as bit field widths
without warning. This is useful for example for register definitions which
are generated from hardware specifications. Default value is `true`.
+
+.. option:: IgnoreTypeAliases
+
+   Boolean value indicating whether to accept magic numbers in ``typedef`` or
+   ``using`` declarations. Default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,11 @@
   ` check. Basic support
   for bit-field and integer members as a loop variable or upper limit were added.
 
+- Improved :doc:`readability-magic-numbers 
+  ` check, now allows for
+  magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
+  the new ``IgnoreTypeAliases`` option is set to true.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
===
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
@@ -84,6 +84,7 @@
   const bool IgnoreAllFloatingPointValues;
   const bool IgnoreBitFieldsWidths;
   const bool IgnorePowersOf2IntegerValues;
+  const bool IgnoreTypeAliases;
   const StringRef RawIgnoredIntegerValues;
   const StringRef RawIgnoredFloatingPointValues;
 
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
@@ -14,6 +14,8 @@
 #include "MagicNumbersCheck.h"

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

"Context not available." - use arcanist (just merge all local commits into 
one), or generate diff with full context (diff -U 999).




Comment at: 
clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst:58
+
+   using containerType = CustomType;
+   struct OtherType {

example put under "Example with magic values refactored:" should be by default 
as (+-):
```
using containerType = CustomType;
```

as this is required by default.


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

https://reviews.llvm.org/D145778

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

[PATCH] D145780: [clang-tidy] Introduce option to allow magic numbers in type aliases

[Florian Humblot via Phabricator via cfe-commits]

florianhumblot abandoned this revision.
florianhumblot added a comment.

Accidentally opened this one with arcanist. The changes have been moved to 
https://reviews.llvm.org/D145778


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145780

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

[PATCH] D144638: [lit] Detect Inconsistent File Access Times

[James Henderson via Phabricator via cfe-commits]

jhenderson added a comment.

Related to my inline comment, your changes will result in some tests being 
disabled on Windows that weren't before (at least one of the tests pass for me 
even on my machine where atime is disabled). I think we need to understand why 
these tests pass on Windows before losing test coverage by disabling them.




Comment at: llvm/utils/lit/lit/llvm/config.py:172
+# Windows: the last access time is disabled by default in the OS, and
+# the check below is written in terms of unix utilities (touch, ls),
+# which will not work on this platform.

As mentioned, I don't think this is a fair statement: many tests use `touch` or 
`ls`, and work fine on Windows. That being said, the top Google result for how 
to detect if access time is enabled on Windows yields the following command: 
`fsutil behavior query disablelastaccess`, which prints some information about 
its state that could be easily queried in python to give a more correct answer 
for Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D145782: [clangd] Don't ignore headers with IWYU export pragmas for unused-include analysis.

[Haojian Wu via Phabricator via cfe-commits]

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

It appears that there is an issue with this patch, so please hold on for review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145782

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

[PATCH] D143418: [libclang] Add API to override preamble storage path

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added inline comments.



Comment at: clang/include/clang-c/Index.h:365
+   */
+  int ExcludeDeclarationsFromPCH : 1;
+  /**

aaron.ballman wrote:
> vedgy wrote:
> > vedgy wrote:
> > > aaron.ballman wrote:
> > > > vedgy wrote:
> > > > > Assigning `true` to `int : 1` bit-fields in C++ code produces a GCC 
> > > > > warning:
> > > > > ```
> > > > > warning: overflow in conversion from ‘int’ to ‘signed char:1’ changes 
> > > > > value from ‘1’ to ‘-1’ [-Woverflow]
> > > > > ```
> > > > > 
> > > > > Following a suggestion in a comment to 
> > > > > https://github.com/llvm/llvm-project/issues/53253, I replaced this 
> > > > > `int` with `unsigned` and the warning disappeared. Same for `int 
> > > > > DisplayDiagnostics : 1`. Should this type change be included in the 
> > > > > upcoming `StorePreamblesInMemory` revision?
> > > > > Assigning true to int : 1 bit-fields in C++ code produces a GCC 
> > > > > warning:
> > > > >
> > > > > `warning: overflow in conversion from ‘int’ to ‘signed char:1’ 
> > > > > changes value from ‘1’ to ‘-1’ [-Woverflow]`
> > > > 
> > > > Ugh, I forgot that the C standard allows that. (C2x 6.7.2.1p12: "A 
> > > > bit-field member is interpreted as having a signed or unsigned integer 
> > > > type consisting of the specified number of bits" -- GCC decided to turn 
> > > > our `int` into `signed char` which is nice for packing data together, 
> > > > but not as nice when it comes to boolean-like bit-fields.)
> > > > 
> > > > > Should this type change be included in the upcoming 
> > > > > StorePreamblesInMemory revision?
> > > > 
> > > > It'd probably be the cleanest to fix that separately. Given that it's 
> > > > NFC and you don't have commit privileges, I can make the change on your 
> > > > behalf and land it today if that's what you'd like.
> > > Or should this change be done in a separate revision, on which the 
> > > `StorePreamblesInMemory` would be based?
> > > 
> > > I also implemented two other changes to the `struct CXIndexOptions` 
> > > (mostly documentation/comments). Should these all be in separate 
> > > revisions or combined into one?
> > Yes, I agree that such changes should be in separate commits. But I don't 
> > want to burden you with committing them all separately. So if 4 is too 
> > much, I can request the commit access for myself. If this burden is not too 
> > heavy, I am fine with you making the change on my behalf.
> No worries, this was a trivial one -- I landed it in 
> dbde7cc17c3a5b6a35e5ec598ba7eaba6f75d90b, so you should be able to fetch and 
> rebase on top of that.
> I also implemented two other changes to the struct CXIndexOptions (mostly 
> documentation/comments).
Here they are: D145775, D145783.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143418

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

[PATCH] D145783: Reserve unused bits in struct CXIndexOptions; NFC

[Igor Kushnir via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145783

Files:
  clang/include/clang-c/Index.h
  clang/tools/libclang/CIndex.cpp


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3724,6 +3724,13 @@
   // if (options->Size >= offsetof(CXIndexOptions, RecentlyAddedMember))
   //   do_something(options->RecentlyAddedMember);
 
+  // An exception: if a new option is small enough, it can be squeezed into the
+  // /*Reserved*/ bits in CXIndexOptions. Since the default value of each 
option
+  // is guaranteed to be 0 and the callers are advised to zero out the struct,
+  // programs built against older libclang versions would implicitly set the 
new
+  // options to default values, which should keep the behavior of previous
+  // libclang versions and thus be backward-compatible.
+
   // If options->Size > sizeof(CXIndexOptions), the user may have set an option
   // we can't handle, in which case we return nullptr to report failure.
   // Replace `!=` with `>` here to support older struct versions. `!=` has the
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -367,6 +367,7 @@
* \see clang_createIndex()
*/
   unsigned DisplayDiagnostics : 1;
+  unsigned /*Reserved*/ : 14;
   /**
* The path to a directory, in which to store temporary PCH files. If null or
* empty, the default system temporary directory is used. These PCH files are


Index: clang/tools/libclang/CIndex.cpp
===
--- clang/tools/libclang/CIndex.cpp
+++ clang/tools/libclang/CIndex.cpp
@@ -3724,6 +3724,13 @@
   // if (options->Size >= offsetof(CXIndexOptions, RecentlyAddedMember))
   //   do_something(options->RecentlyAddedMember);
 
+  // An exception: if a new option is small enough, it can be squeezed into the
+  // /*Reserved*/ bits in CXIndexOptions. Since the default value of each option
+  // is guaranteed to be 0 and the callers are advised to zero out the struct,
+  // programs built against older libclang versions would implicitly set the new
+  // options to default values, which should keep the behavior of previous
+  // libclang versions and thus be backward-compatible.
+
   // If options->Size > sizeof(CXIndexOptions), the user may have set an option
   // we can't handle, in which case we return nullptr to report failure.
   // Replace `!=` with `>` here to support older struct versions. `!=` has the
Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -367,6 +367,7 @@
* \see clang_createIndex()
*/
   unsigned DisplayDiagnostics : 1;
+  unsigned /*Reserved*/ : 14;
   /**
* The path to a directory, in which to store temporary PCH files. If null or
* empty, the default system temporary directory is used. These PCH files are
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Florian Humblot via Phabricator via cfe-commits]

florianhumblot updated this revision to Diff 504089.
florianhumblot edited the summary of this revision.
florianhumblot added a comment.

Made it so that the proposed change doesn't change the default behavior of the 
check.
Introduce an option to allow enabling the exclusion.


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

https://reviews.llvm.org/D145778

Files:
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-type-aliases.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
@@ -3,7 +3,8 @@
 // RUN:  [{key: readability-magic-numbers.IgnoredIntegerValues, value: "0;1;2;10;100;"}, \
 // RUN:   {key: readability-magic-numbers.IgnoredFloatingPointValues, value: "3.14;2.71828;9.81;1.0;101.0;0x1.2p3"}, \
 // RUN:   {key: readability-magic-numbers.IgnoreBitFieldsWidths, value: false}, \
-// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}]}' \
+// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}, \
+// RUN:   {key: readability-magic-numbers.IgnoreTypeAliases, value: false}]}' \
 // RUN: --
 
 template 
@@ -96,6 +97,10 @@
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 };
 
+using NumberInTypeAlias = ValueBucket;
+// CHECK-MESSAGES: :[[@LINE-1]]:44: warning: 25 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+typedef ValueBucket NumberInTypedef;
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 243 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 
 /*
  * Clean code
Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-type-aliases.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-type-aliases.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s readability-magic-numbers %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-magic-numbers.IgnoreTypeAliases, value: true}]}' \
+// RUN: --
+
+template 
+struct ValueBucket {
+  T value[V];
+};
+
+int BadGlobalInt = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 5 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+/**
+ * With the flag enabled these should produce no warning
+ */
+using NumberInTypeAlias = ValueBucket;
+typedef ValueBucket NumberInTypedef;
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -24,6 +24,16 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   struct OtherType {
+  CustomType container;
+   }
+   CustomType values;
+
double circleArea = 3.1415926535 * radius * radius;
 
double totalCharge = 1.08 * itemPrice;
@@ -40,6 +50,17 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   using containerType = CustomType;
+   struct OtherType {
+  containerType container;
+   }
+   containerType values;
+
double circleArea = M_PI * radius * radius;
 
const double TAX_RATE = 0.08;  // or make it variable and read from a file
@@ -116,3 +137,8 @@
Boolean value indicating whether to accept magic numbers as bit field widths
without warning. This is useful for example for register definitions which
are generated from hardware specifications. Default value is `true`.
+
+.. option:: IgnoreTypeAliases
+
+   Boolean value indicating whether to accept magic numbers in ``typedef`` or
+   ``using`` declarations. Default value is `false`.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,11 @@
   ` check. Basic support
   for bit-field and integer members as a loop variable or upper limit were added.
 
+- Improved :doc:`readability-magic-numbers 
+  ` check, now allows for
+  magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
+  the new ``IgnoreTypeAliases`` option is set to true.

[PATCH] D145782: [clangd] Don't ignore headers with IWYU export pragmas for unused-include analysis.

[Haojian Wu via Phabricator via cfe-commits]

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

This check was introduced due to the the lack support of the old implementation.
The new include-cleaner-library based implementation doesn't have this
limitation.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145782

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -300,10 +300,9 @@
   ParsedAST AST = TU.build();
 
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
-  // FIXME: This is not correct: foo.h is unused but is not diagnosed as such
-  // because we ignore headers with IWYU export pragmas for now.
   IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
-  EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
+  EXPECT_THAT(Findings.UnusedIncludes,
+  ElementsAre(Pointee(writtenInclusion("\"foo.h\"";
 }
 
 TEST(IncludeCleaner, NoDiagsForObjC) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -104,10 +104,6 @@
   }
   assert(Inc.HeaderID);
   auto HID = static_cast(*Inc.HeaderID);
-  // FIXME: Ignore the headers with IWYU export pragmas for now, remove this
-  // check when we have more precise tracking of exported headers.
-  if (AST.getIncludeStructure().hasIWYUExport(HID))
-return false;
   auto FE = AST.getSourceManager().getFileManager().getFileRef(
   AST.getIncludeStructure().getRealPath(HID));
   assert(FE);


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -300,10 +300,9 @@
   ParsedAST AST = TU.build();
 
   EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
-  // FIXME: This is not correct: foo.h is unused but is not diagnosed as such
-  // because we ignore headers with IWYU export pragmas for now.
   IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
-  EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
+  EXPECT_THAT(Findings.UnusedIncludes,
+  ElementsAre(Pointee(writtenInclusion("\"foo.h\"";
 }
 
 TEST(IncludeCleaner, NoDiagsForObjC) {
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -104,10 +104,6 @@
   }
   assert(Inc.HeaderID);
   auto HID = static_cast(*Inc.HeaderID);
-  // FIXME: Ignore the headers with IWYU export pragmas for now, remove this
-  // check when we have more precise tracking of exported headers.
-  if (AST.getIncludeStructure().hasIWYUExport(HID))
-return false;
   auto FE = AST.getSourceManager().getFileManager().getFileRef(
   AST.getIncludeStructure().getRealPath(HID));
   assert(FE);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145781: [AArch64] Don't #define __ARM_FP when there's no FPU.

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham created this revision.
simon_tatham added reviewers: lenary, tmatheson, DavidSpickett, efriedma.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
simon_tatham requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

On some R-profile CPUs, leaving out the FPU is an option. Clang will
accept `-march=armv8-r+nofp`, but it's currently not possible to find
out via the preprocessor whether it's in that mode (e.g. to change or
disable inline asm statements in your code).

The __ARM_FP macro, which has a bit set for each size of floating
point number supported by the hardware, is the natural thing to test.
But Clang was defining it unconditionally on AArch64. Now it checks
for FP support before defining it at all.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145781

Files:
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/AArch64.h
  clang/test/Preprocessor/aarch64-target-features.c


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -341,6 +341,10 @@
 // CHECK-MARCH-2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"-fp-armv8" "-target-feature" "-neon" "-target-feature" "-crc" 
"-target-feature" "-crypto"
 // CHECK-MARCH-3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" 
"-neon"
 
+// While we're checking +nofp, also make sure it stops defining __ARM_FP
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8-r+nofp -x c -E -dM 
%s -o - | FileCheck -check-prefix=CHECK-NOFP %s
+// CHECK-NOFP-NOT: #define __ARM_FP{{ }}
+
 // Check +sm4:
 //
 // RUN: %clang -target aarch64 -march=armv8.2a+sm4 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-SM4 %s
Index: clang/lib/Basic/Targets/AArch64.h
===
--- clang/lib/Basic/Targets/AArch64.h
+++ clang/lib/Basic/Targets/AArch64.h
@@ -26,7 +26,7 @@
   static const TargetInfo::GCCRegAlias GCCRegAliases[];
   static const char *const GCCRegNames[];
 
-  enum FPUModeEnum { FPUMode, NeonMode = (1 << 0), SveMode = (1 << 1) };
+  enum FPUModeEnum { FPUMode = (1 << 0), NeonMode = (1 << 1), SveMode = (1 << 
2) };
 
   unsigned FPU = FPUMode;
   bool HasCRC = false;
@@ -73,6 +73,7 @@
   bool HasWFxT = false;
   bool HasJSCVT = false;
   bool HasFCMA = false;
+  bool HasNoFP = false;
   bool HasNoNeon = false;
   bool HasNoSVE = false;
   bool HasFMV = true;
Index: clang/lib/Basic/Targets/AArch64.cpp
===
--- clang/lib/Basic/Targets/AArch64.cpp
+++ clang/lib/Basic/Targets/AArch64.cpp
@@ -373,7 +373,8 @@
   Builder.defineMacro("__ARM_ALIGN_MAX_STACK_PWR", "4");
 
   // 0xe implies support for half, single and double precision operations.
-  Builder.defineMacro("__ARM_FP", "0xE");
+  if (FPU & FPUMode)
+Builder.defineMacro("__ARM_FP", "0xE");
 
   // PCS specifies this for SysV variants, which is all we support. Other ABIs
   // may choose __ARM_FP16_FORMAT_ALTERNATIVE.
@@ -709,6 +710,8 @@
 bool AArch64TargetInfo::handleTargetFeatures(std::vector 
,
  DiagnosticsEngine ) {
   for (const auto  : Features) {
+if (Feature == "-fp-armv8")
+  HasNoFP = true;
 if (Feature == "-neon")
   HasNoNeon = true;
 if (Feature == "-sve")
@@ -937,6 +940,11 @@
   setDataLayout();
   setArchFeatures();
 
+  if (HasNoFP) {
+FPU &= ~FPUMode;
+FPU &= ~NeonMode;
+FPU &= ~SveMode;
+  }
   if (HasNoNeon) {
 FPU &= ~NeonMode;
 FPU &= ~SveMode;


Index: clang/test/Preprocessor/aarch64-target-features.c
===
--- clang/test/Preprocessor/aarch64-target-features.c
+++ clang/test/Preprocessor/aarch64-target-features.c
@@ -341,6 +341,10 @@
 // CHECK-MARCH-2: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-fp-armv8" "-target-feature" "-neon" "-target-feature" "-crc" "-target-feature" "-crypto"
 // CHECK-MARCH-3: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "-neon"
 
+// While we're checking +nofp, also make sure it stops defining __ARM_FP
+// RUN: %clang -target aarch64-none-linux-gnu -march=armv8-r+nofp -x c -E -dM %s -o - | FileCheck -check-prefix=CHECK-NOFP %s
+// CHECK-NOFP-NOT: #define __ARM_FP{{ }}
+
 // Check +sm4:
 //
 // RUN: %clang -target aarch64 -march=armv8.2a+sm4 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-SM4 %s
Index: clang/lib/Basic/Targets/AArch64.h
===
--- clang/lib/Basic/Targets/AArch64.h
+++ clang/lib/Basic/Targets/AArch64.h
@@ -26,7 +26,7 @@
   static const TargetInfo::GCCRegAlias GCCRegAliases[];
   static const char *const GCCRegNames[];
 
-  enum FPUModeEnum { FPUMode, NeonMode = (1 << 0), SveMode = (1 << 1) };
+ 

[PATCH] D145780: [clang-tidy] Introduce option to allow magic numbers in type aliases

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:198
+  magic numbers in type aliases such as ``using`` and ``typedef`` declarations 
if
+  the new ``IgnoreTypeAliases`` option is set to false.
 

is set to `true`, not false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145780

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

[PATCH] D145780: [clang-tidy] Introduce option to allow magic numbers in type aliases

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added a comment.

Merge those commits under D145778 , no need 
for separate review. (git rebase -i HEAD~3)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145780

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

[PATCH] D145780: [clang-tidy] Introduce option to allow magic numbers in type aliases

[Florian Humblot via Phabricator via cfe-commits]

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

This commit introduces an option to the readability-magic-values checker.
The option defaults to false so that the behavior of the checker doesn't change 
unless specifically enabled.
These commits are supposed to fix 
https://github.com/llvm/llvm-project/issues/61259


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145780

Files:
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-type-aliases.cpp
  clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
@@ -3,7 +3,8 @@
 // RUN:  [{key: readability-magic-numbers.IgnoredIntegerValues, value: "0;1;2;10;100;"}, \
 // RUN:   {key: readability-magic-numbers.IgnoredFloatingPointValues, value: "3.14;2.71828;9.81;1.0;101.0;0x1.2p3"}, \
 // RUN:   {key: readability-magic-numbers.IgnoreBitFieldsWidths, value: false}, \
-// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}]}' \
+// RUN:   {key: readability-magic-numbers.IgnorePowersOf2IntegerValues, value: true}, \
+// RUN:   {key: readability-magic-numbers.IgnoreTypeAliases, value: false}]}' \
 // RUN: --
 
 template 
@@ -96,16 +97,15 @@
// CHECK-MESSAGES: :[[@LINE-1]]:25: warning: 3 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 };
 
+using NumberInTypeAlias = ValueBucket;
+// CHECK-MESSAGES: :[[@LINE-1]]:44: warning: 25 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+typedef ValueBucket NumberInTypedef;
+// CHECK-MESSAGES: :[[@LINE-1]]:27: warning: 243 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
 
 /*
  * Clean code
  */
 
-/*
- * No warning for type aliases
- */
-using NumberInTypeAlias = ValueBucket;
-typedef ValueBucket NumberInTypedef;
 
 #define INT_MACRO 5
 
Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-type-aliases.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers-type-aliases.cpp
@@ -0,0 +1,18 @@
+// RUN: %check_clang_tidy %s readability-magic-numbers %t \
+// RUN: -config='{CheckOptions: \
+// RUN:  [{key: readability-magic-numbers.IgnoreTypeAliases, value: true}]}' \
+// RUN: --
+
+template 
+struct ValueBucket {
+  T value[V];
+};
+
+int BadGlobalInt = 5;
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 5 is a magic number; consider replacing it with a named constant [readability-magic-numbers]
+
+/**
+ * With the flag enabled these should produce no warning
+ */
+using NumberInTypeAlias = ValueBucket;
+typedef ValueBucket NumberInTypedef;
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -137,3 +137,8 @@
Boolean value indicating whether to accept magic numbers as bit field widths
without warning. This is useful for example for register definitions which
are generated from hardware specifications. Default value is `true`.
+
+.. option:: IgnoreTypeAliases
+
+   Boolean value indicating whether to accept magic numbers in ``typedef`` or
+   ``using`` declarations. Default value is `false`
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -193,8 +193,9 @@
   for bit-field and integer members as a loop variable or upper limit were added.
 
 - Improved :doc:`readability-magic-numbers 
-  ` check. The check now allows for
-  magic numbers in type aliases such as ``using`` and ``typedef`` declarations.
+  ` check, now allows for
+  magic numbers in type aliases such as ``using`` and ``typedef`` declarations if
+  the new ``IgnoreTypeAliases`` option is set to false.
 
 Removed checks
 ^^
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.h

[PATCH] D144638: [lit] Detect Inconsistent File Access Times

[Sam Elliott via Phabricator via cfe-commits]

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

@jhenderson @int3 I think I have addressed your feedback - are you happy for me 
to land this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144638

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

[PATCH] D141497: [clang][Interp] Record initialization via conditional operator

[Timm Bäder via Phabricator via cfe-commits]

tbaeder updated this revision to Diff 504079.

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

https://reviews.llvm.org/D141497

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/lib/AST/Interp/ByteCodeExprGen.h
  clang/test/AST/Interp/records.cpp

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -335,3 +335,14 @@
   constexpr piecewise_construct_t piecewise_construct =
 piecewise_construct_t();
 };
+
+namespace ConditionalInit {
+  struct S { int a; };
+
+  constexpr S getS(bool b) {
+return b ? S{12} : S{13};
+  }
+
+  static_assert(getS(true).a == 12, "");
+  static_assert(getS(false).a == 13, "");
+};
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -179,6 +179,9 @@
 return this->emitPopPtr(I);
   }
 
+  template 
+  bool visitConditional(const AbstractConditionalOperator *E, VisitFn V);
+
   /// Creates a local primitive value.
   unsigned allocateLocalPrimitive(DeclTy &, PrimType Ty, bool IsMutable,
   bool IsExtended = false);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -588,32 +588,8 @@
 template 
 bool ByteCodeExprGen::VisitAbstractConditionalOperator(
 const AbstractConditionalOperator *E) {
-  const Expr *Condition = E->getCond();
-  const Expr *TrueExpr = E->getTrueExpr();
-  const Expr *FalseExpr = E->getFalseExpr();
-
-  LabelTy LabelEnd = this->getLabel();   // Label after the operator.
-  LabelTy LabelFalse = this->getLabel(); // Label for the false expr.
-
-  if (!this->visit(Condition))
-return false;
-  if (!this->jumpFalse(LabelFalse))
-return false;
-
-  if (!this->visit(TrueExpr))
-return false;
-  if (!this->jump(LabelEnd))
-return false;
-
-  this->emitLabel(LabelFalse);
-
-  if (!this->visit(FalseExpr))
-return false;
-
-  this->fallthrough(LabelEnd);
-  this->emitLabel(LabelEnd);
-
-  return true;
+  return this->visitConditional(
+  E, [this](const Expr *E) { return this->visit(E); });
 }
 
 template 
@@ -944,6 +920,41 @@
   }
 }
 
+/// Visit a conditional operator, i.e. `A ? B : C`.
+/// \V determines what function to call for the B and C expressions.
+template 
+template 
+bool ByteCodeExprGen::visitConditional(
+const AbstractConditionalOperator *E, VisitFn V) {
+
+  const Expr *Condition = E->getCond();
+  const Expr *TrueExpr = E->getTrueExpr();
+  const Expr *FalseExpr = E->getFalseExpr();
+
+  LabelTy LabelEnd = this->getLabel();   // Label after the operator.
+  LabelTy LabelFalse = this->getLabel(); // Label for the false expr.
+
+  if (!this->visit(Condition))
+return false;
+  if (!this->jumpFalse(LabelFalse))
+return false;
+
+  if (!V(TrueExpr))
+return false;
+  if (!this->jump(LabelEnd))
+return false;
+
+  this->emitLabel(LabelFalse);
+
+  if (!V(FalseExpr))
+return false;
+
+  this->fallthrough(LabelEnd);
+  this->emitLabel(LabelEnd);
+
+  return true;
+}
+
 template 
 bool ByteCodeExprGen::visitZeroInitializer(PrimType T, const Expr *E) {
   switch (T) {
@@ -1435,6 +1446,10 @@
 return this->visitInitializer(CE->getSubExpr());
   } else if (const auto *CE = dyn_cast(Initializer)) {
 return this->visitInitializer(CE->getSubExpr());
+  } else if (const auto *ACO =
+ dyn_cast(Initializer)) {
+return this->visitConditional(
+ACO, [this](const Expr *E) { return this->visitRecordInitializer(E); });
   }
 
   return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145773: [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.

[Haojian Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
hokein marked an inline comment as done.
Closed by commit rG7ab16be4c030: [clangd] UnusedIncludes: Strict config now 
uses the include-cleaner-library… (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145773

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -38,7 +38,6 @@
 
 using ::testing::AllOf;
 using ::testing::ElementsAre;
-using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::Pointee;
@@ -64,286 +63,6 @@
   return "#pragma once\n" + Code.str();
 }
 
-TEST(IncludeCleaner, ReferencedLocations) {
-  struct TestCase {
-std::string HeaderCode;
-std::string MainCode;
-  };
-  TestCase Cases[] = {
-  // DeclRefExpr
-  {
-  "int ^x();",
-  "int y = x();",
-  },
-  // RecordDecl
-  {
-  "class ^X;",
-  "X *y;",
-  },
-  // When definition is available, we don't need to mark forward
-  // declarations as used.
-  {
-  "class ^X {}; class X;",
-  "X *y;",
-  },
-  // We already have forward declaration in the main file, the other
-  // non-definition declarations are not needed.
-  {
-  "class ^X {}; class X;",
-  "class X; X *y;",
-  },
-  // Nested class definition can occur outside of the parent class
-  // definition. Bar declaration should be visible to its definition but
-  // it will always be because we will mark Foo definition as used.
-  {
-  "class ^Foo { class Bar; };",
-  "class Foo::Bar {};",
-  },
-  // TypedefType and UsingDecls
-  {
-  "using ^Integer = int;",
-  "Integer x;",
-  },
-  {
-  "namespace ns { void ^foo(); void ^foo() {} }",
-  "using ns::foo;",
-  },
-  {
-  "namespace ns { void foo(); void foo() {}; }",
-  "using namespace ns;",
-  },
-  {
-  // Refs from UsingTypeLoc and implicit constructor!
-  "struct ^A {}; using B = A; using ^C = B;",
-  "C a;",
-  },
-  {"namespace ns { template class A {}; } using ns::^A;",
-   "A* a;"},
-  {"namespace ns { template class A {}; } using ns::^A;",
-   R"cpp(
-  template  class T> class X {};
-  X x;
-)cpp"},
-  {R"cpp(
-  namespace ns { template class A {}; }
-  namespace absl {using ns::^A;}
-   )cpp",
-   R"cpp(
-  template  class T> class X {};
-  X x;
-   )cpp"},
-  {R"cpp(
-  namespace ns { template struct ^A { ^A(T); }; }
-  using ns::^A;
-   )cpp",
-   "A CATD(123);"},
-  {
-  "typedef bool ^Y; template  struct ^X {};",
-  "X x;",
-  },
-  {
-  // https://github.com/clangd/clangd/issues/1036
-  R"cpp(
-struct ^Base { void ^base(); };
-template  struct ^Derived : Base {};
-  )cpp",
-  R"cpp(
-class Holder {
-  void foo() { Member.base(); }
-  Derived<0> Member;
-};
-  )cpp",
-  },
-  {
-  "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
-  "Bar b;",
-  },
-  {
-  "namespace ns { class X; }; using ns::^X;",
-  "X *y;",
-  },
-  // MemberExpr
-  {
-  "struct ^X{int ^a;}; X ^foo();",
-  "int y = foo().a;",
-  },
-  // Expr (type is traversed)
-  {
-  "class ^X{}; X ^foo();",
-  "auto bar() { return foo(); }",
-  },
-  // Redecls
-  {
-  "void ^foo(); void ^foo() {} void ^foo();",
-  "void bar() { foo(); }",
-  },
-  // Constructor
-  {
-  "struct ^X { ^X(int) {} int ^foo(); };",
-  "auto x = X(42); auto y = x.foo();",
-  },
-  // Function
-  {
-  "void ^foo();",
-  "void foo() {}",
-  },
-  {
-  "void foo() {}",
-  "void foo();",
-  },
-  {
-  "inline void ^foo() {}",
-  "void bar() { foo(); }",
-  },
-  {
-  "int ^foo(char); int ^foo(float);",
-  "template int x = foo(T{});",
-  },
-  // Static function
-  {
-  "struct ^X { static 

[clang-tools-extra] 7ab16be - [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2023-03-10T11:56:11+01:00
New Revision: 7ab16be4c0302f648f85dbf1fe3d1a5dde9b9600

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

LOG: [clangd] UnusedIncludes: Strict config now uses the 
include-cleaner-library implementation.

And remove the classical clangd-own unused-include implementation.

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

Added: 


Modified: 
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index ab346aab0e8ac..daadf0ee3d3ce 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -92,9 +92,6 @@ struct Config {
 /// Diagnose missing and unused includes.
 Strict,
 None,
-/// The same as Strict, but using the include-cleaner library for
-/// unused includes.
-Experiment,
   };
   /// Controls warnings and errors when parsing code.
   struct {

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 2f0ef892131ca..fb6c6e86c1acd 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -430,16 +430,24 @@ struct FragmentCompiler {
   C.Diagnostics.Suppress.insert(N);
   });
 
-if (F.UnusedIncludes)
-  if (auto Val = compileEnum("UnusedIncludes",
- **F.UnusedIncludes)
- .map("Strict", Config::IncludesPolicy::Strict)
- .map("Experiment", Config::IncludesPolicy::Experiment)
- .map("None", Config::IncludesPolicy::None)
- .value())
+if (F.UnusedIncludes) {
+  auto Val = compileEnum("UnusedIncludes",
+ **F.UnusedIncludes)
+ .map("Strict", Config::IncludesPolicy::Strict)
+ .map("None", Config::IncludesPolicy::None)
+ .value();
+  if (!Val && **F.UnusedIncludes == "Experiment") {
+diag(Warning,
+ "Experiment is deprecated for UnusedIncludes, use Strict 
instead.",
+ F.UnusedIncludes->Range);
+Val = Config::IncludesPolicy::Strict;
+  }
+  if (Val) {
 Out.Apply.push_back([Val](const Params &, Config ) {
   C.Diagnostics.UnusedIncludes = *Val;
 });
+  }
+}
 
 if (F.AllowStalePreamble) {
   if (auto Val = F.AllowStalePreamble)

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 956e8a8599446..a56e919cbaf7a 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -232,7 +232,6 @@ struct Fragment {
 ///
 /// Valid values are:
 /// - Strict
-/// - Experiment
 /// - None
 std::optional> UnusedIncludes;
 

diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 0d74b888557ed..a990b89edb230 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -16,7 +16,6 @@
 #include "URI.h"
 #include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Types.h"
-#include "index/CanonicalIncludes.h"
 #include "support/Logger.h"
 #include "support/Path.h"
 #include "support/Trace.h"
@@ -24,7 +23,6 @@
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
-#include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/TemplateName.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/Diagnostic.h"
@@ -36,7 +34,6 @@
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Core/Replacement.h"
 #include "clang/Tooling/Inclusions/HeaderIncludes.h"
-#include "clang/Tooling/Inclusions/IncludeStyle.h"
 #include "clang/Tooling/Inclusions/StandardLibrary.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
@@ -66,181 +63,6 @@ void setIncludeCleanerAnalyzesStdlib(bool B) { 
AnalyzeStdlib = B; }
 
 namespace {
 
-/// Crawler traverses the AST and feeds in the locations of (sometimes
-/// implicitly) used symbols into \p Result.
-class ReferencedLocationCrawler
-: public RecursiveASTVisitor {
-public:
-  ReferencedLocationCrawler(ReferencedLocations ,
-const SourceManager )
-  : Result(Result), SM(SM) {}
-
-  

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL requested changes to this revision.
PiotrZSL added a comment.
This revision now requires changes to proceed.

This is for issue: https://github.com/llvm/llvm-project/issues/61259 
Reference it in commit message.

Add config: "flag should be added i.e. IgnoreTypeAliases which will default to 
false."
Except that looks fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145778

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

[PATCH] D145776: [clangd] Remove the classical clangd-own unsued-include implementation.

[Haojian Wu via Phabricator via cfe-commits]

hokein abandoned this revision.
hokein marked an inline comment as done.
hokein added a comment.

Merge the change in https://reviews.llvm.org/D145773 per the review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145776

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

[PATCH] D145773: [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 504073.
hokein added a comment.
Herald added a subscriber: mgrang.

add the removal of the clangd-own implementation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145773

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -38,7 +38,6 @@
 
 using ::testing::AllOf;
 using ::testing::ElementsAre;
-using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::Pointee;
@@ -64,286 +63,6 @@
   return "#pragma once\n" + Code.str();
 }
 
-TEST(IncludeCleaner, ReferencedLocations) {
-  struct TestCase {
-std::string HeaderCode;
-std::string MainCode;
-  };
-  TestCase Cases[] = {
-  // DeclRefExpr
-  {
-  "int ^x();",
-  "int y = x();",
-  },
-  // RecordDecl
-  {
-  "class ^X;",
-  "X *y;",
-  },
-  // When definition is available, we don't need to mark forward
-  // declarations as used.
-  {
-  "class ^X {}; class X;",
-  "X *y;",
-  },
-  // We already have forward declaration in the main file, the other
-  // non-definition declarations are not needed.
-  {
-  "class ^X {}; class X;",
-  "class X; X *y;",
-  },
-  // Nested class definition can occur outside of the parent class
-  // definition. Bar declaration should be visible to its definition but
-  // it will always be because we will mark Foo definition as used.
-  {
-  "class ^Foo { class Bar; };",
-  "class Foo::Bar {};",
-  },
-  // TypedefType and UsingDecls
-  {
-  "using ^Integer = int;",
-  "Integer x;",
-  },
-  {
-  "namespace ns { void ^foo(); void ^foo() {} }",
-  "using ns::foo;",
-  },
-  {
-  "namespace ns { void foo(); void foo() {}; }",
-  "using namespace ns;",
-  },
-  {
-  // Refs from UsingTypeLoc and implicit constructor!
-  "struct ^A {}; using B = A; using ^C = B;",
-  "C a;",
-  },
-  {"namespace ns { template class A {}; } using ns::^A;",
-   "A* a;"},
-  {"namespace ns { template class A {}; } using ns::^A;",
-   R"cpp(
-  template  class T> class X {};
-  X x;
-)cpp"},
-  {R"cpp(
-  namespace ns { template class A {}; }
-  namespace absl {using ns::^A;}
-   )cpp",
-   R"cpp(
-  template  class T> class X {};
-  X x;
-   )cpp"},
-  {R"cpp(
-  namespace ns { template struct ^A { ^A(T); }; }
-  using ns::^A;
-   )cpp",
-   "A CATD(123);"},
-  {
-  "typedef bool ^Y; template  struct ^X {};",
-  "X x;",
-  },
-  {
-  // https://github.com/clangd/clangd/issues/1036
-  R"cpp(
-struct ^Base { void ^base(); };
-template  struct ^Derived : Base {};
-  )cpp",
-  R"cpp(
-class Holder {
-  void foo() { Member.base(); }
-  Derived<0> Member;
-};
-  )cpp",
-  },
-  {
-  "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
-  "Bar b;",
-  },
-  {
-  "namespace ns { class X; }; using ns::^X;",
-  "X *y;",
-  },
-  // MemberExpr
-  {
-  "struct ^X{int ^a;}; X ^foo();",
-  "int y = foo().a;",
-  },
-  // Expr (type is traversed)
-  {
-  "class ^X{}; X ^foo();",
-  "auto bar() { return foo(); }",
-  },
-  // Redecls
-  {
-  "void ^foo(); void ^foo() {} void ^foo();",
-  "void bar() { foo(); }",
-  },
-  // Constructor
-  {
-  "struct ^X { ^X(int) {} int ^foo(); };",
-  "auto x = X(42); auto y = x.foo();",
-  },
-  // Function
-  {
-  "void ^foo();",
-  "void foo() {}",
-  },
-  {
-  "void foo() {}",
-  "void foo();",
-  },
-  {
-  "inline void ^foo() {}",
-  "void bar() { foo(); }",
-  },
-  {
-  "int ^foo(char); int ^foo(float);",
-  "template int x = foo(T{});",
-  },
-  // Static function
-  {
-  "struct ^X { static bool ^foo(); }; bool X::^foo() {}",
-  "auto b = X::foo();",
-  },
-  // TemplateRecordDecl
-  {
-  "template  class ^X;",

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp:40
 
+  if (Node.get() || Node.get())
+return true;

This should be configurable, someone else may don't want magic numbers in 
template arguments.
So we shouldn't change default behaviour.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:196
+- Improved :doc:`readability-magic-numbers 
+  ` check. The check now allows 
for
+  magic numbers in type aliases such as ``using`` and ``typedef`` declarations.

consider: "check, now allows..."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145778

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

[PATCH] D145773: [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:438
+ .map("Experiment",
+  Config::IncludesPolicy::Strict) // for backward
+  // compatibility

hokein wrote:
> kadircet wrote:
> > i think we should at least be emitting a diagnostics to encourage people 
> > for moving back to strict, so what about something like:
> > ```
> > if (F.UnusuedIncludes) {
> >   auto Val = compileEnum; // only for Strict and None
> >   if (!Val && **F.UnusedIncludes == "Experiment") {
> >diag(Warning, "Experiment is deprecated for UnusedIncludes, use 
> > Strict instead.", F.UnusedIncludes.Range);
> >Val = Config::IncludesPolicy::Strict;
> >   }
> > }
> > ```
> I thought it was not worth a diagnostic because this flag was introduced 
> recently, and we have never advertised it to open-source users.
> 
> But the flag is in the recent clangd16 release, so it probably justifies the 
> value. 
> 
> BTW, looks like we forgot to update the release notes for clangd16, 
> https://github.com/llvm/llvm-project/blob/release/16.x/clang-tools-extra/docs/ReleaseNotes.rst#improvements-to-clangd
>  is empty.
> 
> But the flag is in the recent clangd16 release, so it probably justifies the 
> value.

right.

> BTW, looks like we forgot to update the release notes for clangd16, 
> https://github.com/llvm/llvm-project/blob/release/16.x/clang-tools-extra/docs/ReleaseNotes.rst#improvements-to-clangd
>  is empty.

yeah, i've aslo noticed that will take a look. but regarding this feature, I 
don't think we should be advertising `Experiment` there, right?



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:769
-  Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict
-  ? computeUnusedIncludes(AST)
-  : Findings.UnusedIncludes,

hokein wrote:
> kadircet wrote:
> > can you also delete `computeUnusedIncludes` and its friends (also from the 
> > tests)?
> this is in plan, but in a separate patch, https://reviews.llvm.org/D145776
as mentioned on that patch, i think it's better to land them in single step, to 
make sure we can revert a single patch if we want the old behavior rather than 
multiple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145773

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

[PATCH] D145776: [clangd] Remove the classical clangd-own unsued-include implementation.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

i'd still merge this with the previous patch, as all of this is dead code after 
config option deletion. so it'd be better to just revert a single patch if we 
want to restore the old behavior, rather than two.




Comment at: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp:720
 
 TEST(IncludeCleaner, RecursiveInclusion) {
   TestTU TU;

i don't think this test is meaningful for our current usage of the 
include-cleaner library, feel free to drop it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145776

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

[PATCH] D145773: [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:438
+ .map("Experiment",
+  Config::IncludesPolicy::Strict) // for backward
+  // compatibility

kadircet wrote:
> i think we should at least be emitting a diagnostics to encourage people for 
> moving back to strict, so what about something like:
> ```
> if (F.UnusuedIncludes) {
>   auto Val = compileEnum; // only for Strict and None
>   if (!Val && **F.UnusedIncludes == "Experiment") {
>diag(Warning, "Experiment is deprecated for UnusedIncludes, use Strict 
> instead.", F.UnusedIncludes.Range);
>Val = Config::IncludesPolicy::Strict;
>   }
> }
> ```
I thought it was not worth a diagnostic because this flag was introduced 
recently, and we have never advertised it to open-source users.

But the flag is in the recent clangd16 release, so it probably justifies the 
value. 

BTW, looks like we forgot to update the release notes for clangd16, 
https://github.com/llvm/llvm-project/blob/release/16.x/clang-tools-extra/docs/ReleaseNotes.rst#improvements-to-clangd
 is empty.




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:769
-  Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict
-  ? computeUnusedIncludes(AST)
-  : Findings.UnusedIncludes,

kadircet wrote:
> can you also delete `computeUnusedIncludes` and its friends (also from the 
> tests)?
this is in plan, but in a separate patch, https://reviews.llvm.org/D145776


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145773

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

[PATCH] D145773: [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 504069.
hokein marked an inline comment as done.
hokein added a comment.

Add diagnostic for Experiment flag usage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145773

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -697,7 +697,7 @@
 void foo() {}
   )cpp");
   Config Cfg;
-  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Experiment;
+  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
   WithContextValue Ctx(Config::Key, std::move(Cfg));
   ParsedAST AST = TU.build();
 
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -128,7 +128,7 @@
 SourceMgr = ();
 Includes.collect(CI);
 if (Config::current().Diagnostics.UnusedIncludes ==
-Config::IncludesPolicy::Experiment ||
+Config::IncludesPolicy::Strict ||
 Config::current().Diagnostics.MissingIncludes ==
 Config::IncludesPolicy::Strict)
   Pragmas.record(CI);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -758,17 +758,13 @@
   const Config  = Config::current();
   IncludeCleanerFindings Findings;
   if (Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::Strict ||
-  Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Experiment) {
+  Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict) {
 // will need include-cleaner results, call it once
 Findings = computeIncludeCleanerFindings(AST);
   }
 
   std::vector Result = generateUnusedIncludeDiagnostics(
-  AST.tuPath(),
-  Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict
-  ? computeUnusedIncludes(AST)
-  : Findings.UnusedIncludes,
-  Code);
+  AST.tuPath(), Findings.UnusedIncludes, Code);
   llvm::move(
   generateMissingIncludeDiagnostics(AST, Findings.MissingIncludes, Code),
   std::back_inserter(Result));
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -232,7 +232,6 @@
 ///
 /// Valid values are:
 /// - Strict
-/// - Experiment
 /// - None
 std::optional> UnusedIncludes;
 
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -430,16 +430,24 @@
   C.Diagnostics.Suppress.insert(N);
   });
 
-if (F.UnusedIncludes)
-  if (auto Val = compileEnum("UnusedIncludes",
- **F.UnusedIncludes)
- .map("Strict", Config::IncludesPolicy::Strict)
- .map("Experiment", Config::IncludesPolicy::Experiment)
- .map("None", Config::IncludesPolicy::None)
- .value())
+if (F.UnusedIncludes) {
+  auto Val = compileEnum("UnusedIncludes",
+ **F.UnusedIncludes)
+ .map("Strict", Config::IncludesPolicy::Strict)
+ .map("None", Config::IncludesPolicy::None)
+ .value();
+  if (!Val && **F.UnusedIncludes == "Experiment") {
+diag(Warning,
+ "Experiment is deprecated for UnusedIncludes, use Strict instead.",
+ F.UnusedIncludes->Range);
+Val = Config::IncludesPolicy::Strict;
+  }
+  if (Val) {
 Out.Apply.push_back([Val](const Params &, Config ) {
   C.Diagnostics.UnusedIncludes = *Val;
 });
+  }
+}
 
 if (F.AllowStalePreamble) {
   if (auto Val = F.AllowStalePreamble)
Index: clang-tools-extra/clangd/Config.h
===
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -92,9 +92,6 @@
 /// Diagnose missing and unused includes.
 Strict,
 None,
-/// The same as Strict, but using the include-cleaner library 

[PATCH] D145778: [clang-tidy] Change readability-magic-numbers to allow numbers in type aliases.

[Florian Humblot via Phabricator via cfe-commits]

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

This commit changes the default behavior of the readability-magic-numbers check 
to allow
"magic" numbers to be used in ``using`` and ``typedef`` declarations.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145778

Files:
  clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
  clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
@@ -101,6 +101,12 @@
  * Clean code
  */
 
+/*
+ * No warning for type aliases
+ */
+using NumberInTypeAlias = ValueBucket;
+typedef ValueBucket NumberInTypedef;
+
 #define INT_MACRO 5
 
 const int GoodGlobalIntConstant = 42;
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -24,6 +24,16 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   struct OtherType {
+  CustomType container;
+   }
+   CustomType values;
+
double circleArea = 3.1415926535 * radius * radius;
 
double totalCharge = 1.08 * itemPrice;
@@ -40,6 +50,17 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   using containerType = CustomType;
+   struct OtherType {
+  containerType container;
+   }
+   containerType values;
+
double circleArea = M_PI * radius * radius;
 
const double TAX_RATE = 0.08;  // or make it variable and read from a file
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,6 +192,10 @@
   ` check. Basic support
   for bit-field and integer members as a loop variable or upper limit were 
added.
 
+- Improved :doc:`readability-magic-numbers 
+  ` check. The check now allows 
for
+  magic numbers in type aliases such as ``using`` and ``typedef`` declarations.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/MagicNumbersCheck.cpp
@@ -14,6 +14,7 @@
 #include "MagicNumbersCheck.h"
 #include "../utils/OptionsUtils.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Type.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "llvm/ADT/STLExtras.h"
 #include 
@@ -36,6 +37,9 @@
   if (Node.get())
 return true;
 
+  if (Node.get() || Node.get())
+return true;
+
   return llvm::any_of(Result.Context->getParents(Node),
   [](const DynTypedNode ) {
 return isUsedToInitializeAConstant(Result, Parent);


Index: clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/magic-numbers.cpp
@@ -101,6 +101,12 @@
  * Clean code
  */
 
+/*
+ * No warning for type aliases
+ */
+using NumberInTypeAlias = ValueBucket;
+typedef ValueBucket NumberInTypedef;
+
 #define INT_MACRO 5
 
 const int GoodGlobalIntConstant = 42;
Index: clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability/magic-numbers.rst
@@ -24,6 +24,16 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   struct OtherType {
+  CustomType container;
+   }
+   CustomType values;
+
double circleArea = 3.1415926535 * radius * radius;
 
double totalCharge = 1.08 * itemPrice;
@@ -40,6 +50,17 @@
 
 .. code-block:: c++
 
+   template
+   struct CustomType {
+  T arr[N];
+   };
+
+   using containerType = CustomType;
+   struct OtherType {
+  containerType container;
+   }
+   containerType values;
+
double 

[PATCH] D145715: Remove -lower-global-dtors-via-cxa-atexit flag

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added a comment.

It might be worth adding something to the release notes explaining that removal 
of the flag


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145715

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

[PATCH] D145069: [analyzer][NFC] Split the no state change logic and bug report suppression into two visitors

[Gábor Spaits via Phabricator via cfe-commits]

spaits updated this revision to Diff 504062.
spaits added a comment.

As @Szelethus has mentioned in his reply I tried to improve on the comment. I 
hope now it describes correctly what the new visitor is good for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145069

Files:
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp

Index: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
===
--- clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
+++ clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
@@ -452,29 +452,8 @@
   CallEventRef<> Call =
   BR.getStateManager().getCallEventManager().getCaller(SCtx, State);
 
-  // Optimistically suppress uninitialized value bugs that result
-  // from system headers having a chance to initialize the value
-  // but failing to do so. It's too unlikely a system header's fault.
-  // It's much more likely a situation in which the function has a failure
-  // mode that the user decided not to check. If we want to hunt such
-  // omitted checks, we should provide an explicit function-specific note
-  // describing the precondition under which the function isn't supposed to
-  // initialize its out-parameter, and additionally check that such
-  // precondition can actually be fulfilled on the current path.
-  if (Call->isInSystemHeader()) {
-// We make an exception for system header functions that have no branches.
-// Such functions unconditionally fail to initialize the variable.
-// If they call other functions that have more paths within them,
-// this suppression would still apply when we visit these inner functions.
-// One common example of a standard function that doesn't ever initialize
-// its out parameter is operator placement new; it's up to the follow-up
-// constructor (if any) to initialize the memory.
-if (!N->getStackFrame()->getCFG()->isLinear()) {
-  static int i = 0;
-  R.markInvalid(, nullptr);
-}
+  if (Call->isInSystemHeader())
 return nullptr;
-  }
 
   if (const auto *MC = dyn_cast(Call)) {
 // If we failed to construct a piece for self, we still want to check
@@ -492,6 +471,70 @@
   return maybeEmitNoteForParameters(R, *Call, N);
 }
 
+//===--===//
+// Implementation of SuppressSystemHeaderWarningVistor.
+//===--===//
+
+// This visitor suppresses warnings coming from inlined system
+// headers functions when we exit the call that have no branches.
+// This is a very primitive visitor. It flat-out suppresses every report that
+// have a node that matches all the conditions above.
+// It should be used carefully!
+//
+// A good example for the usage of this visitor is when we want to suppress
+// warnings when a system header function does not initialize their arguments.
+// It's too unlikely a system header's fault.
+// It's much more likely a situation in which the function has a failure
+// mode that the user decided not to check. If we want to hunt such
+// omitted checks, we should provide an explicit function-specific note
+// describing the precondition under which the function isn't supposed to
+// initialize its out-parameter, and additionally check that such
+// precondition can actually be fulfilled on the current path.
+//
+// System header functions that have no branches unconditionally fail to
+// initialize the variable.
+// If they call other functions that have more paths within them,
+// this suppression would still apply when we visit these inner functions.
+// One common example of a standard function that doesn't ever initialize
+// its out parameter is operator placement new; it's up to the follow-up
+// constructor (if any) to initialize the memory
+namespace {
+class SuppressSystemHeaderWarningVisitor : public BugReporterVisitor {
+public:
+  virtual PathDiagnosticPieceRef VisitNode(const ExplodedNode *,
+   BugReporterContext &,
+   PathSensitiveBugReport &) override;
+  void Profile(llvm::FoldingSetNodeID ) const override {
+static int Tag = 0;
+ID.AddPointer();
+  }
+};
+} // namespace
+
+PathDiagnosticPieceRef
+SuppressSystemHeaderWarningVisitor::VisitNode(const ExplodedNode *Succ,
+  BugReporterContext ,
+  PathSensitiveBugReport ) {
+  const LocationContext *Ctx = Succ->getLocationContext();
+  const StackFrameContext *SCtx = Ctx->getStackFrame();
+  ProgramStateRef State = Succ->getState();
+  auto CallExitLoc = Succ->getLocationAs();
+
+  if (!CallExitLoc)
+return nullptr;
+
+  CallEventRef<> Call =
+  BRC.getStateManager().getCallEventManager().getCaller(SCtx, State);
+
+  if 

[PATCH] D143342: [clang-tidy] Support std::format and std::print in readability-redundant-string-cstr

[Piotr Zegar via Phabricator via cfe-commits]

PiotrZSL added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/RedundantStringCStrCheck.cpp:189
+  getLangOpts().CPlusPlus2b
+  ? hasAnyName("::std::print", "::std::format")
+  : hasName("::std::format"))),

Please introduce configuration option to specify custom functions.
For example if some project (like mine) is wrapping fmt::format with some 
variadic template function, then such function could be specified.
Same goes to things like some loggers.

Check utils/OptionsUtils.h for configuration, and  utils/Matchers.h 
(matchesAnyListedName)


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

https://reviews.llvm.org/D143342

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

[PATCH] D145311: [clang-tidy] Make abseil-redundant-strcat-calls checker use header

[Piotr Zegar via Phabricator via cfe-commits]

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

It's fine, absl::StrCat returns std::string.
So those changes are correct.
Here you can find source code 
https://github.com/abseil/abseil-cpp/blob/master/absl/strings/str_cat.h


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145311

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

[PATCH] D145776: [clangd] Remove the classical clangd-own unsued-include implementation.

[Haojian Wu via Phabricator via cfe-commits]

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

We have switch to use the include-library-based implementation.

Based on https://reviews.llvm.org/D145773


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145776

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp

Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -38,7 +38,6 @@
 
 using ::testing::AllOf;
 using ::testing::ElementsAre;
-using ::testing::ElementsAreArray;
 using ::testing::IsEmpty;
 using ::testing::Matcher;
 using ::testing::Pointee;
@@ -64,286 +63,6 @@
   return "#pragma once\n" + Code.str();
 }
 
-TEST(IncludeCleaner, ReferencedLocations) {
-  struct TestCase {
-std::string HeaderCode;
-std::string MainCode;
-  };
-  TestCase Cases[] = {
-  // DeclRefExpr
-  {
-  "int ^x();",
-  "int y = x();",
-  },
-  // RecordDecl
-  {
-  "class ^X;",
-  "X *y;",
-  },
-  // When definition is available, we don't need to mark forward
-  // declarations as used.
-  {
-  "class ^X {}; class X;",
-  "X *y;",
-  },
-  // We already have forward declaration in the main file, the other
-  // non-definition declarations are not needed.
-  {
-  "class ^X {}; class X;",
-  "class X; X *y;",
-  },
-  // Nested class definition can occur outside of the parent class
-  // definition. Bar declaration should be visible to its definition but
-  // it will always be because we will mark Foo definition as used.
-  {
-  "class ^Foo { class Bar; };",
-  "class Foo::Bar {};",
-  },
-  // TypedefType and UsingDecls
-  {
-  "using ^Integer = int;",
-  "Integer x;",
-  },
-  {
-  "namespace ns { void ^foo(); void ^foo() {} }",
-  "using ns::foo;",
-  },
-  {
-  "namespace ns { void foo(); void foo() {}; }",
-  "using namespace ns;",
-  },
-  {
-  // Refs from UsingTypeLoc and implicit constructor!
-  "struct ^A {}; using B = A; using ^C = B;",
-  "C a;",
-  },
-  {"namespace ns { template class A {}; } using ns::^A;",
-   "A* a;"},
-  {"namespace ns { template class A {}; } using ns::^A;",
-   R"cpp(
-  template  class T> class X {};
-  X x;
-)cpp"},
-  {R"cpp(
-  namespace ns { template class A {}; }
-  namespace absl {using ns::^A;}
-   )cpp",
-   R"cpp(
-  template  class T> class X {};
-  X x;
-   )cpp"},
-  {R"cpp(
-  namespace ns { template struct ^A { ^A(T); }; }
-  using ns::^A;
-   )cpp",
-   "A CATD(123);"},
-  {
-  "typedef bool ^Y; template  struct ^X {};",
-  "X x;",
-  },
-  {
-  // https://github.com/clangd/clangd/issues/1036
-  R"cpp(
-struct ^Base { void ^base(); };
-template  struct ^Derived : Base {};
-  )cpp",
-  R"cpp(
-class Holder {
-  void foo() { Member.base(); }
-  Derived<0> Member;
-};
-  )cpp",
-  },
-  {
-  "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
-  "Bar b;",
-  },
-  {
-  "namespace ns { class X; }; using ns::^X;",
-  "X *y;",
-  },
-  // MemberExpr
-  {
-  "struct ^X{int ^a;}; X ^foo();",
-  "int y = foo().a;",
-  },
-  // Expr (type is traversed)
-  {
-  "class ^X{}; X ^foo();",
-  "auto bar() { return foo(); }",
-  },
-  // Redecls
-  {
-  "void ^foo(); void ^foo() {} void ^foo();",
-  "void bar() { foo(); }",
-  },
-  // Constructor
-  {
-  "struct ^X { ^X(int) {} int ^foo(); };",
-  "auto x = X(42); auto y = x.foo();",
-  },
-  // Function
-  {
-  "void ^foo();",
-  "void foo() {}",
-  },
-  {
-  "void foo() {}",
-  "void foo();",
-  },
-  {
-  "inline void ^foo() {}",
-  "void bar() { foo(); }",
-  },
-  {
-  "int ^foo(char); int ^foo(float);",
-  "template int x = foo(T{});",
-  },
-  // Static function
-  {
-  "struct ^X { static bool ^foo(); }; bool X::^foo() {}",
-  "auto b = X::foo();",
-  },
-  // TemplateRecordDecl
-  {
-  "template  class ^X;",
- 

[PATCH] D145313: [clang-tidy] Make readability-container-size-empty check using header

[Piotr Zegar via Phabricator via cfe-commits]

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

LGTM, this is anyway non functional change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145313

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

[PATCH] D145312: [clang-tidy] Make readability-string-compare check use header

[Piotr Zegar via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D145312

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

[PATCH] D145775: Improve documentation of CXIndexOptions; NFC

[Igor Kushnir via Phabricator via cfe-commits]

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

Document one more alternative way to initialize struct CXIndexOptions,
which is used in LibclangSetPreambleStoragePathTest since
df8f8f76207df40dca11c9c0c2328d6b3dfba9ca 
 because 
the previous
initialization approach causes compiler warnings.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145775

Files:
  clang/include/clang-c/Index.h


Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -329,8 +329,8 @@
  * Index initialization options.
  *
  * 0 is the default value of each member of this struct except for Size.
- * Initialize the struct in one of the following two ways to avoid adapting 
code
- * each time a new member is added to it:
+ * Initialize the struct in one of the following three ways to avoid adapting
+ * code each time a new member is added to it:
  * \code
  * CXIndexOptions Opts;
  * memset(, 0, sizeof(Opts));
@@ -340,6 +340,11 @@
  * \code
  * CXIndexOptions Opts = { sizeof(CXIndexOptions) };
  * \endcode
+ * or to prevent the -Wmissing-field-initializers warning for the above 
version:
+ * \code
+ * CXIndexOptions Opts{};
+ * Opts.Size = sizeof(CXIndexOptions);
+ * \endcode
  */
 typedef struct CXIndexOptions {
   /**


Index: clang/include/clang-c/Index.h
===
--- clang/include/clang-c/Index.h
+++ clang/include/clang-c/Index.h
@@ -329,8 +329,8 @@
  * Index initialization options.
  *
  * 0 is the default value of each member of this struct except for Size.
- * Initialize the struct in one of the following two ways to avoid adapting code
- * each time a new member is added to it:
+ * Initialize the struct in one of the following three ways to avoid adapting
+ * code each time a new member is added to it:
  * \code
  * CXIndexOptions Opts;
  * memset(, 0, sizeof(Opts));
@@ -340,6 +340,11 @@
  * \code
  * CXIndexOptions Opts = { sizeof(CXIndexOptions) };
  * \endcode
+ * or to prevent the -Wmissing-field-initializers warning for the above version:
+ * \code
+ * CXIndexOptions Opts{};
+ * Opts.Size = sizeof(CXIndexOptions);
+ * \endcode
  */
 typedef struct CXIndexOptions {
   /**
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D145724: [clang-tidy] Provide default template arguments in

[Piotr Zegar via Phabricator via cfe-commits]

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

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145724

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

[PATCH] D145773: [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/ConfigCompile.cpp:438
+ .map("Experiment",
+  Config::IncludesPolicy::Strict) // for backward
+  // compatibility

i think we should at least be emitting a diagnostics to encourage people for 
moving back to strict, so what about something like:
```
if (F.UnusuedIncludes) {
  auto Val = compileEnum; // only for Strict and None
  if (!Val && **F.UnusedIncludes == "Experiment") {
   diag(Warning, "Experiment is deprecated for UnusedIncludes, use Strict 
instead.", F.UnusedIncludes.Range);
   Val = Config::IncludesPolicy::Strict;
  }
}
```



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:769
-  Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict
-  ? computeUnusedIncludes(AST)
-  : Findings.UnusedIncludes,

can you also delete `computeUnusedIncludes` and its friends (also from the 
tests)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145773

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

[PATCH] D145773: [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.

[Haojian Wu via Phabricator via cfe-commits]

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

This is the first step to deprecate the orginal clangd-own
implementation (the removal/cleanup will be in a followup patch).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D145773

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/Preamble.cpp
  clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -697,7 +697,7 @@
 void foo() {}
   )cpp");
   Config Cfg;
-  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Experiment;
+  Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
   WithContextValue Ctx(Config::Key, std::move(Cfg));
   ParsedAST AST = TU.build();
 
Index: clang-tools-extra/clangd/Preamble.cpp
===
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -128,7 +128,7 @@
 SourceMgr = ();
 Includes.collect(CI);
 if (Config::current().Diagnostics.UnusedIncludes ==
-Config::IncludesPolicy::Experiment ||
+Config::IncludesPolicy::Strict ||
 Config::current().Diagnostics.MissingIncludes ==
 Config::IncludesPolicy::Strict)
   Pragmas.record(CI);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -758,17 +758,13 @@
   const Config  = Config::current();
   IncludeCleanerFindings Findings;
   if (Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::Strict ||
-  Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Experiment) {
+  Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict) {
 // will need include-cleaner results, call it once
 Findings = computeIncludeCleanerFindings(AST);
   }
 
   std::vector Result = generateUnusedIncludeDiagnostics(
-  AST.tuPath(),
-  Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict
-  ? computeUnusedIncludes(AST)
-  : Findings.UnusedIncludes,
-  Code);
+  AST.tuPath(), Findings.UnusedIncludes, Code);
   llvm::move(
   generateMissingIncludeDiagnostics(AST, Findings.MissingIncludes, Code),
   std::back_inserter(Result));
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -232,7 +232,6 @@
 ///
 /// Valid values are:
 /// - Strict
-/// - Experiment
 /// - None
 std::optional> UnusedIncludes;
 
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -434,7 +434,9 @@
   if (auto Val = compileEnum("UnusedIncludes",
  **F.UnusedIncludes)
  .map("Strict", Config::IncludesPolicy::Strict)
- .map("Experiment", Config::IncludesPolicy::Experiment)
+ .map("Experiment",
+  Config::IncludesPolicy::Strict) // for backward
+  // compatibility
  .map("None", Config::IncludesPolicy::None)
  .value())
 Out.Apply.push_back([Val](const Params &, Config ) {
Index: clang-tools-extra/clangd/Config.h
===
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -92,9 +92,6 @@
 /// Diagnose missing and unused includes.
 Strict,
 None,
-/// The same as Strict, but using the include-cleaner library for
-/// unused includes.
-Experiment,
   };
   /// Controls warnings and errors when parsing code.
   struct {


Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -697,7 +697,7 @@
 void foo() {}
   )cpp");
   Config Cfg;
-  

[PATCH] D145034: [Clang][Sema] Start fixing handling of out-of-line definitions of constrained templates

[Alexander Shaposhnikov 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 rG421c098b32bd: [Clang][Sema] Start fixing handling of 
out-of-line definitions of constrained… (authored by alexander-shaposhnikov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145034

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Sema/SemaCXXScopeSpec.cpp
  
clang/test/CXX/temp/temp.decls/temp.class.spec/temp.class.spec.mfunc/p1-neg.cpp
  clang/test/SemaTemplate/concepts-out-of-line-def.cpp

Index: clang/test/SemaTemplate/concepts-out-of-line-def.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/concepts-out-of-line-def.cpp
@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 -std=c++20 -verify %s
+// expected-no-diagnostics
+
+static constexpr int PRIMARY = 0;
+static constexpr int SPECIALIZATION_CONCEPT = 1;
+static constexpr int SPECIALIZATION_REQUIRES = 2;
+
+template 
+concept Concept = (sizeof(T) >= 2 * sizeof(int));
+
+struct XY {
+  int x;
+  int y;
+};
+
+namespace members {
+
+template  struct S {
+  static constexpr int primary();
+};
+
+template  constexpr int S::primary() {
+  return PRIMARY;
+};
+
+template  struct S {
+  static constexpr int specialization();
+};
+
+template 
+  requires(sizeof(T) == sizeof(int))
+struct S {
+  static constexpr int specialization();
+};
+
+template  constexpr int S::specialization() {
+  return SPECIALIZATION_CONCEPT;
+}
+
+template 
+  requires(sizeof(T) == sizeof(int))
+constexpr int S::specialization() {
+  return SPECIALIZATION_REQUIRES;
+}
+
+static_assert(S::primary() == PRIMARY);
+static_assert(S::specialization() == SPECIALIZATION_CONCEPT);
+static_assert(S::specialization() == SPECIALIZATION_REQUIRES);
+
+} // namespace members
+
+namespace enumerations {
+
+template  struct S {
+  enum class E : int;
+};
+
+template  enum class S::E { Value = PRIMARY };
+
+template  struct S {
+  enum class E : int;
+};
+
+template 
+enum class S::E {
+  Value = SPECIALIZATION_CONCEPT
+};
+
+template 
+  requires(sizeof(T) == sizeof(int))
+struct S {
+  enum class E : int;
+};
+
+template 
+  requires(sizeof(T) == sizeof(int))
+enum class S::E {
+  Value = SPECIALIZATION_REQUIRES
+};
+
+static_assert(static_cast(S::E::Value) == PRIMARY);
+static_assert(static_cast(S::E::Value) ==
+  SPECIALIZATION_CONCEPT);
+static_assert(static_cast(S::E::Value) ==
+  SPECIALIZATION_REQUIRES);
+
+} // namespace  enumerations
+
+namespace multiple_template_parameter_lists {
+
+template 
+struct S {
+  template 
+  static constexpr int primary(Inner);
+};
+
+template 
+template 
+constexpr int S::primary(Inner) {
+  return PRIMARY;
+};
+
+template 
+struct S {
+  template 
+  static constexpr int specialization(Inner);
+};
+
+template 
+template 
+constexpr int S::specialization(Inner) { return SPECIALIZATION_CONCEPT; }
+
+template 
+  requires(sizeof(Outer) == sizeof(int))
+struct S {
+  template 
+  static constexpr int specialization(Inner);
+};
+
+template 
+  requires(sizeof(Outer) == sizeof(int))
+template 
+constexpr int S::specialization(Inner) { return SPECIALIZATION_REQUIRES; }
+
+static_assert(S::primary("str") == PRIMARY);
+static_assert(S::specialization("str") == SPECIALIZATION_CONCEPT);
+static_assert(S::specialization("str") == SPECIALIZATION_REQUIRES);
+
+} // namespace multiple_template_parameter_lists
Index: clang/test/CXX/temp/temp.decls/temp.class.spec/temp.class.spec.mfunc/p1-neg.cpp
===
--- clang/test/CXX/temp/temp.decls/temp.class.spec/temp.class.spec.mfunc/p1-neg.cpp
+++ clang/test/CXX/temp/temp.decls/temp.class.spec/temp.class.spec.mfunc/p1-neg.cpp
@@ -3,7 +3,7 @@
 template
 struct A;
 
-template // expected-note{{previous template declaration}}
+template
 struct A {
   void f0();
   void f1();
@@ -15,11 +15,10 @@
   void g0();
 };
 
-// FIXME: We should probably give more precise diagnostics here, but the
-// diagnostics we give aren't terrible.
-// FIXME: why not point to the first parameter that's "too many"?
-template // expected-error{{too many template parameters}}
-void A::f0() { }
+// FIXME: We should produce diagnostics pointing out the
+// non-matching candidates.
+template
+void A::f0() { } // expected-error{{does not refer into a class, class template or class template partial specialization}}
 
 template
 void A::f1() { } // expected-error{{out-of-line definition}}
Index: clang/lib/Sema/SemaCXXScopeSpec.cpp
===
--- clang/lib/Sema/SemaCXXScopeSpec.cpp
+++ clang/lib/Sema/SemaCXXScopeSpec.cpp
@@ -99,34 +99,52 @@
 if (ClassTemplateDecl *ClassTemplate
   = 

[clang] 421c098 - [Clang][Sema] Start fixing handling of out-of-line definitions of constrained templates

[Alexander Shaposhnikov via cfe-commits]

Author: Alexander Shaposhnikov
Date: 2023-03-10T09:21:09Z
New Revision: 421c098b32bd50122de8de03a71092c7f36994eb

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

LOG: [Clang][Sema] Start fixing handling of out-of-line definitions of 
constrained templates

This diff starts fixing our handling of out-of-line definitions of constrained 
templates.
Initially it was motivated by https://github.com/llvm/llvm-project/issues/49620 
and
https://github.com/llvm/llvm-project/issues/60231.
In particular, this diff adjusts Sema::computeDeclContext to work properly in 
the case of
constrained template parameters.

Test plan:
1/ ninja check-all
2/ Bootstrapped Clang passes all the tests
3/ Internal testing

Differential revision: https://reviews.llvm.org/D145034

Added: 
clang/test/SemaTemplate/concepts-out-of-line-def.cpp

Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/DeclSpec.h
clang/lib/Parse/ParseDecl.cpp
clang/lib/Parse/ParseDeclCXX.cpp
clang/lib/Sema/SemaCXXScopeSpec.cpp

clang/test/CXX/temp/temp.decls/temp.class.spec/temp.class.spec.mfunc/p1-neg.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 332431e08ce07..8d880a4fba266 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -78,6 +78,8 @@ C++ Language Changes
 
 C++20 Feature Support
 ^
+- Support for out-of-line definitions of constrained templates has been 
improved.
+  This partially fixes `https://github.com/llvm/llvm-project/issues/49620`.
 
 C++2b Feature Support
 ^

diff  --git a/clang/include/clang/Parse/Parser.h 
b/clang/include/clang/Parse/Parser.h
index 96963fb6aa807..65111b1ac6b36 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -2513,7 +2513,8 @@ class Parser : public CodeCompletionHandler {
   /// this is a constructor declarator.
   bool isConstructorDeclarator(
   bool Unqualified, bool DeductionGuide = false,
-  DeclSpec::FriendSpecified IsFriend = DeclSpec::FriendSpecified::No);
+  DeclSpec::FriendSpecified IsFriend = DeclSpec::FriendSpecified::No,
+  const ParsedTemplateInfo *TemplateInfo = nullptr);
 
   /// Specifies the context in which type-id/expression
   /// disambiguation will occur.

diff  --git a/clang/include/clang/Sema/DeclSpec.h 
b/clang/include/clang/Sema/DeclSpec.h
index 69fe2c541607b..b0bf87dc18d79 100644
--- a/clang/include/clang/Sema/DeclSpec.h
+++ b/clang/include/clang/Sema/DeclSpec.h
@@ -62,9 +62,18 @@ namespace clang {
 /// often used as if it meant "present".
 ///
 /// The actual scope is described by getScopeRep().
+///
+/// If the kind of getScopeRep() is TypeSpec then TemplateParamLists may be 
empty
+/// or contain the template parameter lists attached to the current 
declaration.
+/// Consider the following example:
+/// template  void SomeType::some_method() {}
+/// If CXXScopeSpec refers to SomeType then TemplateParamLists will contain
+/// a single element referring to template .
+
 class CXXScopeSpec {
   SourceRange Range;
   NestedNameSpecifierLocBuilder Builder;
+  ArrayRef TemplateParamLists;
 
 public:
   SourceRange getRange() const { return Range; }
@@ -74,6 +83,13 @@ class CXXScopeSpec {
   SourceLocation getBeginLoc() const { return Range.getBegin(); }
   SourceLocation getEndLoc() const { return Range.getEnd(); }
 
+  void setTemplateParamLists(ArrayRef L) {
+TemplateParamLists = L;
+  }
+  ArrayRef getTemplateParamLists() const {
+return TemplateParamLists;
+  }
+
   /// Retrieve the representation of the nested-name-specifier.
   NestedNameSpecifier *getScopeRep() const {
 return Builder.getRepresentation();

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index 3106571728692..da84da04e43d0 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -3380,6 +3380,8 @@ void Parser::ParseDeclarationSpecifiers(
 goto DoneWithDeclSpec;
 
   CXXScopeSpec SS;
+  if (TemplateInfo.TemplateParams)
+SS.setTemplateParamLists(*TemplateInfo.TemplateParams);
   Actions.RestoreNestedNameSpecifierAnnotation(Tok.getAnnotationValue(),
Tok.getAnnotationRange(),
SS);
@@ -3475,7 +3477,8 @@ void Parser::ParseDeclarationSpecifiers(
  ) &&
   isConstructorDeclarator(/*Unqualified=*/false,
   /*DeductionGuide=*/false,
-  DS.isFriendSpecified()))
+  DS.isFriendSpecified(),
+  ))
 goto 

[PATCH] D141194: [clang][Interp] Implement bitcasts

[Timm Bäder via Phabricator via cfe-commits]

tbaeder abandoned this revision.
tbaeder added a comment.

Abandoning this since it's superseded by https://reviews.llvm.org/D144943


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141194

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