[PATCH] D112642: [clang][NFC] Inclusive terms: Replace uses of whitelist in clang/lib/StaticAnalyzer

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
Herald added a subscriber: rnkovacs.

Thanks @ZarkoCA, LGTM, but please address the sensible changes advised by 
Aaron. And thanks @aaron.ballman for the review!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112642

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

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

> people DO incorporate clang-tidy into their own commercial offerings

Gna, that //can// be a problem. I wonder if in that case it would be possible 
to add a few lines into the `LLVM Exceptions` part of the license. If it's too 
much of a hassle I guess I'll need to drop it and continue on a local fork 
unfortunately.

It's interesting however that this fork has implemented quite a few AUTOSAR 
checks keeping the existing LLVM license:
https://github.com/Bareflank/llvm-project/blob/bsl-tidy


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

https://reviews.llvm.org/D112730

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

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added reviewers: alexfh, tonic.
aaron.ballman added a subscriber: tonic.
aaron.ballman added a comment.

In D112730#3095767 , @carlosgalvezp 
wrote:

> Also, I've sent a mail to AUTOSAR directly asking for consent, so there's no 
> doubt.

Thank you for reaching out to them. I am also not a lawyer, but that license 
worries me because people DO incorporate clang-tidy into their own commercial 
offerings (downstream), so we could be creating a legal hassle for them. We may 
need to reach out to the LLVM Foundation for guidance depending on what the 
AUTOSAR folks come back with (adding @tonic here for awareness as llvm 
foundation president).


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

https://reviews.llvm.org/D112730

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

[PATCH] D112773: Allow __attribute__((swift_attr)) in attribute push pragmas

[Aaron Ballman via Phabricator via cfe-commits]

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

`swift_attr` has no subjects, so this will attempt to spray the attribute onto 
literally *everything*. That makes this incredibly risky to use with the pragma 
approach (not to mention what it could do to memory consumption of large ASTs). 
I'm not certain I'm comfortable allowing this without an explicit set of 
subjects for the attribute. I took a look and the only other attribute 
currently allowed there is `annotate`, and I'm not convinced that was a good 
idea to allow for use in this pragma. Can we add subjects to this attribute to 
try to limit the damage somewhat?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112773

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

[PATCH] D103395: PR45879: Keep evaluated expression in LValue object

[Raul Tambre via Phabricator via cfe-commits]

tambre added a comment.

This breaks the following code:

  struct sub
  {
char data;
  };
  
  struct main
  {
constexpr main()
{
member = {};
}
  
sub member;
  };
  
  constexpr main a{};

With:

  fmt.cpp:16:16: error: constexpr variable 'a' must be initialized by a 
constant expression
  constexpr main a{};
 ^~~
  1 error generated.

Clang trunk and GCC (Debian 11.2.0-10) handle it fine.
Noticed in libfmt 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103395

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

[PATCH] D112409: [clang-tidy] Add check 'cert-err33-c'.

[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/D112409/new/

https://reviews.llvm.org/D112409

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

[PATCH] D111199: [Clang][LLVM][Attr] support btf_type_tag attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D99#3095623 , @yonghong-song 
wrote:

>> Ah, yeah, I see what you mean - that does seem sort of unfortunate. Is it 
>> possible these attributes could only appear on typedefs and they'd be more 
>> readily carried through that without needing extra typeloc tracking? (sorry 
>> for not having read back through the rest of the review - which may've 
>> started there and ended up here as a more general form of the attribute?)
>
> For the question, "is it possible these attributes could only appear on 
> typedefs?" The answer is "not really possible". We are targeting existing 
> linux kernel where existing type attributes (__user, __rcu, ...) have been 
> used in places other than typedef quite extensively (e.g., function argument 
> type, function return type, field type, etc.).
>
> In one of my earlier prototypes, I put the tag string itself in 
> AttributedType and with this we can avoid TypeLoc, but I understand this is 
> not conventional usage and that is why we do through TypeLoc mechanism. 
> @aaron.ballman can further comment on this.

FWIW, I made that request because AttributedTypeLoc stores the Attr * for the 
attributed type, so we can get the attribute argument information from that 
rather than having to duplicate it within a new TypeLoc object.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99

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

[PATCH] D112059: Fix inline builtin handling in case of redefinition

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM aside from a formatting nit. I don't think the precommit CI failures are 
related to your patch from what I was seeing, but may be worth keeping an eye 
on once you land just in case.




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:1323
+else {
+  for(const FunctionDecl* PD = FD->getPreviousDecl(); PD; PD = 
PD->getPreviousDecl()) {
+if(LLVM_UNLIKELY(PD->isInlineBuiltinDeclaration())) {

Please fix the formatting.


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

https://reviews.llvm.org/D112059

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

[PATCH] D111797: [clang][scan-build] Use uname -s to detect the operating system.

[Frederic Cambus via Phabricator via cfe-commits]

fcambus added a comment.

Sure, I reverted to the previous revision.


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

https://reviews.llvm.org/D111797

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

[PATCH] D111797: [clang][scan-build] Use uname -s to detect the operating system.

[Frederic Cambus via Phabricator via cfe-commits]

fcambus updated this revision to Diff 383293.

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

https://reviews.llvm.org/D111797

Files:
  clang/tools/scan-build/libexec/ccc-analyzer


Index: clang/tools/scan-build/libexec/ccc-analyzer
===
--- clang/tools/scan-build/libexec/ccc-analyzer
+++ clang/tools/scan-build/libexec/ccc-analyzer
@@ -72,7 +72,7 @@
 # If on OSX, use xcrun to determine the SDK root.
 my $UseXCRUN = 0;
 
-if (`uname -a` =~ m/Darwin/) {
+if (`uname -s` =~ m/Darwin/) {
   $DefaultCCompiler = 'clang';
   $DefaultCXXCompiler = 'clang++';
   # Older versions of OSX do not have xcrun to
@@ -80,7 +80,7 @@
   if (-x "/usr/bin/xcrun") {
 $UseXCRUN = 1;
   }
-} elsif (`uname -a` =~ m/OpenBSD/) {
+} elsif (`uname -s` =~ m/OpenBSD/) {
   $DefaultCCompiler = 'cc';
   $DefaultCXXCompiler = 'c++';
 } else {


Index: clang/tools/scan-build/libexec/ccc-analyzer
===
--- clang/tools/scan-build/libexec/ccc-analyzer
+++ clang/tools/scan-build/libexec/ccc-analyzer
@@ -72,7 +72,7 @@
 # If on OSX, use xcrun to determine the SDK root.
 my $UseXCRUN = 0;
 
-if (`uname -a` =~ m/Darwin/) {
+if (`uname -s` =~ m/Darwin/) {
   $DefaultCCompiler = 'clang';
   $DefaultCXXCompiler = 'clang++';
   # Older versions of OSX do not have xcrun to
@@ -80,7 +80,7 @@
   if (-x "/usr/bin/xcrun") {
 $UseXCRUN = 1;
   }
-} elsif (`uname -a` =~ m/OpenBSD/) {
+} elsif (`uname -s` =~ m/OpenBSD/) {
   $DefaultCCompiler = 'cc';
   $DefaultCXXCompiler = 'c++';
 } else {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG32f102912493: [clangd] Track performance of IncludeCleaner 
(authored by kbobyrev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112783

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


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure ,
   const llvm::DenseSet ) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion  : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet ,
  const IncludeStructure ,
  const SourceManager ) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST ,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config  = Config::current();
   if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
   Cfg.Diagnostics.SuppressAll ||


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure ,
   const llvm::DenseSet ) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion  : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet ,
  const IncludeStructure ,
  const SourceManager ) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST ,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config  = Config::current();
   if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
   Cfg.Diagnostics.SuppressAll ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] 32f1029 - [clangd] Track performance of IncludeCleaner

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2021-10-29T12:59:22+02:00
New Revision: 32f102912493db3df864cc80d5bb602962839f68

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

LOG: [clangd] Track performance of IncludeCleaner

Reviewed By: kadircet

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

Added: 


Modified: 
clang-tools-extra/clangd/IncludeCleaner.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 26ae356e243e9..240eb864d087c 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@ bool mayConsiderUnused(const Inclusion *Inc) {
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@ findReferencedFiles(const llvm::DenseSet 
,
 std::vector
 getUnused(const IncludeStructure ,
   const llvm::DenseSet ) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion  : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@ llvm::DenseSet
 translateToHeaderIDs(const llvm::DenseSet ,
  const IncludeStructure ,
  const SourceManager ) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@ std::vector 
computeUnusedIncludes(ParsedAST ) {
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST ,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config  = Config::current();
   if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
   Cfg.Diagnostics.SuppressAll ||



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

[PATCH] D105168: [RISCV] Unify the arch string parsing logic to RISCVISAInfo.

[Zixuan Wu via Phabricator via cfe-commits]

zixuan-wu added a comment.
Herald added subscribers: VincentWu, luke957.

Hi, @kito-cheng as your this patch unify the extension handing in one same 
place by new infra, are you going to support the multiple extension version in 
next step?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105168

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

[PATCH] D106339: Add support to generate Sphinx DOCX documentation

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.
Herald added a project: Flang.

If there is no plan to move forward with generating `.docx` documentation, can 
we please abandon this review? I'm trying to clean up libc++'s review queue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106339

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

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

oops forgot to LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112783

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

[PATCH] D110927: [analyzer] Access stored value of a constant array through a pointer to another type

[Gabor Marton via Phabricator via cfe-commits]

martong added a subscriber: vabridgers.
martong added a comment.

Adding @vabridgers as a subscriber, he might be interested in this.




Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1675
+   //  belong to an array with one element of type T.
+   // Hence, the first element can be retrieved only. At least untill a
+   // paper P1839R0 be considered by the committee.

typo: `untill` -> `until`



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1758
+  // type.
+  QualType ElemT = Ctx.getCanonicalType(R->getElementType());
+  if (!canAccessStoredValue(ArrT, ElemT, I))

steakhal wrote:
> If you already compute the //canonical type// why do you recompute in the 
> `canAccessStoredValue()`?
> You could simply assert that instead.
He removes the qualifiers there, but getting the canonical type is probably 
redundant here.



Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1759-1760
+  QualType ElemT = Ctx.getCanonicalType(R->getElementType());
+  if (!canAccessStoredValue(ArrT, ElemT, I))
+return UndefinedVal();
+

ASDenysPetrov wrote:
> steakhal wrote:
> > Even though I agree with you, I think it would be useful to hide this 
> > behavior behind an analyzer option.
> > There is quite a lot code out in the wild that violate the 
> > //strict-aliasing// rule and they probably pass the `-fno-strict-aliasing` 
> > compiler flag to accommodate this in codegen. AFAIK Linux is one of these 
> > projects for example.
> > So, I think there is a need to opt-out of this and/or bind the behavior to 
> > the presence of the mentioned compiler flag.
> > 
> > By not doing this, the user would get //garbage// value reports all over 
> > the place.
> > @NoQ @martong WDYT?
> > There is quite a lot code out in the wild that violate the strict-aliasing 
> > rule 
> Agree.
> > By not doing this, the user would get garbage value reports all over the 
> > place.
> Definitely.
> Using the flag is a good option. But the question whether to use existing 
> `-fno-strict-aliasing` or introduce a new one?
I think we could simply reuse the existing `-fno-strict-aliasing`.


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

https://reviews.llvm.org/D110927

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

[PATCH] D112289: Support: Use sys::path::is_style_{posix,windows}() in a few places

[Martin Storsjö via Phabricator via cfe-commits]

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

LGTM, too. D112786  does a couple more things 
that do pretty much the same - I don't mind if you want to fold them into this, 
or keep it as-is, with the bits that are closer to separator handling split out 
for clarity.


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

https://reviews.llvm.org/D112289

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

[PATCH] D112796: [ASTImporter] Remove redundant IsStructuralMatch overloads

[Raphael Isemann 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 rG96808c69a13c: [ASTImporter] Remove redundant 
IsStructuralMatch overloads (authored by teemperor).
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112796

Files:
  clang/lib/AST/ASTImporter.cpp

Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -467,18 +467,8 @@
 template 
 bool hasSameVisibilityContextAndLinkage(T *Found, T *From);
 
-bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
-bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
-   bool Complain = true);
-bool IsStructuralMatch(VarDecl *FromVar, VarDecl *ToVar,
-   bool Complain = true);
-bool IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToRecord);
+bool IsStructuralMatch(Decl *From, Decl *To, bool Complain = true);
 bool IsStructuralMatch(EnumConstantDecl *FromEC, EnumConstantDecl *ToEC);
-bool IsStructuralMatch(FunctionTemplateDecl *From,
-   FunctionTemplateDecl *To);
-bool IsStructuralMatch(FunctionDecl *From, FunctionDecl *To);
-bool IsStructuralMatch(ClassTemplateDecl *From, ClassTemplateDecl *To);
-bool IsStructuralMatch(VarTemplateDecl *From, VarTemplateDecl *To);
 ExpectedDecl VisitDecl(Decl *D);
 ExpectedDecl VisitImportDecl(ImportDecl *D);
 ExpectedDecl VisitEmptyDecl(EmptyDecl *D);
@@ -2178,68 +2168,17 @@
 }
 
 bool ASTNodeImporter::IsStructuralMatch(Decl *From, Decl *To, bool Complain) {
-  StructuralEquivalenceContext Ctx(
-  Importer.getFromContext(), Importer.getToContext(),
-  Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
-  false, Complain);
-  return Ctx.IsEquivalent(From, To);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord,
-RecordDecl *ToRecord, bool Complain) {
   // Eliminate a potential failure point where we attempt to re-import
   // something we're trying to import while completing ToRecord.
-  Decl *ToOrigin = Importer.GetOriginalDecl(ToRecord);
+  Decl *ToOrigin = Importer.GetOriginalDecl(To);
   if (ToOrigin) {
-auto *ToOriginRecord = dyn_cast(ToOrigin);
-if (ToOriginRecord)
-  ToRecord = ToOriginRecord;
+To = ToOrigin;
   }
 
-  StructuralEquivalenceContext Ctx(Importer.getFromContext(),
-   ToRecord->getASTContext(),
-   Importer.getNonEquivalentDecls(),
-   getStructuralEquivalenceKind(Importer),
-   false, Complain);
-  return Ctx.IsEquivalent(FromRecord, ToRecord);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(VarDecl *FromVar, VarDecl *ToVar,
-bool Complain) {
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
   false, Complain);
-  return Ctx.IsEquivalent(FromVar, ToVar);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
-  // Eliminate a potential failure point where we attempt to re-import
-  // something we're trying to import while completing ToEnum.
-  if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
-if (auto *ToOriginEnum = dyn_cast(ToOrigin))
-ToEnum = ToOriginEnum;
-
-  StructuralEquivalenceContext Ctx(
-  Importer.getFromContext(), Importer.getToContext(),
-  Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer));
-  return Ctx.IsEquivalent(FromEnum, ToEnum);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(FunctionTemplateDecl *From,
-FunctionTemplateDecl *To) {
-  StructuralEquivalenceContext Ctx(
-  Importer.getFromContext(), Importer.getToContext(),
-  Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
-  false, false);
-  return Ctx.IsEquivalent(From, To);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(FunctionDecl *From, FunctionDecl *To) {
-  StructuralEquivalenceContext Ctx(
-  Importer.getFromContext(), Importer.getToContext(),
-  Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
-  false, false);
   return Ctx.IsEquivalent(From, To);
 }
 
@@ -2253,24 +2192,6 @@
  FromVal == ToVal;
 }
 
-bool ASTNodeImporter::IsStructuralMatch(ClassTemplateDecl *From,
-ClassTemplateDecl *To) {
-  StructuralEquivalenceContext Ctx(Importer.getFromContext(),
-   Importer.getToContext(),

[clang] 96808c6 - [ASTImporter] Remove redundant IsStructuralMatch overloads

[Raphael Isemann via cfe-commits]

Author: Raphael Isemann
Date: 2021-10-29T12:23:38+02:00
New Revision: 96808c69a13c68280c2808b04dc5b733193bef6d

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

LOG: [ASTImporter] Remove redundant IsStructuralMatch overloads

Nearly all of the overloads have pretty much the same behaviour. The only
exception here is that some of them call back `GetOriginalDecl` and others
don't, but the only real user of that overload (which is LLDB) actually prefers
getting this callback.

Reviewed By: martong

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

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 4e74355f2639a..183849c86f01c 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -467,18 +467,8 @@ namespace clang {
 template 
 bool hasSameVisibilityContextAndLinkage(T *Found, T *From);
 
-bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
-bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
-   bool Complain = true);
-bool IsStructuralMatch(VarDecl *FromVar, VarDecl *ToVar,
-   bool Complain = true);
-bool IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToRecord);
+bool IsStructuralMatch(Decl *From, Decl *To, bool Complain = true);
 bool IsStructuralMatch(EnumConstantDecl *FromEC, EnumConstantDecl *ToEC);
-bool IsStructuralMatch(FunctionTemplateDecl *From,
-   FunctionTemplateDecl *To);
-bool IsStructuralMatch(FunctionDecl *From, FunctionDecl *To);
-bool IsStructuralMatch(ClassTemplateDecl *From, ClassTemplateDecl *To);
-bool IsStructuralMatch(VarTemplateDecl *From, VarTemplateDecl *To);
 ExpectedDecl VisitDecl(Decl *D);
 ExpectedDecl VisitImportDecl(ImportDecl *D);
 ExpectedDecl VisitEmptyDecl(EmptyDecl *D);
@@ -2178,68 +2168,17 @@ getStructuralEquivalenceKind(const ASTImporter 
) {
 }
 
 bool ASTNodeImporter::IsStructuralMatch(Decl *From, Decl *To, bool Complain) {
-  StructuralEquivalenceContext Ctx(
-  Importer.getFromContext(), Importer.getToContext(),
-  Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
-  false, Complain);
-  return Ctx.IsEquivalent(From, To);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(RecordDecl *FromRecord,
-RecordDecl *ToRecord, bool Complain) {
   // Eliminate a potential failure point where we attempt to re-import
   // something we're trying to import while completing ToRecord.
-  Decl *ToOrigin = Importer.GetOriginalDecl(ToRecord);
+  Decl *ToOrigin = Importer.GetOriginalDecl(To);
   if (ToOrigin) {
-auto *ToOriginRecord = dyn_cast(ToOrigin);
-if (ToOriginRecord)
-  ToRecord = ToOriginRecord;
+To = ToOrigin;
   }
 
-  StructuralEquivalenceContext Ctx(Importer.getFromContext(),
-   ToRecord->getASTContext(),
-   Importer.getNonEquivalentDecls(),
-   getStructuralEquivalenceKind(Importer),
-   false, Complain);
-  return Ctx.IsEquivalent(FromRecord, ToRecord);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(VarDecl *FromVar, VarDecl *ToVar,
-bool Complain) {
   StructuralEquivalenceContext Ctx(
   Importer.getFromContext(), Importer.getToContext(),
   Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
   false, Complain);
-  return Ctx.IsEquivalent(FromVar, ToVar);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(EnumDecl *FromEnum, EnumDecl *ToEnum) {
-  // Eliminate a potential failure point where we attempt to re-import
-  // something we're trying to import while completing ToEnum.
-  if (Decl *ToOrigin = Importer.GetOriginalDecl(ToEnum))
-if (auto *ToOriginEnum = dyn_cast(ToOrigin))
-ToEnum = ToOriginEnum;
-
-  StructuralEquivalenceContext Ctx(
-  Importer.getFromContext(), Importer.getToContext(),
-  Importer.getNonEquivalentDecls(), 
getStructuralEquivalenceKind(Importer));
-  return Ctx.IsEquivalent(FromEnum, ToEnum);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(FunctionTemplateDecl *From,
-FunctionTemplateDecl *To) {
-  StructuralEquivalenceContext Ctx(
-  Importer.getFromContext(), Importer.getToContext(),
-  Importer.getNonEquivalentDecls(), getStructuralEquivalenceKind(Importer),
-  false, false);
-  return Ctx.IsEquivalent(From, To);
-}
-
-bool ASTNodeImporter::IsStructuralMatch(FunctionDecl *From, FunctionDecl *To) {
-  StructuralEquivalenceContext Ctx(
-  Importer.getFromContext(), 

[PATCH] D112791: [IR] Merge createReplacementInstr into ConstantExpr::getAsInstruction

[Mahesha S via Phabricator via cfe-commits]

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

Thanks for this clean-up patch. Looks good to me. However, please wait for some 
time if in case other reviewers have any comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112791

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

[PATCH] D51650: Implement target_clones multiversioning

[Daniel Kiss via Phabricator via cfe-commits]

danielkiss added a comment.

Hi @erichkeane,

At Arm we are going to add the multiversioning support for Arm targets[1]. It 
would be nice to land this change because we could build top of it.
Please let me know how can I help.

[1]. https://github.com/ARM-software/acle/pull/21


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

https://reviews.llvm.org/D51650

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

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 383278.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112783

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


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure ,
   const llvm::DenseSet ) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion  : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet ,
  const IncludeStructure ,
  const SourceManager ) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST ,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config  = Config::current();
   if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
   Cfg.Diagnostics.SuppressAll ||


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure ,
   const llvm::DenseSet ) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion  : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet ,
  const IncludeStructure ,
  const SourceManager ) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST ,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config  = Config::current();
   if (Cfg.Diagnostics.UnusedIncludes != Config::UnusedIncludesPolicy::Strict ||
   Cfg.Diagnostics.SuppressAll ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112791: [IR] Merge createReplacementInstr into ConstantExpr::getAsInstruction

[Jay Foad via Phabricator via cfe-commits]

foad created this revision.
Herald added subscribers: ormris, dexonsmith, hiraditya.
foad requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

createReplacementInstr was a trivial wrapper around
ConstantExpr::getAsInstruction, which also inserted the new instruction
into a basic block. Implement this directly in getAsInstruction by
adding an InsertBefore parameter and change all callers to use it. NFC.

A follow-up patch will remove createReplacementInstr.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112791

Files:
  clang/lib/CodeGen/CGCUDANV.cpp
  llvm/include/llvm/IR/Constants.h
  llvm/lib/IR/Constants.cpp
  llvm/lib/IR/ReplaceConstant.cpp
  llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
  llvm/lib/Transforms/IPO/GlobalOpt.cpp
  llvm/lib/Transforms/Scalar/ConstantHoisting.cpp

Index: llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
===
--- llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
+++ llvm/lib/Transforms/Scalar/ConstantHoisting.cpp
@@ -819,10 +819,9 @@
 
 // Aside from constant GEPs, only constant cast expressions are collected.
 assert(ConstExpr->isCast() && "ConstExpr should be a cast");
-Instruction *ConstExprInst = ConstExpr->getAsInstruction();
+Instruction *ConstExprInst = ConstExpr->getAsInstruction(
+findMatInsertPt(ConstUser.Inst, ConstUser.OpndIdx));
 ConstExprInst->setOperand(0, Mat);
-ConstExprInst->insertBefore(findMatInsertPt(ConstUser.Inst,
-ConstUser.OpndIdx));
 
 // Use the same debug location as the instruction we are about to update.
 ConstExprInst->setDebugLoc(ConstUser.Inst->getDebugLoc());
Index: llvm/lib/Transforms/IPO/GlobalOpt.cpp
===
--- llvm/lib/Transforms/IPO/GlobalOpt.cpp
+++ llvm/lib/Transforms/IPO/GlobalOpt.cpp
@@ -1490,8 +1490,7 @@
 append_range(UUsers, U->users());
 for (auto *UU : UUsers) {
   Instruction *UI = cast(UU);
-  Instruction *NewU = U->getAsInstruction();
-  NewU->insertBefore(UI);
+  Instruction *NewU = U->getAsInstruction(UI);
   UI->replaceUsesOfWith(U, NewU);
 }
 // We've replaced all the uses, so destroy the constant. (destroyConstant
Index: llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
===
--- llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
+++ llvm/lib/Target/XCore/XCoreLowerThreadLocal.cpp
@@ -21,7 +21,6 @@
 #include "llvm/IR/IntrinsicsXCore.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/NoFolder.h"
-#include "llvm/IR/ReplaceConstant.h"
 #include "llvm/IR/ValueHandle.h"
 #include "llvm/Pass.h"
 #include "llvm/Support/CommandLine.h"
@@ -90,11 +89,11 @@
   if (PredBB->getTerminator()->getNumSuccessors() > 1)
 PredBB = SplitEdge(PredBB, PN->getParent());
   Instruction *InsertPos = PredBB->getTerminator();
-  Instruction *NewInst = createReplacementInstr(CE, InsertPos);
+  Instruction *NewInst = CE->getAsInstruction(InsertPos);
   PN->setOperand(I, NewInst);
 }
 } else if (Instruction *Instr = dyn_cast(WU)) {
-  Instruction *NewInst = createReplacementInstr(CE, Instr);
+  Instruction *NewInst = CE->getAsInstruction(Instr);
   Instr->replaceUsesOfWith(CE, NewInst);
 } else {
   ConstantExpr *CExpr = dyn_cast(WU);
@@ -103,7 +102,7 @@
 }
   }
   } while (CE->hasNUsesOrMore(1)); // We need to check because a recursive
-  // sibling may have used 'CE' when createReplacementInstr was called.
+  // sibling may have used 'CE' when getAsInstruction was called.
   CE->destroyConstant();
   return true;
 }
Index: llvm/lib/IR/ReplaceConstant.cpp
===
--- llvm/lib/IR/ReplaceConstant.cpp
+++ llvm/lib/IR/ReplaceConstant.cpp
@@ -20,9 +20,7 @@
 // Replace a constant expression by instructions with equivalent operations at
 // a specified location.
 Instruction *createReplacementInstr(ConstantExpr *CE, Instruction *Instr) {
-  auto *CEInstr = CE->getAsInstruction();
-  CEInstr->insertBefore(Instr);
-  return CEInstr;
+  return CE->getAsInstruction(Instr);
 }
 
 void convertConstantExprsToInstructions(Instruction *I, ConstantExpr *CE,
@@ -63,8 +61,7 @@
   for (auto *CE : Path) {
 if (!Visited.insert(CE).second)
   continue;
-auto *NI = CE->getAsInstruction();
-NI->insertBefore(BI);
+auto *NI = CE->getAsInstruction(BI);
 II->replaceUsesOfWith(CE, NI);
 CE->removeDeadConstantUsers();
 BI = II = NI;
Index: llvm/lib/IR/Constants.cpp
===
--- llvm/lib/IR/Constants.cpp
+++ llvm/lib/IR/Constants.cpp
@@ -3492,7 +3492,7 @@
   

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:275
 std::vector computeUnusedIncludes(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::computeUnusedIncludes");
   const auto  = AST.getSourceManager();

this doesn't look interesting on its own, as it doesn't perform much magic. 
maybe just drop?



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:284
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST ,
  llvm::StringRef Code) {

i believe this is the function that deserves a trace the most, as it is alive 
throughout the whole interaction and will reflect full latency.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112783

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

[PATCH] D112777: [X86][FP16] add alias for *_fmul_pch intrinsics

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

> *_mul_pch is to align with *_mul_ps annd *_mul_pd

And *_mul_ph?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112777

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

[PATCH] D108696: [Coroutines] [Frontend] Lookup in std namespace first

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu marked 5 inline comments as done.
ChuanqiXu added a comment.

In D108696#3090484 , @Quuxplusone 
wrote:

> @lewissbaker wrote:
>
>>   #include  // which transitively includes 
>>   #include 
>
> Good example! I had not previously been thinking about transitive includes. I 
> believe we "obviously" don't need to cater to code that manually includes 
> both `` and `` in the same source file; 
> but transitive includes are //vastly// more likely to happen in practice, and 
> so if we're not going to care about //them//, that's a policy decision. Might 
> still be a good tradeoff, to break some code in the short term in exchange 
> for a simpler compiler (also in the short term), but its goodness is not 
> //obvious.//
>
>> The only way I can think of making this work is to just make 
>> `std::experimental::*` an alias for `std::*`.
>> But that only works for `std::experimental::coroutine_handle`. It doesn't 
>> work for `std::experimental::coroutine_traits` as you can't add 
>> specialisations through an alias.
>
> We //could// use a `using`-declaration to bring `std::coroutine_traits` into 
> `namespace std::experimental`. That works, and you can still add 
> specializations for `std::experimental::coroutine_traits` because that's 
> just a //name// that looks-up-to the same template. 
> https://godbolt.org/z/fWGrT5js5 However, as shown in that godbolt, this would 
> have the (salutary) effect of breaking users who are out there (in the year 
> of our lord 2021!) still reopening `namespace std` just to add a template 
> specialization.
>
> But! My understanding is that the only reason we're keeping 
> `` alive at all, in 14.x, is to provide continuity 
> for its users and not break their existing code right away. If we go changing 
> the API of `` (by aliasing it to ``), then 
> we //do// break those users right away (because their code depends on the old 
> experimental API, not the new conforming one). So "alias it to ``" 
> doesn't seem like a viable path forward, anyway. (Also, `` wants 
> to use C++20-only features, but `` must continue to 
> work in C++14.)  I think we need to start from the premise that 
> `` and `` will have different APIs; and if 
> that makes it difficult to support Lewis's very reasonable transitive-include 
> scenario, then we have to either implement something difficult, or else make 
> a policy decision that 14.x simply won't support translation units that 
> transitively include both APIs. (15.x certainly will not support such TUs, 
> because it won't support //any// TUs that include ``, 
> transitively or otherwise.)
>
> IOW, it sounds like we're all (@ChuanqiXu @lewissbaker @Quuxplusone) 
> reluctantly OK with the resolution "Do not support translation units that 
> transitively include both APIs"; but it would be helpful to have someone more 
> authoritative weigh in (with either "yes that's OK policy" or "no we //need// 
> to find some other solution"), if such a person is watching.

Yeah, the last key point here may be the problem that how do we treat for 
programs which contains both APIs. Since there are other `experimental/*` 
headers moved in to normal include paths, I guess there may be similar problems 
before. I think this problem is not limited in coroutine. So how does libc++ do 
before for this situation @Quuxplusone ?

Since it looks like that people here may agree that we don't support both APIs. 
So I add an error if the compiler founds the mixed use of `std::coro*` and 
`std::experimental::coro*`. I think this is the best we could do if we decide 
to not support it.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11002
 def err_coroutine_handle_missing_member : Error<
-  "std::experimental::coroutine_handle missing a member named '%0'">;
+  "std::coroutine_handle missing a member named '%0'">;
 def err_malformed_std_coroutine_traits : Error<

Quuxplusone wrote:
> Pre-existing: It's weird that the surrounding messages are of the form "foo 
> must be bar," and then this one is "foo isn't baz". This message //could// be 
> re-worded as `std::coroutine_handle must have a member named '%0'` for 
> consistency. (But that might be out of scope for this PR.)
Oh, many thanks for the detailed reviews. I edit this to the way you 
introduced. Otherwise we couldn't remember this defect after this patch.


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

https://reviews.llvm.org/D108696

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

[PATCH] D111062: [RISCV] Rename some assembler mnemonic and intrinsic functions for RVV 1.0.

[Fraser Cormack via Phabricator via cfe-commits]

frasercrmck accepted this revision.
frasercrmck added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: luke957.

Thanks @khchen, that's great. LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111062

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

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

Also, I've sent a mail to AUTOSAR directly asking for consent, so there's no 
doubt.


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

https://reviews.llvm.org/D112730

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

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp updated this revision to Diff 383259.
carlosgalvezp added a comment.

Removed unnecessary dependency in CMakeLists.txt


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

https://reviews.llvm.org/D112730

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidyForceLinker.h
  clang-tools-extra/clang-tidy/autosar/AutosarTidyModule.cpp
  clang-tools-extra/clang-tidy/autosar/CMakeLists.txt
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/autosar-a5-4-2-reinterpret-cast.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/index.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-reinterpret-cast.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-reinterpret-cast.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-reinterpret-cast.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-pro-type-reinterpret-cast.cpp
@@ -1,6 +1,6 @@
-// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-reinterpret-cast %t
+// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-reinterpret-cast,autosar-a5-2-4-reinterpret-cast %t
 
 int i = 0;
 void *j;
 void f() { j = reinterpret_cast(i); }
-// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use reinterpret_cast [cppcoreguidelines-pro-type-reinterpret-cast]
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use reinterpret_cast [autosar-a5-2-4-reinterpret-cast,cppcoreguidelines-pro-type-reinterpret-cast]
Index: clang-tools-extra/docs/clang-tidy/index.rst
===
--- clang-tools-extra/docs/clang-tidy/index.rst
+++ clang-tools-extra/docs/clang-tidy/index.rst
@@ -60,6 +60,7 @@
 ``abseil-``Checks related to Abseil library.
 ``altera-``Checks related to OpenCL programming for FPGAs.
 ``android-``   Checks related to Android.
+``autosar-``   Checks related to AUTOSAR C++14 Coding Guidelines.
 ``boost-`` Checks related to Boost library.
 ``bugprone-``  Checks that target bugprone code constructs.
 ``cert-``  Checks related to CERT Secure Coding Guidelines.
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
@@ -50,6 +50,7 @@
`android-cloexec-pipe2 `_,
`android-cloexec-socket `_,
`android-comparison-in-temp-failure-retry `_,
+   `autosar-a5-4-2-reinterpret-cast `_,
`boost-use-to-string `_, "Yes"
`bugprone-argument-comment `_, "Yes"
`bugprone-assert-side-effect `_,
@@ -328,6 +329,7 @@
 .. csv-table:: Aliases..
:header: "Name", "Redirect", "Offers fixes"
 
+   `autosar-a5-4-2-reinterpret-cast `_, `cppcoreguidelines-pro-type-reinterpret-cast `_,
`cert-con36-c `_, `bugprone-spuriously-wake-up-functions `_,
`cert-con54-cpp `_, `bugprone-spuriously-wake-up-functions `_,
`cert-dcl03-c `_, `misc-static-assert `_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/autosar-a5-4-2-reinterpret-cast.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/autosar-a5-4-2-reinterpret-cast.rst
@@ -0,0 +1,10 @@
+.. title:: clang-tidy - autosar-a5-4-2-reinterpret-cast
+.. meta::
+   :http-equiv=refresh: 5;URL=cppcoreguidelines-pro-type-reinterpret-cast.html
+
+autosar-a5-4-2-reinterpret-cast
+===
+
+The autosar-a5-4-2-reinterpret-cast check is an alias, please see
+`cppcoreguidelines-pro-type-reinterpret-cast `_
+for more information.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,6 +67,8 @@
 Improvements to clang-tidy
 --
 
+- New module: `autosar`, covering the AUTOSAR C++14 Coding Guidelines.
+
 - Added support for globbing in `NOLINT*` expressions, to simplify suppressing
   multiple warnings in the same line.
 
@@ -103,6 +105,11 @@
 New check aliases
 ^
 
+- New alias :doc:`autosar-a5-4-2-reinterpret-cast
+  ` to
+  :doc:`cppcoreguidelines-pro-type-reinterpret-cast
+  ` was added.
+
 - New alias :doc:`cert-exp42-c
   ` to
   :doc:`bugprone-suspicious-memory-comparison
Index: clang-tools-extra/clang-tidy/autosar/CMakeLists.txt
===
--- /dev/null
+++ clang-tools-extra/clang-tidy/autosar/CMakeLists.txt
@@ -0,0 +1,26 @@
+set(LLVM_LINK_COMPONENTS
+  FrontendOpenMP
+  Support
+  )
+
+add_clang_library(clangTidyAutosarModule
+  AutosarTidyModule.cpp
+
+  LINK_LIBS
+  clangTidy
+  

[PATCH] D112783: [clangd] Track performance of IncludeCleaner

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112783

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


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure ,
   const llvm::DenseSet ) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion  : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet ,
  const IncludeStructure ,
  const SourceManager ) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -269,6 +272,7 @@
 }
 
 std::vector computeUnusedIncludes(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::computeUnusedIncludes");
   const auto  = AST.getSourceManager();
 
   auto Refs = findReferencedLocations(AST);


Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -197,6 +197,7 @@
 } // namespace
 
 ReferencedLocations findReferencedLocations(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure ,
   const llvm::DenseSet ) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion  : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet ,
  const IncludeStructure ,
  const SourceManager ) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -269,6 +272,7 @@
 }
 
 std::vector computeUnusedIncludes(ParsedAST ) {
+  trace::Span Tracer("IncludeCleaner::computeUnusedIncludes");
   const auto  = AST.getSourceManager();
 
   auto Refs = findReferencedLocations(AST);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112695: [clangd] IncludeCleaner: Skip non self-contained headers

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 383253.
kbobyrev added a comment.

Add a header guard in tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112695

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/unittests/DiagnosticsTests.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
@@ -19,6 +19,10 @@
 using ::testing::ElementsAre;
 using ::testing::UnorderedElementsAre;
 
+std::string guard(llvm::StringRef Code) {
+  return "#pragma once\n" + Code.str();
+}
+
 TEST(IncludeCleaner, ReferencedLocations) {
   struct TestCase {
 std::string HeaderCode;
@@ -206,6 +210,7 @@
 #include "b.h"
 #include "dir/c.h"
 #include "dir/unused.h"
+#include "unguarded.h"
 #include "unused.h"
 #include 
 void foo() {
@@ -216,13 +221,14 @@
   // Build expected ast with symbols coming from headers.
   TestTU TU;
   TU.Filename = "foo.cpp";
-  TU.AdditionalFiles["foo.h"] = "void foo();";
-  TU.AdditionalFiles["a.h"] = "void a();";
-  TU.AdditionalFiles["b.h"] = "void b();";
-  TU.AdditionalFiles["dir/c.h"] = "void c();";
-  TU.AdditionalFiles["unused.h"] = "void unused();";
-  TU.AdditionalFiles["dir/unused.h"] = "void dirUnused();";
-  TU.AdditionalFiles["system/system_header.h"] = "";
+  TU.AdditionalFiles["foo.h"] = guard("void foo();");
+  TU.AdditionalFiles["a.h"] = guard("void a();");
+  TU.AdditionalFiles["b.h"] = guard("void b();");
+  TU.AdditionalFiles["dir/c.h"] = guard("void c();");
+  TU.AdditionalFiles["unused.h"] = guard("void unused();");
+  TU.AdditionalFiles["dir/unused.h"] = guard("void dirUnused();");
+  TU.AdditionalFiles["system/system_header.h"] = guard("");
+  TU.AdditionalFiles["unguarded.h"] = "";
   TU.ExtraArgs.push_back("-I" + testPath("dir"));
   TU.ExtraArgs.push_back("-isystem" + testPath("system"));
   TU.Code = MainFile.str();
@@ -231,8 +237,7 @@
   for (const auto  : computeUnusedIncludes(AST))
 UnusedIncludes.push_back(Include->Written);
   EXPECT_THAT(UnusedIncludes,
-  UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\"",
-   ""));
+  UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\""));
 }
 
 TEST(IncludeCleaner, VirtualBuffers) {
@@ -252,6 +257,9 @@
   // ScratchBuffer with `flags::FLAGS_FOO` that will have FileID but not
   // FileEntry.
   TU.AdditionalFiles["macros.h"] = R"cpp(
+#ifndef MACROS_H
+#define MACROS_H
+
 #define DEFINE_FLAG(X) \
 namespace flags { \
 int FLAGS_##X; \
@@ -261,6 +269,8 @@
 
 #define ab x
 #define concat(x, y) x##y
+
+#endif // MACROS_H
 )cpp";
   TU.ExtraArgs = {"-DCLI=42"};
   ParsedAST AST = TU.build();
@@ -286,7 +296,7 @@
   EXPECT_THAT(ReferencedHeaderNames, ElementsAre(testPath("macros.h")));
 
   // Sanity check.
-  EXPECT_THAT(getUnused(Includes, ReferencedHeaders), ::testing::IsEmpty());
+  EXPECT_THAT(getUnused(AST, ReferencedHeaders), ::testing::IsEmpty());
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1483,7 +1483,8 @@
 TEST(DiagnosticsTest, IncludeCleaner) {
   Annotations Test(R"cpp(
 $fix[[  $diag[[#include "unused.h"]]
-]]#include "used.h"
+]]
+  #include "used.h"
 
   #include 
 
@@ -1494,9 +1495,11 @@
   TestTU TU;
   TU.Code = Test.code().str();
   TU.AdditionalFiles["unused.h"] = R"cpp(
+#pragma once
 void unused() {}
   )cpp";
   TU.AdditionalFiles["used.h"] = R"cpp(
+#pragma once
 void used() {}
   )cpp";
   TU.AdditionalFiles["system/system_header.h"] = "";
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -61,8 +61,9 @@
  const IncludeStructure , const SourceManager );
 
 /// Retrieves headers that are referenced from the main file but not used.
+/// System headers and inclusions of headers with no header guard are dropped.
 std::vector
-getUnused(const IncludeStructure ,
+getUnused(ParsedAST ,
   const llvm::DenseSet );
 
 std::vector computeUnusedIncludes(ParsedAST );
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -8,12 +8,15 @@
 
 #include "IncludeCleaner.h"
 #include 

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

> "C++14 Guidelines"? So it's always and definitely C++14 specific?

To my knowledge, AUTOSAR handed over the work to MISRA and the current 
direction is that MISRA will merge AUTOSAR C++14 and older MISRA revisions into 
a brand-new MISRA release, coming up 202x (The "x" is very unclear here, it 
could take years to get it in place). So I would be surprised if AUTOSAR 
released a new version after this. They way they are written they are 
targetting the use of C++14.

I suppose we can change the description in the future + add configuration to 
select whether to choose C++14 or something else if that ever comes up, but I 
think it's a bit premature at this point. Would you agree?

> What is the licencing approach of this guideline? Looking up with searchers 
> seems to turn up a few PDFs which say `--- AUTOSAR CONFIDENTIAL ---`, yet 
> they are up and out on the official-looking website.

Yes, that's strange. The disclaimer says the following:

> The material contained in this work is protected by copyright and other types 
> of
>  intellectual property rights. The commercial exploitation of the material 
> contained in
>  this work requires a license to such intellectual property rights.
>  This work may be utilized or reproduced without any modification, in any 
> form or by
>  any means, for informational purposes only. For any other purpose, no part 
> of the
>  work may be utilized or reproduced, in any form or by any means, without 
> permission
>  in writing from the publisher.

I'm not a lawyer for I think we can be sure that this is not a commercial use 
case? I guess if we were a company selling an AUTOSAR C++14-compliant static 
analyzer then we would need a license.


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

https://reviews.llvm.org/D112730

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

[PATCH] D112707: [clangd] IncludeCleaner: Be more conservative in marking RecordDecl usage

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D112707#3093876 , @sammccall wrote:

> What's the purpose of this patch at a high level? Was it triggered by a real 
> example?
> IIUC it's avoiding false negatives, where we're using a class and an 
> otherwise-unused header forward-declares that class.
> Avoiding false negatives isn't a high priority at this point, unless it's a 
> *really* common case.
> As Kadir says this is subtle and risks introducing false positives.
>
> My inclination is that we shouldn't spend time making to make these 
> heuristics more precise/complicated right now, but I'm willing to be 
> convinced...

LLVM has tons of widely used headers with lots and lots of fowrard-declared 
classes (mainly AST ones) which result in less-useful unused includes warnings. 
If I drop the change for the case when definition is not available (or fix by 
checking whether the last redecl is in the mainfile) then this seems like a 
clear improvement with no false-positives, WDYT?




Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:51
+  Result.insert(Definition ? Definition->getLocation()
+   : RD->getMostRecentDecl()->getLocation());
+  return true;

kadircet wrote:
> i think this might turn a direct dependency into a transitive one, e.g. you 
> got forward declarations of `struct Foo;` in a.h and b.h, then c.h includes 
> b.h. In the main file you might have includes for a.h and c.h. Now the most 
> recent redecl happens through c.h hence a.h will be marked as unused, even 
> though it's the one header providing the forward decl directly.
> 
> what about just rolling with the definition when it's visible and handling 
> the forward-decl in main file case inside the `add` ? i suppose that's 
> something we'd want for all decls and not just records? it implies passing in 
> main file id and a sourcemanager into the crawler, and inside the `add` 
> before going over all redecls, we just check if most recent decl falls into 
> the main file.
I was considering that part but I decided it's probably more complications for 
less benefits but I can see how the false positives might turn out to be a 
problem.

I think this is not something we want for all decls for type checking reasons 
(e.g. functions?). @sammccall and I talked about similar things and I believe 
this is the conclusion, isn't it?

Thank you, will do!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112707

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

[PATCH] D112695: [clangd] IncludeCleaner: Skip non self-contained headers

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 383250.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.

Address review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112695

Files:
  clang-tools-extra/clangd/IncludeCleaner.cpp
  clang-tools-extra/clangd/IncludeCleaner.h
  clang-tools-extra/clangd/unittests/DiagnosticsTests.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
@@ -19,6 +19,10 @@
 using ::testing::ElementsAre;
 using ::testing::UnorderedElementsAre;
 
+std::string guard(llvm::StringRef Code) {
+  return "#pragma once\n" + Code.str();
+}
+
 TEST(IncludeCleaner, ReferencedLocations) {
   struct TestCase {
 std::string HeaderCode;
@@ -206,6 +210,7 @@
 #include "b.h"
 #include "dir/c.h"
 #include "dir/unused.h"
+#include "unguarded.h"
 #include "unused.h"
 #include 
 void foo() {
@@ -216,13 +221,14 @@
   // Build expected ast with symbols coming from headers.
   TestTU TU;
   TU.Filename = "foo.cpp";
-  TU.AdditionalFiles["foo.h"] = "void foo();";
-  TU.AdditionalFiles["a.h"] = "void a();";
-  TU.AdditionalFiles["b.h"] = "void b();";
-  TU.AdditionalFiles["dir/c.h"] = "void c();";
-  TU.AdditionalFiles["unused.h"] = "void unused();";
-  TU.AdditionalFiles["dir/unused.h"] = "void dirUnused();";
-  TU.AdditionalFiles["system/system_header.h"] = "";
+  TU.AdditionalFiles["foo.h"] = guard("void foo();");
+  TU.AdditionalFiles["a.h"] = guard("void a();");
+  TU.AdditionalFiles["b.h"] = guard("void b();");
+  TU.AdditionalFiles["dir/c.h"] = guard("void c();");
+  TU.AdditionalFiles["unused.h"] = guard("void unused();");
+  TU.AdditionalFiles["dir/unused.h"] = guard("void dirUnused();");
+  TU.AdditionalFiles["system/system_header.h"] = guard("");
+  TU.AdditionalFiles["unguarded.h"] = "";
   TU.ExtraArgs.push_back("-I" + testPath("dir"));
   TU.ExtraArgs.push_back("-isystem" + testPath("system"));
   TU.Code = MainFile.str();
@@ -231,8 +237,7 @@
   for (const auto  : computeUnusedIncludes(AST))
 UnusedIncludes.push_back(Include->Written);
   EXPECT_THAT(UnusedIncludes,
-  UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\"",
-   ""));
+  UnorderedElementsAre("\"unused.h\"", "\"dir/unused.h\""));
 }
 
 TEST(IncludeCleaner, VirtualBuffers) {
@@ -286,7 +291,7 @@
   EXPECT_THAT(ReferencedHeaderNames, ElementsAre(testPath("macros.h")));
 
   // Sanity check.
-  EXPECT_THAT(getUnused(Includes, ReferencedHeaders), ::testing::IsEmpty());
+  EXPECT_THAT(getUnused(AST, ReferencedHeaders), ::testing::IsEmpty());
 }
 
 } // namespace
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1483,7 +1483,8 @@
 TEST(DiagnosticsTest, IncludeCleaner) {
   Annotations Test(R"cpp(
 $fix[[  $diag[[#include "unused.h"]]
-]]#include "used.h"
+]]
+  #include "used.h"
 
   #include 
 
@@ -1494,9 +1495,11 @@
   TestTU TU;
   TU.Code = Test.code().str();
   TU.AdditionalFiles["unused.h"] = R"cpp(
+#pragma once
 void unused() {}
   )cpp";
   TU.AdditionalFiles["used.h"] = R"cpp(
+#pragma once
 void used() {}
   )cpp";
   TU.AdditionalFiles["system/system_header.h"] = "";
Index: clang-tools-extra/clangd/IncludeCleaner.h
===
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -61,8 +61,9 @@
  const IncludeStructure , const SourceManager );
 
 /// Retrieves headers that are referenced from the main file but not used.
+/// System headers and inclusions of headers with no header guard are dropped.
 std::vector
-getUnused(const IncludeStructure ,
+getUnused(ParsedAST ,
   const llvm::DenseSet );
 
 std::vector computeUnusedIncludes(ParsedAST );
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -8,12 +8,15 @@
 
 #include "IncludeCleaner.h"
 #include "Config.h"
+#include "ParsedAST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
 #include "support/Logger.h"
 #include "support/Trace.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Lex/Preprocessor.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
@@ 

[PATCH] D112730: [clang-tidy] Add AUTOSAR module

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

"C++14 Guidelines"? So it's always and definitely C++14 specific? What is the 
licencing approach of this guideline? Looking up with searchers seems to turn 
up a few PDFs which say `--- AUTOSAR CONFIDENTIAL ---`, yet they are up and out 
on the official-looking website.




Comment at: clang-tools-extra/clang-tidy/autosar/CMakeLists.txt:26
+  clangTooling
+  clangStaticAnalyzerCheckers
+  )

I'm not sure this is needed here, for this module, as of now.


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

https://reviews.llvm.org/D112730

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

[PATCH] D112409: [clang-tidy] Add check 'cert-err33-c'.

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

balazske updated this revision to Diff 383245.
balazske added a comment.

Using a much smaller test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112409

Files:
  clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-unused-return-value.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-err33-c.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-err33-c.c

Index: clang-tools-extra/test/clang-tidy/checkers/cert-err33-c.c
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cert-err33-c.c
@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s cert-err33-c %t
+
+typedef __SIZE_TYPE__ size_t;
+void *aligned_alloc(size_t alignment, size_t size);
+void test_aligned_alloc() {
+  aligned_alloc(2, 10);
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
+  // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+}
+
+long strtol(const char *restrict nptr, char **restrict endptr, int base);
+void test_strtol() {
+  strtol("123", 0, 10);
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
+  // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+}
+
+typedef char wchar_t;
+int wscanf_s(const wchar_t *restrict format, ...);
+void test_wscanf_s() {
+  int Val;
+  wscanf_s("%i", );
+  // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used
+  // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning
+}
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
@@ -333,6 +333,7 @@
`cert-dcl03-c `_, `misc-static-assert `_, "Yes"
`cert-dcl16-c `_, `readability-uppercase-literal-suffix `_, "Yes"
`cert-dcl37-c `_, `bugprone-reserved-identifier `_, "Yes"
+   `cert-err33-c `_, `bugprone-unused-return-value `_,
`cert-dcl51-cpp `_, `bugprone-reserved-identifier `_, "Yes"
`cert-dcl54-cpp `_, `misc-new-delete-overloads `_,
`cert-dcl59-cpp `_, `google-build-namespaces `_,
Index: clang-tools-extra/docs/clang-tidy/checks/cert-err33-c.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/cert-err33-c.rst
@@ -0,0 +1,199 @@
+.. title:: clang-tidy - cert-err33-c
+
+cert-err33-c
+
+
+Warns on unused function return values. Many of the standard library functions
+return a value that indicates if the call was successful. Ignoring the returned
+value can cause unexpected behavior if an error has occured. The following
+functions are checked:
+
+* aligned_alloc()
+* asctime_s()
+* at_quick_exit()
+* atexit()
+* bsearch()
+* bsearch_s()
+* btowc()
+* c16rtomb()
+* c32rtomb()
+* calloc()
+* clock()
+* cnd_broadcast()
+* cnd_init()
+* cnd_signal()
+* cnd_timedwait()
+* cnd_wait()
+* ctime_s()
+* fclose()
+* fflush()
+* fgetc()
+* fgetpos()
+* fgets()
+* fgetwc()
+* fopen()
+* fopen_s()
+* fprintf()
+* fprintf_s()
+* fputc()
+* fputs()
+* fputwc()
+* fputws()
+* fread()
+* freopen()
+* freopen_s()
+* fscanf()
+* fscanf_s()
+* fseek()
+* fsetpos()
+* ftell()
+* fwprintf()
+* fwprintf_s()
+* fwrite()
+* fwscanf()
+* fwscanf_s()
+* getc()
+* getchar()
+* getenv()
+* getenv_s()
+* gets_s()
+* getwc()
+* getwchar()
+* gmtime()
+* gmtime_s()
+* localtime()
+* localtime_s()
+* malloc()
+* mbrtoc16()
+* mbrtoc32()
+* mbsrtowcs()
+* mbsrtowcs_s()
+* mbstowcs()
+* mbstowcs_s()
+* memchr()
+* mktime()
+* mtx_init()
+* mtx_lock()
+* mtx_timedlock()
+* mtx_trylock()
+* mtx_unlock()
+* printf_s()
+* putc()
+* putwc()
+* raise()
+* realloc()
+* remove()
+* rename()
+* setlocale()
+* setvbuf()
+* scanf()
+* scanf_s()
+* signal()
+* snprintf()
+* snprintf_s()
+* sprintf()
+* sprintf_s()
+* sscanf()
+* sscanf_s()
+* strchr()
+* strerror_s()
+* strftime()
+* strpbrk()
+* strrchr()
+* strstr()
+* strtod()
+* strtof()
+* strtoimax()
+* strtok()
+* strtok_s()
+* strtol()
+* strtold()
+* strtoll()
+* strtoumax()
+* strtoul()
+* strtoull()
+* strxfrm()
+* swprintf()
+* swprintf_s()
+* swscanf()
+* swscanf_s()
+* thrd_create()
+* thrd_detach()
+* thrd_join()
+* thrd_sleep()
+* time()
+* timespec_get()
+* tmpfile()
+* tmpfile_s()
+* tmpnam()
+* tmpnam_s()
+* tss_create()
+* tss_get()
+* tss_set()
+* ungetc()
+* ungetwc()
+* vfprintf()
+* vfprintf_s()
+* vfscanf()
+* vfscanf_s()
+* vfwprintf()
+* vfwprintf_s()
+* vfwscanf()
+* vfwscanf_s()
+* vprintf_s()
+* vscanf()
+* vscanf_s()
+* vsnprintf()
+* vsnprintf_s()
+* vsprintf()
+* vsprintf_s()
+* vsscanf()
+* vsscanf_s()
+* 

[clang] 95e6e1c - [clang] Partially revert d8cd7806310c51af912a647a6ca46de62ff13214.

[Brad Smith via cfe-commits]

Author: Brad Smith
Date: 2021-10-29T02:41:54-04:00
New Revision: 95e6e1cc923d6b88ebb4ac5e8f0887c2c77476cc

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

LOG: [clang] Partially revert d8cd7806310c51af912a647a6ca46de62ff13214.

The C11 atomics part was wrong.

Added: 


Modified: 
clang/lib/Basic/Targets/OSTargets.h
clang/test/Preprocessor/init.c

Removed: 




diff  --git a/clang/lib/Basic/Targets/OSTargets.h 
b/clang/lib/Basic/Targets/OSTargets.h
index f49e2200cd5b8..7fbe2cbc56538 100644
--- a/clang/lib/Basic/Targets/OSTargets.h
+++ b/clang/lib/Basic/Targets/OSTargets.h
@@ -464,10 +464,8 @@ class LLVM_LIBRARY_VISIBILITY OpenBSDTargetInfo : public 
OSTargetInfo {
 if (this->HasFloat128)
   Builder.defineMacro("__FLOAT128__");
 
-if (Opts.C11) {
-  Builder.defineMacro("__STDC_NO_ATOMICS__");
+if (Opts.C11)
   Builder.defineMacro("__STDC_NO_THREADS__");
-}
   }
 
 public:

diff  --git a/clang/test/Preprocessor/init.c b/clang/test/Preprocessor/init.c
index 6a3cb9b8ff6a9..8bbfe85b9ba74 100644
--- a/clang/test/Preprocessor/init.c
+++ b/clang/test/Preprocessor/init.c
@@ -1467,11 +1467,9 @@
 // RUN: %clang_cc1 -x c -std=c11 -E -dM -ffreestanding 
-triple=amd64-unknown-openbsd < /dev/null | FileCheck -match-full-lines 
-check-prefix OPENBSD-STDC %s
 // RUN: %clang_cc1 -x c -std=gnu11 -E -dM -ffreestanding 
-triple=amd64-unknown-openbsd < /dev/null | FileCheck -match-full-lines 
-check-prefix OPENBSD-STDC %s
 // RUN: %clang_cc1 -x c -std=c17 -E -dM -ffreestanding 
-triple=amd64-unknown-openbsd < /dev/null | FileCheck -match-full-lines 
-check-prefix OPENBSD-STDC %s
-// OPENBSD-STDC:#define __STDC_NO_ATOMICS__ 1
 // OPENBSD-STDC:#define __STDC_NO_THREADS__ 1
 //
 // RUN: %clang_cc1 -x c -std=c99 -E -dM -ffreestanding 
-triple=amd64-unknown-openbsd < /dev/null | FileCheck -match-full-lines 
-check-prefix OPENBSD-STDC-N %s
-// OPENBSD-STDC-N-NOT:#define __STDC_NO_ATOMICS__ 1
 // OPENBSD-STDC-N-NOT:#define __STDC_NO_THREADS__ 1
 //
 // RUN: %clang_cc1 -E -dM -ffreestanding -triple=xcore-none-none < /dev/null | 
FileCheck -match-full-lines -check-prefix XCORE %s



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

[PATCH] D112059: Fix inline builtin handling in case of redefinition

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 383241.
serge-sans-paille added a comment.

Re-uploading previous version that walks redef, with a slight change in the 
walking algorithm.


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

https://reviews.llvm.org/D112059

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/strlen-inline-builtin-redecl.c
  clang/test/CodeGen/user-func-gnu-inline-redecl.c

Index: clang/test/CodeGen/user-func-gnu-inline-redecl.c
===
--- /dev/null
+++ clang/test/CodeGen/user-func-gnu-inline-redecl.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -O1 -o - %s | FileCheck %s
+//
+// Verifies that the gnu_inline version is ignored in favor of the redecl
+
+extern inline __attribute__((gnu_inline)) unsigned long some_size(int c) {
+  return 1;
+}
+unsigned long mycall(int s) {
+  // CHECK-LABEL: i64 @mycall
+  // CHECK: ret i64 2
+  return some_size(s);
+}
+unsigned long some_size(int c) {
+  return 2;
+}
+unsigned long yourcall(int s) {
+  // CHECK-LABEL: i64 @yourcall
+  // CHECK: ret i64 2
+  return some_size(s);
+}
Index: clang/test/CodeGen/strlen-inline-builtin-redecl.c
===
--- /dev/null
+++ clang/test/CodeGen/strlen-inline-builtin-redecl.c
@@ -0,0 +1,21 @@
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -disable-llvm-passes -o - %s | FileCheck %s
+//
+// Verifies that clang-generated *.inline are removed when shadowed by an external definition
+
+// CHECK-NOT: strlen.inline
+
+unsigned long strnlen(const char *, unsigned long);
+void fortify_panic(const char *);
+
+extern inline __attribute__((always_inline)) __attribute__((gnu_inline)) unsigned long strlen(const char *p) {
+  return 1;
+}
+unsigned long mystrlen(char const *s) {
+  return strlen(s);
+}
+unsigned long strlen(const char *s) {
+  return 2;
+}
+unsigned long yourstrlen(char const *s) {
+  return strlen(s);
+}
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1299,18 +1299,39 @@
   // When generating code for a builtin with an inline declaration, use a
   // mangled name to hold the actual body, while keeping an external definition
   // in case the function pointer is referenced somewhere.
-  if (FD->isInlineBuiltinDeclaration() && Fn) {
-std::string FDInlineName = (Fn->getName() + ".inline").str();
-llvm::Module *M = Fn->getParent();
-llvm::Function *Clone = M->getFunction(FDInlineName);
-if (!Clone) {
-  Clone = llvm::Function::Create(Fn->getFunctionType(),
- llvm::GlobalValue::InternalLinkage,
- Fn->getAddressSpace(), FDInlineName, M);
-  Clone->addFnAttr(llvm::Attribute::AlwaysInline);
+  if (Fn) {
+if (FD->isInlineBuiltinDeclaration()) {
+  std::string FDInlineName = (Fn->getName() + ".inline").str();
+  llvm::Module *M = Fn->getParent();
+  llvm::Function *Clone = M->getFunction(FDInlineName);
+  if (!Clone) {
+Clone = llvm::Function::Create(Fn->getFunctionType(),
+   llvm::GlobalValue::InternalLinkage,
+   Fn->getAddressSpace(), FDInlineName, M);
+Clone->addFnAttr(llvm::Attribute::AlwaysInline);
+  }
+  Fn->setLinkage(llvm::GlobalValue::ExternalLinkage);
+  Fn = Clone;
+}
+
+// Detect the unusual situation where an inline version is shadowed by a
+// non-inline version. In that case we should pick the external one
+// everywhere. That's GCC behavior too. Unfortunately, I cannot find a way
+// to detect that situation before we reach codegen, so do some late
+// replacement.
+else {
+  for(const FunctionDecl* PD = FD->getPreviousDecl(); PD; PD = PD->getPreviousDecl()) {
+if(LLVM_UNLIKELY(PD->isInlineBuiltinDeclaration())) {
+  std::string FDInlineName = (Fn->getName() + ".inline").str();
+  llvm::Module *M = Fn->getParent();
+  if (llvm::Function *Clone = M->getFunction(FDInlineName)) {
+Clone->replaceAllUsesWith(Fn);
+Clone->eraseFromParent();
+  }
+  break;
+}
+  }
 }
-Fn->setLinkage(llvm::GlobalValue::ExternalLinkage);
-Fn = Clone;
   }
 
   // Check if we should generate debug info for this function.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112722: [clang-tidy]performance-unnecessary-copy-initialization: fix false negative

[Clement Courbet via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGfc1b24d7360f: 
[clang-tidy]performance-unnecessary-copy-initialization: fix false negative 
(authored by courbet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112722

Files:
  clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
  
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ 
clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -12,6 +12,8 @@
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType () const;
+  using ConstRef = const ExpensiveToCopyType &;
+  ConstRef referenceWithAlias() const;
   const ExpensiveToCopyType *pointer() const;
   Iterator begin() const;
   Iterator end() const;
@@ -206,6 +208,15 @@
   }
 }
 
+void positiveNonConstVarInCodeBlockWithAlias(const ExpensiveToCopyType ) {
+  {
+const ExpensiveToCopyType Assigned = Obj.referenceWithAlias();
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: the const qualified variable
+// CHECK-FIXES: const ExpensiveToCopyType& Assigned = 
Obj.referenceWithAlias();
+useAsConstReference(Assigned);
+  }
+}
+
 void negativeNonConstVarWithNonConstUse(const ExpensiveToCopyType ) {
   {
 auto NonConstInvoked = Obj.reference();
Index: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -84,7 +84,8 @@
   // returned either points to a global static variable or to a member of the
   // called object.
   return cxxMemberCallExpr(
-  callee(cxxMethodDecl(returns(matchers::isReferenceToConst()))
+  callee(cxxMethodDecl(
+ returns(hasCanonicalType(matchers::isReferenceToConst(
  .bind(MethodDeclId)),
   on(declRefExpr(to(
   varDecl(
@@ -97,7 +98,8 @@
   // Only allow initialization of a const reference from a free function if it
   // has no arguments. Otherwise it could return an alias to one of its
   // arguments and the arguments need to be checked for const use as well.
-  return callExpr(callee(functionDecl(returns(matchers::isReferenceToConst()))
+  return callExpr(callee(functionDecl(returns(hasCanonicalType(
+  matchers::isReferenceToConst(
  .bind(FunctionDeclId)),
   argumentCountIs(0), unless(callee(cxxMethodDecl(
   .bind(InitFunctionCallId);


Index: clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -12,6 +12,8 @@
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType () const;
+  using ConstRef = const ExpensiveToCopyType &;
+  ConstRef referenceWithAlias() const;
   const ExpensiveToCopyType *pointer() const;
   Iterator begin() const;
   Iterator end() const;
@@ -206,6 +208,15 @@
   }
 }
 
+void positiveNonConstVarInCodeBlockWithAlias(const ExpensiveToCopyType ) {
+  {
+const ExpensiveToCopyType Assigned = Obj.referenceWithAlias();
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: the const qualified variable
+// CHECK-FIXES: const ExpensiveToCopyType& Assigned = Obj.referenceWithAlias();
+useAsConstReference(Assigned);
+  }
+}
+
 void negativeNonConstVarWithNonConstUse(const ExpensiveToCopyType ) {
   {
 auto NonConstInvoked = Obj.reference();
Index: clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
===
--- clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -84,7 +84,8 @@
   // returned either points to a global static variable or to a member of the
   // called object.
   return cxxMemberCallExpr(
-  callee(cxxMethodDecl(returns(matchers::isReferenceToConst()))
+  callee(cxxMethodDecl(
+ returns(hasCanonicalType(matchers::isReferenceToConst(
  .bind(MethodDeclId)),
   on(declRefExpr(to(
   varDecl(
@@ -97,7 +98,8 @@
   // Only allow initialization of a 

[clang-tools-extra] fc1b24d - [clang-tidy]performance-unnecessary-copy-initialization: fix false negative

[Clement Courbet via cfe-commits]

Author: Clement Courbet
Date: 2021-10-29T08:41:03+02:00
New Revision: fc1b24d7360f59c8b89f1b6290fe849a6207dc44

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

LOG: [clang-tidy]performance-unnecessary-copy-initialization: fix false negative

We're missing all cases where the return value is a type alias.

Unfortunately, this includes things we care about, such as
`std::vector::operator[]` (return value is `const_reference`,
not `const T&`).

Match the canonical type instead.

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp

clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp

Removed: 




diff  --git 
a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp 
b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
index 6f9495fd1e66c..2cdd7827ee42a 100644
--- a/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
+++ b/clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
@@ -84,7 +84,8 @@ AST_MATCHER_FUNCTION_P(StatementMatcher, 
isConstRefReturningMethodCall,
   // returned either points to a global static variable or to a member of the
   // called object.
   return cxxMemberCallExpr(
-  callee(cxxMethodDecl(returns(matchers::isReferenceToConst()))
+  callee(cxxMethodDecl(
+ returns(hasCanonicalType(matchers::isReferenceToConst(
  .bind(MethodDeclId)),
   on(declRefExpr(to(
   varDecl(
@@ -97,7 +98,8 @@ AST_MATCHER_FUNCTION(StatementMatcher, 
isConstRefReturningFunctionCall) {
   // Only allow initialization of a const reference from a free function if it
   // has no arguments. Otherwise it could return an alias to one of its
   // arguments and the arguments need to be checked for const use as well.
-  return callExpr(callee(functionDecl(returns(matchers::isReferenceToConst()))
+  return callExpr(callee(functionDecl(returns(hasCanonicalType(
+  matchers::isReferenceToConst(
  .bind(FunctionDeclId)),
   argumentCountIs(0), unless(callee(cxxMethodDecl(
   .bind(InitFunctionCallId);

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
 
b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
index 68a010c4d953c..4c469c966860b 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/performance-unnecessary-copy-initialization.cpp
@@ -12,6 +12,8 @@ struct ExpensiveToCopyType {
   ExpensiveToCopyType();
   virtual ~ExpensiveToCopyType();
   const ExpensiveToCopyType () const;
+  using ConstRef = const ExpensiveToCopyType &;
+  ConstRef referenceWithAlias() const;
   const ExpensiveToCopyType *pointer() const;
   Iterator begin() const;
   Iterator end() const;
@@ -206,6 +208,15 @@ void positiveNonConstVarInCodeBlock(const 
ExpensiveToCopyType ) {
   }
 }
 
+void positiveNonConstVarInCodeBlockWithAlias(const ExpensiveToCopyType ) {
+  {
+const ExpensiveToCopyType Assigned = Obj.referenceWithAlias();
+// CHECK-MESSAGES: [[@LINE-1]]:31: warning: the const qualified variable
+// CHECK-FIXES: const ExpensiveToCopyType& Assigned = 
Obj.referenceWithAlias();
+useAsConstReference(Assigned);
+  }
+}
+
 void negativeNonConstVarWithNonConstUse(const ExpensiveToCopyType ) {
   {
 auto NonConstInvoked = Obj.reference();



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

[PATCH] D111952: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd758069f5e0d: [clang] [MinGW] Guess the right ix86 arch name 
spelling as sysroot (authored by mstorsjo).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111952

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/MinGW.cpp
  clang/lib/Driver/ToolChains/MinGW.h
  clang/test/Driver/mingw-sysroot.cpp

Index: clang/test/Driver/mingw-sysroot.cpp
===
--- clang/test/Driver/mingw-sysroot.cpp
+++ clang/test/Driver/mingw-sysroot.cpp
@@ -12,6 +12,7 @@
 // RUN: mkdir -p %T/testroot-clang/bin
 // RUN: ln -s %clang %T/testroot-clang/bin/x86_64-w64-mingw32-clang
 // RUN: ln -s %S/Inputs/mingw_ubuntu_posix_tree/usr/x86_64-w64-mingw32 %T/testroot-clang/x86_64-w64-mingw32
+// RUN: ln -s %S/Inputs/mingw_arch_tree/usr/i686-w64-mingw32 %T/testroot-clang/i686-w64-mingw32
 
 
 // If we find a gcc in the path with the right triplet prefix, pick that as
@@ -36,3 +37,14 @@
 // the libgcc directory:
 
 // RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-gcc/bin/x86_64-w64-mingw32-clang -target x86_64-w64-mingw32 -rtlib=platform -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_GCC %s
+
+
+// If the user requests a different arch via the -m32 option, which changes
+// x86_64 into i386, check that the driver notices that it can't find a
+// sysroot for i386 but there is one for i686, and uses that one.
+// (In practice, the real usecase is when using an unprefixed native clang
+// that defaults to x86_64 mingw, but it's easier to test this in cross setups
+// with symlinks, like the other tests here.)
+
+// RUN: env "PATH=%T/testroot-gcc/bin:%PATH%" %T/testroot-clang/bin/x86_64-w64-mingw32-clang --target=x86_64-w64-mingw32 -m32 -rtlib=compiler-rt -stdlib=libstdc++ --sysroot="" -c -### %s 2>&1 | FileCheck -check-prefix=CHECK_TESTROOT_CLANG_I686 %s
+// CHECK_TESTROOT_CLANG_I686: "{{[^"]+}}/testroot-clang{{/|}}i686-w64-mingw32{{/|}}include"
Index: clang/lib/Driver/ToolChains/MinGW.h
===
--- clang/lib/Driver/ToolChains/MinGW.h
+++ clang/lib/Driver/ToolChains/MinGW.h
@@ -60,6 +60,9 @@
   MinGW(const Driver , const llvm::Triple ,
 const llvm::opt::ArgList );
 
+  static void fixTripleArch(const Driver , llvm::Triple ,
+const llvm::opt::ArgList );
+
   bool HasNativeLLVMSupport() const override;
 
   bool IsIntegratedAssemblerDefault() const override;
@@ -103,8 +106,6 @@
   mutable std::unique_ptr Preprocessor;
   mutable std::unique_ptr Compiler;
   void findGccLibDir();
-  llvm::ErrorOr findGcc();
-  llvm::ErrorOr findClangRelativeSysroot();
 
   bool NativeLLVMSupport;
 };
Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SubdirName))
 Base = 

[clang] d758069 - [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

[Martin Storsjö via cfe-commits]

Author: Martin Storsjö
Date: 2021-10-29T09:32:36+03:00
New Revision: d758069f5e0d483f85ed1ee2c84b3955238eedce

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

LOG: [clang] [MinGW] Guess the right ix86 arch name spelling as sysroot

For x86, most contempory mingw toolchains use i686 as 32 bit
x86 arch target.

As long as the target triple is set to the right form, this works
fine, either as the compiler's default target, or via e.g.
a triple prefix like i686-w64-mingw32-clang.

However, if the unprefixed toolchain targets x86_64, but the user
tries to switch it to target 32 bit by adding the -m32 option, the
computeTargetTriple function in Clang, together with
Triple::get32BitArchVariant, sets the arch to i386. This causes
the right sysroot to not be found.

When targeting an arch where there are potential spelling ambiguities
with respect to the sysroots (i386 and arm), check if the driver can
find a sysroot with the arch name - if not, try a couple other
candidates.

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChains/MinGW.cpp
clang/lib/Driver/ToolChains/MinGW.h
clang/test/Driver/mingw-sysroot.cpp

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 2e69e6c50f4af..cd13a6d8b8303 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -525,8 +525,11 @@ static llvm::Triple computeTargetTriple(const Driver ,
   Target.setEnvironment(llvm::Triple::CODE16);
 }
 
-if (AT != llvm::Triple::UnknownArch && AT != Target.getArch())
+if (AT != llvm::Triple::UnknownArch && AT != Target.getArch()) {
   Target.setArch(AT);
+  if (Target.isWindowsGNUEnvironment())
+toolchains::MinGW::fixTripleArch(D, Target, Args);
+}
   }
 
   // Handle -miamcu flag.

diff  --git a/clang/lib/Driver/ToolChains/MinGW.cpp 
b/clang/lib/Driver/ToolChains/MinGW.cpp
index 0bdeff38f52ec..b2a5b3782957d 100644
--- a/clang/lib/Driver/ToolChains/MinGW.cpp
+++ b/clang/lib/Driver/ToolChains/MinGW.cpp
@@ -369,9 +369,9 @@ void toolchains::MinGW::findGccLibDir() {
   }
 }
 
-llvm::ErrorOr toolchains::MinGW::findGcc() {
+static llvm::ErrorOr findGcc(const llvm::Triple ) {
   llvm::SmallVector, 2> Gccs;
-  Gccs.emplace_back(getTriple().getArchName());
+  Gccs.emplace_back(T.getArchName());
   Gccs[0] += "-w64-mingw32-gcc";
   Gccs.emplace_back("mingw32-gcc");
   // Please do not add "gcc" here
@@ -381,13 +381,14 @@ llvm::ErrorOr toolchains::MinGW::findGcc() {
   return make_error_code(std::errc::no_such_file_or_directory);
 }
 
-llvm::ErrorOr toolchains::MinGW::findClangRelativeSysroot() {
+static llvm::ErrorOr
+findClangRelativeSysroot(const Driver , const llvm::Triple ,
+ std::string ) {
   llvm::SmallVector, 2> Subdirs;
-  Subdirs.emplace_back(getTriple().str());
-  Subdirs.emplace_back(getTriple().getArchName());
+  Subdirs.emplace_back(T.str());
+  Subdirs.emplace_back(T.getArchName());
   Subdirs[1] += "-w64-mingw32";
-  StringRef ClangRoot =
-  llvm::sys::path::parent_path(getDriver().getInstalledDir());
+  StringRef ClangRoot = llvm::sys::path::parent_path(D.getInstalledDir());
   StringRef Sep = llvm::sys::path::get_separator();
   for (StringRef CandidateSubdir : Subdirs) {
 if (llvm::sys::fs::is_directory(ClangRoot + Sep + CandidateSubdir)) {
@@ -404,13 +405,16 @@ toolchains::MinGW::MinGW(const Driver , const 
llvm::Triple ,
   RocmInstallation(D, Triple, Args) {
   getProgramPaths().push_back(getDriver().getInstalledDir());
 
+  // The sequence for detecting a sysroot here should be kept in sync with
+  // the testTriple function below.
   if (getDriver().SysRoot.size())
 Base = getDriver().SysRoot;
   // Look for /../; if found, use /.. as the
   // base as it could still be a base for a gcc setup with libgcc.
-  else if (llvm::ErrorOr TargetSubdir = 
findClangRelativeSysroot())
+  else if (llvm::ErrorOr TargetSubdir =
+   findClangRelativeSysroot(getDriver(), getTriple(), SubdirName))
 Base = std::string(llvm::sys::path::parent_path(TargetSubdir.get()));
-  else if (llvm::ErrorOr GPPName = findGcc())
+  else if (llvm::ErrorOr GPPName = findGcc(getTriple()))
 Base = std::string(llvm::sys::path::parent_path(
 llvm::sys::path::parent_path(GPPName.get(;
   else
@@ -625,3 +629,55 @@ void toolchains::MinGW::AddClangCXXStdlibIncludeArgs(
 break;
   }
 }
+
+static bool testTriple(const Driver , const llvm::Triple ,
+   const ArgList ) {
+  // If an explicit sysroot is set, that will be used and we shouldn't try to
+  // detect anything else.
+  std::string SubdirName;
+  if (D.SysRoot.size())
+return true;
+  if (llvm::ErrorOr 

[PATCH] D111199: [Clang][LLVM][Attr] support btf_type_tag attribute

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

> Ah, yeah, I see what you mean - that does seem sort of unfortunate. Is it 
> possible these attributes could only appear on typedefs and they'd be more 
> readily carried through that without needing extra typeloc tracking? (sorry 
> for not having read back through the rest of the review - which may've 
> started there and ended up here as a more general form of the attribute?)

For the question, "is it possible these attributes could only appear on 
typedefs?" The answer is "not really possible". We are targeting existing linux 
kernel where existing type attributes (__user, __rcu, ...) have been used in 
places other than typedef quite extensively (e.g., function argument type, 
function return type, field type, etc.).

In one of my earlier prototypes, I put the tag string itself in AttributedType 
and with this we can avoid TypeLoc, but I understand this is not conventional 
usage and that is why we do through TypeLoc mechanism. @aaron.ballman can 
further comment on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99

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