[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", &Val);
+  // 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()
+

[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] 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 &Include : 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 &Includes, const SourceManager &SM);
 
 /// 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 &Includes,
+getUnused(ParsedAST &AST,
   const llvm::DenseSet &ReferencedFiles);
 
 std::vector computeUnusedIncludes(ParsedAST &AST);
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/

[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] 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] 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 &Include : 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 &Includes, const SourceManager &SM);
 
 /// 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 &Includes,
+getUnused(ParsedAST &AST,
   const llvm::DenseSet &ReferencedFiles);
 
 std::vector computeUnusedIncludes(ParsedAST &AST);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -8,12 +8,15

[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 &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -269,6 +272,7 @@
 }
 
 std::vector computeUnusedIncludes(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::computeUnusedIncludes");
   const auto &SM = 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 &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -269,6 +272,7 @@
 }
 
 std::vector computeUnusedIncludes(ParsedAST &AST) {
+  trace::Span Tracer("IncludeCleaner::computeUnusedIncludes");
   const auto &SM = 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] 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
+  clangTidyUt

[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] 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] 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] 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] 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 &AST) {
+  trace::Span Tracer("IncludeCleaner::computeUnusedIncludes");
   const auto &SM = 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 &AST,
  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] 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 @@
   Ne

[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 &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config &Cfg = 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 &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config &Cfg = 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] 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] 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

[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 
&Importer) {
 }
 
 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] 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(),
-

[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] 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] 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] 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] 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

[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 &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@ findReferencedFiles(const llvm::DenseSet 
&Locs,
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@ llvm::DenseSet
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@ std::vector 
computeUnusedIncludes(ParsedAST &AST) {
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config &Cfg = 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] 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 &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config &Cfg = 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 &AST) {
+  trace::Span Tracer("IncludeCleaner::findReferencedLocations");
   ReferencedLocations Result;
   ReferencedLocationCrawler Crawler(Result);
   Crawler.TraverseAST(AST.getASTContext());
@@ -225,6 +226,7 @@
 std::vector
 getUnused(const IncludeStructure &Includes,
   const llvm::DenseSet &ReferencedFiles) {
+  trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
   for (const Inclusion &MFI : Includes.MainFileIncludes) {
 // FIXME: Skip includes that are not self-contained.
@@ -253,6 +255,7 @@
 translateToHeaderIDs(const llvm::DenseSet &Files,
  const IncludeStructure &Includes,
  const SourceManager &SM) {
+  trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
   llvm::DenseSet TranslatedHeaderIDs;
   TranslatedHeaderIDs.reserve(Files.size());
   for (FileID FID : Files) {
@@ -279,6 +282,7 @@
 
 std::vector issueUnusedIncludesDiagnostics(ParsedAST &AST,
  llvm::StringRef Code) {
+  trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
   const Config &Cfg = 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] 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] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.

Nice work! Thanks!


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

https://reviews.llvm.org/D107339

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

[PATCH] D112765: [AST] injected-class-name is not a redecl, even in template specializations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1841-1842
+   /*DelayTypeCreation=*/IsInjectedClassName);
+  if (IsInjectedClassName)
+SemaRef.Context.getTypeDeclType(Record, cast(Owner));
 

Why is this call needed? (It seems strange to me that we call it and ignore the 
return value.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112765

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

[PATCH] D112421: [clang][ARM] PACBTI-M frontend support

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Basic/Targets/AArch64.cpp:134-135
  StringRef &Err) const {
-  llvm::AArch64::ParsedBranchProtection PBP;
-  if (!llvm::AArch64::parseBranchProtection(Spec, PBP, Err))
+  llvm::ARM::ParsedBranchProtection PBP;
+  if (!llvm::ARM::parseBranchProtection(Spec, PBP, Err))
 return false;

chill wrote:
> vhscampos wrote:
> > aaron.ballman wrote:
> > > This change surprises me. Why should AArch64TargetInfo prefer calling 
> > > into ARM instead?
> > Since that particular function ended up identical in both ARM and AArch64, 
> > we removed the AArch64 specific function and kept only one under ARM. You 
> > can spot the removal further down the patch.
> > 
> > The ARM namespace under ARMTargetParser.h already had code used in 
> > AArch64TargetParser, so we did not introduce new cross dependencies.
> It's the unfortunate overload of "ARM" used to denote the backend and the 
> organisation.
Ah, that's good to know, thank you for the explanation. (And yeah, that is an 
unfortunate overload of the term.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112421

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

[PATCH] D112491: Add `LambdaCapture`-related matchers.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: sammccall.
aaron.ballman added a subscriber: sammccall.
aaron.ballman added a comment.

In general, I'm happy with this. Adding @sammccall in case I've missed anything 
regarding the AST matcher internals or design concerns.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4629-4630
+///   matches `[x](){}`.
+AST_MATCHER_P(LambdaCapture, refersToVarDecl, internal::Matcher,
+  InnerMatcher) {
+  auto *capturedVar = Node.getCapturedVar();

jcking1034 wrote:
> aaron.ballman wrote:
> > jcking1034 wrote:
> > > ymandel wrote:
> > > > aaron.ballman wrote:
> > > > > The name here is a bit unclear -- whether it is the matcher matching 
> > > > > `int x;` or the `x` from the capture is not clear from the name. The 
> > > > > comment suggests it's matching `x` from the capture, but I think it's 
> > > > > actually matching the `int x;` variable declaration.
> > > > > 
> > > > > Being clear on what's matched here is important when we think about 
> > > > > initializers:
> > > > > ```
> > > > > void foo() {
> > > > >   int x = 12;
> > > > >   auto f = [x = 100](){};
> > > > > }
> > > > > ```
> > > > > and
> > > > > ```
> > > > > lambdaExpr(hasAnyCapture(lambdaCapture(refersToVarDecl(hasName("x"), 
> > > > > hasInitializer(integerLiteral(equals(100))
> > > > > ```
> > > > > Would you expect this to match? (This might be a good test case to 
> > > > > add.)
> > > > In a similar vein, do we want a separate matcher on the name of the 
> > > > capture itself? e.g. an overload of `hasName`? And what about matchers 
> > > > for the initializers?  Those don't have to land in this patch, but do 
> > > > you think those would be doable?
> > > I would expect @aaron.ballman's initializer example to match, and I added 
> > > a similar test case to the one  described. I think that if a capture does 
> > > not have an initializer, then `refersToVarDecl` will match on the 
> > > variable declaration before the lambda. However, if a capture does have 
> > > an initializer, that initializer itself seems to be represented as a 
> > > `VarDecl` in the AST, which is the `VarDecl` that gets matched.
> > > 
> > > For that reason, I think we may not need to have a separate matcher on 
> > > the name of the capture itself. Additionally, since captures with/without 
> > > initializers are both represented the same way, there may not be a good 
> > > way to distinguish between them, so matchers for initializers may not be 
> > > possible.
> > > I think that if a capture does not have an initializer, then 
> > > refersToVarDecl will match on the variable declaration before the lambda. 
> > > However, if a capture does have an initializer, that initializer itself 
> > > seems to be represented as a VarDecl in the AST, which is the VarDecl 
> > > that gets matched.
> > 
> > Oof, that'd be confusing! :-(
> > 
> > > For that reason, I think we may not need to have a separate matcher on 
> > > the name of the capture itself.
> > 
> > Er, but there are init captures where you can introduce a whole new 
> > declaration. I think we do want to be able to match on that, right? e.g.,
> > ```
> > [x = 12](){ return x; }();
> > ```
> > 
> > > Additionally, since captures with/without initializers are both 
> > > represented the same way, there may not be a good way to distinguish 
> > > between them, so matchers for initializers may not be possible.
> > 
> > That's a bummer! :-( If this turns out to be a limitation, we should 
> > probably document it as such.
> For the example you've provided, these can be matched with the 
> `refersToVarDecl` matcher, as seen in the test 
> `LambdaCaptureTest_BindsToCaptureWithInitializer`. I've gone ahead and 
> updated the documentation to include an example with an initializer.
> 
> Having that limitation with initializer representation is definitely a 
> concern, though. Looking through the [[ 
> https://clang.llvm.org/doxygen/LambdaCapture_8h_source.html | source ]] for 
> the `LambdaCapture` class, the documentation for the `DeclAndBits` (line 
> 42-48) suggests that there isn't a distinguishment between the two cases. 
> However, do you think it's feasible to update the classes related to 
> `LambdaCapture` obtain and store this information (possibly through updating 
> the `LambdaCaptureKind` enum, updating the constructor/fields of the class, 
> etc)?
> However, do you think it's feasible to update the classes related to 
> LambdaCapture obtain and store this information (possibly through updating 
> the LambdaCaptureKind enum, updating the constructor/fields of the class, 
> etc)?

I think that would make sense (thought perhaps as an orthogonal patch). That we 
don't track init captures seems like a deficiency of the AST to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

___
cfe-comm

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16155
+  if (getLangOpts().CPlusPlusModules) {
+auto *GlobalModule =
+PushGlobalModuleFragment(ExternLoc, /*IsImplicit=*/true);

`Module *` instead of `auto *` (the type is not spelled out in the 
initialization).



Comment at: clang/lib/Sema/SemaModule.cpp:71
   // module-private (though they do not have module linkage).
-  auto &Map = PP.getHeaderSearchInfo().getModuleMap();
-  auto *GlobalModule = Map.createGlobalModuleFragmentForModuleUnit(ModuleLoc);
-  assert(GlobalModule && "module creation should not fail");
-
-  // Enter the scope of the global module.
-  ModuleScopes.push_back({});
-  ModuleScopes.back().BeginLoc = ModuleLoc;
-  ModuleScopes.back().Module = GlobalModule;
-  VisibleModules.setVisible(GlobalModule, ModuleLoc);
+  auto *GlobalModule =
+  PushGlobalModuleFragment(ModuleLoc, /*IsImplicit=*/false);

`Module *`



Comment at: clang/lib/Sema/SemaModule.cpp:707-708
+   bool IsImplicit) {
+  auto &Map = PP.getHeaderSearchInfo().getModuleMap();
+  auto *GlobalModule = Map.createGlobalModuleFragmentForModuleUnit(BeginLoc);
+  assert(GlobalModule && "module creation should not fail");

Please spell these out as well.



Comment at: clang/lib/Sema/SemaModule.cpp:712-715
+  ModuleScopes.push_back({});
+  ModuleScopes.back().BeginLoc = BeginLoc;
+  ModuleScopes.back().Module = GlobalModule;
+  ModuleScopes.back().ImplicitGlobalModuleFragment = IsImplicit;

Can we use `emplace_back()` and construct in place rather than constructing 
piecemeal?



Comment at: clang/lib/Sema/SemaModule.cpp:722-723
+void Sema::PopGlobalModuleFragment() {
+  assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
+ "left the wrong module scope, which is not global module fragment");
+  ModuleScopes.pop_back();

FWIW, this seems to be failing to compile according to the precommit CI.
```
FAILED: tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o
CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ 
-DBUILD_EXAMPLES -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG 
-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS -Itools/clang/lib/Sema 
-I/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema 
-I/var/lib/buildkite-agent/builds/llvm-project/clang/include 
-Itools/clang/include -Iinclude 
-I/var/lib/buildkite-agent/builds/llvm-project/llvm/include -gmlt -fPIC 
-fvisibility-inlines-hidden -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
-DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT 
tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -MF 
tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o.d -o 
tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -c 
/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp
/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp:722:53:
 error: member reference type 'clang::Module *' is a pointer; did you mean to 
use '->'?
  assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
  ~~^
->
/usr/include/assert.h:93:27: note: expanded from macro 'assert'
 (static_cast  (expr) \
  ^~~~
1 error generated.
```



Comment at: clang/test/CXX/module/module.linkage_specification/p1.cpp:1
+// RUN: %clang_cc1 -std=c++2a %s -verify
+// expected-no-diagnostics

Might as well switch all these to use `-std=c++20`.


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

https://reviews.llvm.org/D110215

___
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

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D51650#3095997 , @danielkiss wrote:

> Hi @erichkeane,
>
> At Arm we are going to add the multiversioning support for Arm targets[1][2]. 
> 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
> [2] https://bugs.llvm.org/show_bug.cgi?id=50400

Hi Daniel-
This patch initially required that I refactor some stuff with the existing two 
multiversioning mechanisms (`cpu-specific` and `target`) which happened.  At a 
point after that, @lebedev.ri asked for me to rebase it, and I did.  However, 
it never gained attention from reviewers, and life got in the way :)

This needs rebasing/probably some cleanup, plus code review.  If you have 
active/proficient CFE code reviewers who could take a look, I'd be willing to 
spend some time rebasing.


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] D112765: [AST] injected-class-name is not a redecl, even in template specializations

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

I think we also want to update the ast dumper bit: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ASTDumper.cpp#L94-L100




Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1841-1842
+   /*DelayTypeCreation=*/IsInjectedClassName);
+  if (IsInjectedClassName)
+SemaRef.Context.getTypeDeclType(Record, cast(Owner));
 

aaron.ballman wrote:
> Why is this call needed? (It seems strange to me that we call it and ignore 
> the return value.)
my understanding is
  - `getTypeDeclType` is more than a getter, it also has a side-effort -- if 
the type of the passed `Record` is empty, it creates a type, and propagates the 
type to `Record->TypeForDecl`;
  - from the above line, we delay the type creation when `IsInjectedClassName` 
is true;
  - so we need to create a type for the `Record` by invoking `getTypeDeclType`;

might be worth a comment.




Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1910
 
+  if (IsInjectedClassName)
+assert(Record->isInjectedClassName() && "Broken injected-class-name");

it is unclear to me what's the intention of the assertion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112765

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

[PATCH] D112765: [AST] injected-class-name is not a redecl, even in template specializations

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

looks like 
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/RecursiveASTVisitor.h#L1684-L1686
 can be simplified as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112765

___
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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: llvm/include/llvm/IR/Constants.h:1317
   /// would make it harder to remove ConstantExprs altogether.
-  Instruction *getAsInstruction() const;
+  Instruction *getAsInstruction(Instruction *InsertBefore = nullptr) const;
 

Can you add a comment about the insertion location when 'InsertBefore' is 
nullptr? Thanks.


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] D112791: [IR] Merge createReplacementInstr into ConstantExpr::getAsInstruction

[Jay Foad via Phabricator via cfe-commits]

foad updated this revision to Diff 383341.
foad added a comment.

Add comment about InsertBefore.


Repository:
  rG LLVM Github Monorepo

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

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 @@
   NewOps, this, From, To, NumUpdated, OperandNo);
 }
 
-Instruction *ConstantExpr::getAsInstruction() const {
+Instruction *ConstantExpr::getAsInstruction(Instruction *InsertBefore) const {
   SmallVector ValueOperands(operands());
   ArrayRef Ops(ValueOperands);
 
@@ -3510,40 +3510,43 @@
   case Instruction::IntToPtr:
   case Instruction::BitCast:
   case Instruction::

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl accepted this revision.
yaxunl added a comment.

LGTM. Thanks!


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] D112791: [IR] Merge createReplacementInstr into ConstantExpr::getAsInstruction

[Jay Foad via Phabricator via cfe-commits]

foad added inline comments.



Comment at: llvm/include/llvm/IR/Constants.h:1317
   /// would make it harder to remove ConstantExprs altogether.
-  Instruction *getAsInstruction() const;
+  Instruction *getAsInstruction(Instruction *InsertBefore = nullptr) const;
 

yaxunl wrote:
> Can you add a comment about the insertion location when 'InsertBefore' is 
> nullptr? Thanks.
Done, although there are loads of InsertBefore arguments in Instructions.h that 
all work the same way, and no comments explaining them :)


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

[clang] 1b75892 - [IR] Merge createReplacementInstr into ConstantExpr::getAsInstruction

[Jay Foad via cfe-commits]

Author: Jay Foad
Date: 2021-10-29T15:02:58+01:00
New Revision: 1b758925adf6d78c89c70d2673689695e90fa993

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

LOG: [IR] Merge createReplacementInstr into ConstantExpr::getAsInstruction

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.

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

Added: 


Modified: 
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

Removed: 




diff  --git a/clang/lib/CodeGen/CGCUDANV.cpp b/clang/lib/CodeGen/CGCUDANV.cpp
index b0b5a60c35212..69499672bd861 100644
--- a/clang/lib/CodeGen/CGCUDANV.cpp
+++ b/clang/lib/CodeGen/CGCUDANV.cpp
@@ -472,7 +472,7 @@ static void replaceManagedVar(llvm::GlobalVariable *Var,
   // variable with instructions.
   for (auto &&Op : WorkItem) {
 auto *CE = cast(Op);
-auto *NewInst = llvm::createReplacementInstr(CE, I);
+auto *NewInst = CE->getAsInstruction(I);
 NewInst->replaceUsesOfWith(OldV, NewV);
 OldV = CE;
 NewV = NewInst;

diff  --git a/llvm/include/llvm/IR/Constants.h 
b/llvm/include/llvm/IR/Constants.h
index 92fcd74949b8a..71414d95d9a37 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -1308,13 +1308,14 @@ class ConstantExpr : public Constant {
 Type *SrcTy = nullptr) const;
 
   /// Returns an Instruction which implements the same operation as this
-  /// ConstantExpr. The instruction is not linked to any basic block.
+  /// ConstantExpr. If \p InsertBefore is not null, the new instruction is
+  /// inserted before it, otherwise it is not inserted into any basic block.
   ///
   /// A better approach to this could be to have a constructor for Instruction
   /// which would take a ConstantExpr parameter, but that would have spread
   /// implementation details of ConstantExpr outside of Constants.cpp, which
   /// would make it harder to remove ConstantExprs altogether.
-  Instruction *getAsInstruction() const;
+  Instruction *getAsInstruction(Instruction *InsertBefore = nullptr) const;
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Value *V) {

diff  --git a/llvm/lib/IR/Constants.cpp b/llvm/lib/IR/Constants.cpp
index 0b4334442d4fa..c66cfb6e9ac12 100644
--- a/llvm/lib/IR/Constants.cpp
+++ b/llvm/lib/IR/Constants.cpp
@@ -3492,7 +3492,7 @@ Value *ConstantExpr::handleOperandChangeImpl(Value *From, 
Value *ToV) {
   NewOps, this, From, To, NumUpdated, OperandNo);
 }
 
-Instruction *ConstantExpr::getAsInstruction() const {
+Instruction *ConstantExpr::getAsInstruction(Instruction *InsertBefore) const {
   SmallVector ValueOperands(operands());
   ArrayRef Ops(ValueOperands);
 
@@ -3510,40 +3510,43 @@ Instruction *ConstantExpr::getAsInstruction() const {
   case Instruction::IntToPtr:
   case Instruction::BitCast:
   case Instruction::AddrSpaceCast:
-return CastInst::Create((Instruction::CastOps)getOpcode(),
-Ops[0], getType());
+return CastInst::Create((Instruction::CastOps)getOpcode(), Ops[0],
+getType(), "", InsertBefore);
   case Instruction::Select:
-return SelectInst::Create(Ops[0], Ops[1], Ops[2]);
+return SelectInst::Create(Ops[0], Ops[1], Ops[2], "", InsertBefore);
   case Instruction::InsertElement:
-return InsertElementInst::Create(Ops[0], Ops[1], Ops[2]);
+return InsertElementInst::Create(Ops[0], Ops[1], Ops[2], "", InsertBefore);
   case Instruction::ExtractElement:
-return ExtractElementInst::Create(Ops[0], Ops[1]);
+return ExtractElementInst::Create(Ops[0], Ops[1], "", InsertBefore);
   case Instruction::InsertValue:
-return InsertValueInst::Create(Ops[0], Ops[1], getIndices());
+return InsertValueInst::Create(Ops[0], Ops[1], getIndices(), "",
+   InsertBefore);
   case Instruction::ExtractValue:
-return ExtractValueInst::Create(Ops[0], getIndices());
+return ExtractValueInst::Create(Ops[0], getIndices(), "", InsertBefore);
   case Instruction::ShuffleVector:
-return new ShuffleVectorInst(Ops[0], Ops[1], getShuffleMask());
+return new ShuffleVectorInst(Ops[0], Ops[1], getShuffleMask(), "",
+ InsertBefore);
 
   cas

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

[Jay Foad 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 rG1b758925adf6: [IR] Merge createReplacementInstr into 
ConstantExpr::getAsInstruction (authored by foad).

Repository:
  rG LLVM Github Monorepo

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

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 @@
   NewOps, this, From, To, NumUpdated, OperandNo);
 }
 
-Instruction *ConstantExpr::getAsInstruction() const {
+Instruction *ConstantExpr::getAsInstruction(Instruction *InsertBefore) const {
   SmallVector ValueOperand

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

[Ard Biesheuvel via Phabricator via cfe-commits]

ardb added a comment.

I have split off the LOAD_STACK_GUARD changes into

  [ARM] implement LOAD_STACK_GUARD for remaining targets




Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:4900
+.addImm(15)
+.addImm(0)
+.addImm(13)

nickdesaulniers wrote:
> I think we want to use `Reg` in this instruction, in order to load into the 
> specified destination?
We use Reg, no? MRC does not take reg operands, but I pass it as the target 
register.



Comment at: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp:4904
+.addImm(3)
+.add(predOps(ARMCC::AL));
 

nickdesaulniers wrote:
> do we need to add `al`?
It appears so.



Comment at: llvm/lib/Target/ARM/ARMInstrInfo.cpp:102
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;

nickdesaulniers wrote:
> do we have to worry about soft tp, ie. `__aeabi_read_tp` vs `mrc`?
I think we should only allow this hard TP is supported in the first place. I'll 
add a check somewhere.
Calling a helper in both the prologue and the epilogue for emulated TLS seems a 
bit too heavyweight to me, and the Linux kernel will not use it in that way 
anyway.



Comment at: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp:250
 
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {

nickdesaulniers wrote:
> what about `Thumb1InstrInfo::expandLoadStackGuard`? Do we have `mrc` 
> available in Thumb[1] as well?
No Thumb1 does not have an encoding for MRC so we can ignore it here. It does 
mean we should not allow this feature to be turned out for such targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112768

___
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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

To summarize what I remember from the offline discussion:

- when we have multiple decls, eliminating as many as possible will reduce 
false negatives.
- reducing false negatives isn't our top priority, so we should focus only on 
the most impactful/safe cases
- if we see a decl in the main file, we can skip other non-definition decls.
- if we see a record definition anywhere, we can skip all other decls. This can 
have false positives (opaque types whose definition is incidentally visible) 
but we think it's hugely impactful. We should be clear that we *only* plan to 
do this for plain classes, and why.

The last two points could be separate patches, up to you.

Some exceptions that seem relevant:

- we can't skip based on a definition (main file or class) that can't stand 
alone, usually because of qualifiers. e.g. `void foo::bar() {}`.
- friend declarations are special. I suspect we should conservatively never 
eliminate based on them.




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

kbobyrev wrote:
> 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!
Seems like the problem Kadir described still exists if b.h is a definition, 
you'll mark a.h as unused even though it's the only header providing the symbol 
directly at all (and maybe you don't need the definition!).

> 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?

Sorry, I can't really understand what any of the "it"s or "this"s in this 
sentence refer to :-)


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

[Sam McCall via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:250
 if (!MFI.HeaderID) {
   elog("File {0} not found.", MFI.Written);
   continue;

(sorry, I missed this before)

We should not be elogging this case, we're already emitting a diagnostic and it 
doesn't indicate anything wrong with clangd itself.



Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:256
+if (!Used && !mayConsiderUnused(MFI, AST)) {
+  dlog("{0} is not a valid unused header and will be filtered out",
+   MFI.Written);

This seems pretty vague text (particularly: what does valid mean, and is it 
valid but used or invalid but unused). Maybe "{0} was not used, but is not 
eligible to be diagnosed as unused"?



Comment at: clang-tools-extra/clangd/IncludeCleaner.h:64
 /// 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

I'm slightly worried about these sorts of comments becoming stale, they're 
really just describing what we think "not used" means.

I'd replace it with something more general like "In unclear cases, headers are 
not marked as unused", but up to you


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112695

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

[PATCH] D112765: [AST] injected-class-name is not a redecl, even in template specializations

[Sam McCall via Phabricator via cfe-commits]

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

Add a comment, remove some unneccesary special cases from traversals.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112765

Files:
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
  clang/include/clang/AST/RecursiveASTVisitor.h
  clang/lib/AST/ASTDumper.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/test/AST/ast-dump-decl.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
  clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp

Index: clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
===
--- clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
+++ clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
@@ -69,7 +69,7 @@
 // CHECK-NEXT: | | | `-Destructor simple irrelevant trivial
 // CHECK-NEXT: | | |-TemplateArgument type 'int'
 // CHECK-NEXT: | | | `-BuiltinType [[ADDR_9:0x[a-z0-9]*]] 'int'
-// CHECK-NEXT: | | |-CXXRecordDecl [[ADDR_10:0x[a-z0-9]*]] prev [[ADDR_8]]  col:30 implicit struct S
+// CHECK-NEXT: | | |-CXXRecordDecl [[ADDR_10:0x[a-z0-9]*]]  col:30 implicit struct S
 // CHECK-NEXT: | | |-CXXConstructorDecl [[ADDR_11:0x[a-z0-9]*]]  col:3 used S 'void (int, int *)'
 // CHECK-NEXT: | | | |-ParmVarDecl [[ADDR_12:0x[a-z0-9]*]]  col:8 'int'
 // CHECK-NEXT: | | | |-ParmVarDecl [[ADDR_13:0x[a-z0-9]*]]  col:13 'int *'
Index: clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
===
--- clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
+++ clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
@@ -120,7 +120,7 @@
 // CHECK-NEXT: | | |-TemplateArgument type 'float &'
 // CHECK-NEXT: | | | `-LValueReferenceType [[ADDR_7:0x[a-z0-9]*]] 'float &'
 // CHECK-NEXT: | | |   `-BuiltinType [[ADDR_8:0x[a-z0-9]*]] 'float'
-// CHECK-NEXT: | | |-CXXRecordDecl [[ADDR_9:0x[a-z0-9]*]] prev [[ADDR_6]]  col:29 implicit struct remove_reference
+// CHECK-NEXT: | | |-CXXRecordDecl [[ADDR_9:0x[a-z0-9]*]]  col:29 implicit struct remove_reference
 // CHECK-NEXT: | | `-TypedefDecl [[ADDR_10:0x[a-z0-9]*]]  col:67 referenced type 'float':'float'
 // CHECK-NEXT: | |   `-SubstTemplateTypeParmType [[ADDR_11:0x[a-z0-9]*]] 'float' sugar
 // CHECK-NEXT: | | |-TemplateTypeParmType [[ADDR_12:0x[a-z0-9]*]] '_Tp' dependent depth 0 index 0
@@ -137,7 +137,7 @@
 // CHECK-NEXT: |   |-TemplateArgument type 'short &'
 // CHECK-NEXT: |   | `-LValueReferenceType [[ADDR_15:0x[a-z0-9]*]] 'short &'
 // CHECK-NEXT: |   |   `-BuiltinType [[ADDR_16:0x[a-z0-9]*]] 'short'
-// CHECK-NEXT: |   |-CXXRecordDecl [[ADDR_17:0x[a-z0-9]*]] prev [[ADDR_14]]  col:29 implicit struct remove_reference
+// CHECK-NEXT: |   |-CXXRecordDecl [[ADDR_17:0x[a-z0-9]*]]  col:29 implicit struct remove_reference
 // CHECK-NEXT: |   `-TypedefDecl [[ADDR_18:0x[a-z0-9]*]]  col:67 referenced type 'short':'short'
 // CHECK-NEXT: | `-SubstTemplateTypeParmType [[ADDR_19:0x[a-z0-9]*]] 'short' sugar
 // CHECK-NEXT: |   |-TemplateTypeParmType [[ADDR_12]] '_Tp' dependent depth 0 index 0
Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -321,7 +321,7 @@
 // CHECK-NEXT:  | |-TemplateArgument type 'testClassTemplateDecl::A'
 // CHECK-NEXT:  | | `-RecordType 0{{.+}} 'testClassTemplateDecl::A'
 // CHECK-NEXT:  | |   `-CXXRecord 0x{{.+}} 'A'
-// CHECK-NEXT:  | |-CXXRecordDecl 0x{{.+}} prev 0x{{.+}}  col:30 implicit class TestClassTemplate
+// CHECK-NEXT:  | |-CXXRecordDecl 0x{{.+}}  col:30 implicit class TestClassTemplate
 // CHECK-NEXT:  | |-AccessSpecDecl 0x{{.+}}  col:3 public
 // CHECK-NEXT:  | |-CXXConstructorDecl 0x{{.+}}  col:5 used TestClassTemplate 'void ()'
 // CHECK-NEXT:  | |-CXXDestructorDecl 0x{{.+}}  col:5 used ~TestClassTemplate 'void () noexcept'
@@ -358,7 +358,7 @@
 // CHECK-NEXT:  |-TemplateArgument type 'testClassTemplateDecl::C'
 // CHECK-NEXT:  | `-RecordType 0{{.+}} 'testClassTemplateDecl::C'
 // CHECK-NEXT:  |   `-CXXRecord 0x{{.+}} 'C'
-// CHECK-NEXT:  |-CXXRecordDecl 0x{{.+}} prev 0x{{.+}}  col:30 implicit class TestClassTemplate
+// CHECK-NEXT:  |-CXXRecordDecl 0x{{.+}}  col:30 implicit class TestClassTemplate
 // CHECK-NEXT:  |-AccessSpecDecl 0x{{.+}}  col:3 public
 // CHECK-NEXT:  |-CXXConstructorDecl 0x{{.+}}  col:5 TestClassTemplate 'void ()'
 // CHECK-NEXT:  |-CXXDestructorDecl 0x{{.+}}  col:5 ~TestClassTemplate 'void ()' noexcept-unevaluated 0x{{.+}}
@@ -376,7 +376,7 @@
 // CHECK-NEXT:  |-TemplateArgument type 'testClassTemplateDecl::D'
 // CHECK-NEXT:  | `-RecordType 0{{.+}} 'testClassTemplateDecl::D'
 // CHECK-NEXT:  |   `-CXXRecord 0x{{.+}} 'D'
-// CHECK-NEX

[PATCH] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

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

uabelho added a comment.

Hi,

We're seeing a problem with this patch in our downstream (not public) 
buildbots. With an asan-built compiler we see the following:

  10:08:55 FAIL: Clang-Unit :: 
Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException
 (25832 of 79960)
  10:08:55  TEST 'Clang-Unit :: 
Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException'
 FAILED 
  10:08:55 Script:
  10:08:55 --
  10:08:55 
/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests
 --gtest_filter=InterpreterTest.CatchException
  10:08:55 --
  10:08:55 Note: Google Test filter = InterpreterTest.CatchException
  10:08:55 [==] Running 1 test from 1 test suite.
  10:08:55 [--] Global test environment set-up.
  10:08:55 [--] 1 test from InterpreterTest
  10:08:55 [ RUN  ] InterpreterTest.CatchException
  10:08:55 libunwind: __unw_add_dynamic_fde: bad fde: FDE is really a CIE
  10:08:55 libc++abi: terminating with uncaught exception of type 
custom_exception
  10:08:55  #0 0x0052072b backtrace 
/repo/uabelho/flacc_6.144/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:4205:13
  10:08:55  #1 0x01873774 
llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:565:13
  10:08:55  #2 0x0186c328 llvm::sys::RunSignalHandlers() 
/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Signals.cpp:0:5
  10:08:55  #3 0x018745a8 SignalHandler(int) 
/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:0:3
  10:08:55  #4 0x7f61246da630 __restore_rt sigaction.c:0:0
  10:08:55  #5 0x7f6122025387 raise (/lib64/libc.so.6+0x36387)
  10:08:55  #6 0x7f6122026a78 abort (/lib64/libc.so.6+0x37a78)
  10:08:55  #7 0x0cbdedd6 
(/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbdedd6)
  10:08:55  #8 0x0cbe4407 
(/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbe4407)
  10:08:55  #9 0x0cbdecb3 std::__terminate(void (*)()) crtstuff.c:0:0
  10:08:55 #10 0x0cbdca26 
(/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbdca26)
  10:08:55 #11 0x0cbdc9c0 
__cxxabiv1::exception_cleanup_func(_Unwind_Reason_Code, _Unwind_Exception*) 
cxa_exception.cpp:0:0
  10:08:55 #12 0x7f611ea00171 
  10:08:55 
  10:08:55 
  10:08:55 Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
  10:25:10 
  10:25:10 1 warning(s) in tests
  10:25:10 
  10:25:10 Failed Tests (1):
  10:25:10   Clang-Unit :: 
Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException
  10:25:10 

Does an asan-build run clean for everyone else?


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

https://reviews.llvm.org/D107049

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

[PATCH] D98895: [X86][clang] Disable long double type for -mno-x87 option

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:9573
 
-  checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);
+  if (D.getFunctionDefinitionKind() != FunctionDefinitionKind::Declaration)
+checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);

erichkeane wrote:
> Why are we not checking declarations here?  This doesn't seem right to me.  
> At least in the offload languages, we still need to check declarations.  
> Also, if something is a problem with a declaration, why is it not a problem 
> with defaulted/deleted?
> Why are we not checking declarations here?  This doesn't seem right to me.  
> At least in the offload languages, we still need to check declarations.

I assume that if if a function is declared and not used, then it is discarded 
and no code is generated for it. So it should not really matter if it uses an 
"unsupported" type.
This is important for `long double`, because there are C standard library 
functions that have `long double` arguments. We skip diagnostics for 
declarations to avoid errors from standard library headers when the type is 
actually not used.

> Also, if something is a problem with a declaration, why is it not a problem 
> with defaulted/deleted?

If we can expect that defaulted or deleted functions never result in a code 
with unsupported types, then we can exclude them as well. Something like this 
perhaps?
```
void no_long_double(long double) = delete;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895

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

[PATCH] D98895: [X86][clang] Disable long double type for -mno-x87 option

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:9573
 
-  checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);
+  if (D.getFunctionDefinitionKind() != FunctionDefinitionKind::Declaration)
+checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);

asavonic wrote:
> erichkeane wrote:
> > Why are we not checking declarations here?  This doesn't seem right to me.  
> > At least in the offload languages, we still need to check declarations.  
> > Also, if something is a problem with a declaration, why is it not a problem 
> > with defaulted/deleted?
> > Why are we not checking declarations here?  This doesn't seem right to me.  
> > At least in the offload languages, we still need to check declarations.
> 
> I assume that if if a function is declared and not used, then it is discarded 
> and no code is generated for it. So it should not really matter if it uses an 
> "unsupported" type.
> This is important for `long double`, because there are C standard library 
> functions that have `long double` arguments. We skip diagnostics for 
> declarations to avoid errors from standard library headers when the type is 
> actually not used.
> 
> > Also, if something is a problem with a declaration, why is it not a problem 
> > with defaulted/deleted?
> 
> If we can expect that defaulted or deleted functions never result in a code 
> with unsupported types, then we can exclude them as well. Something like this 
> perhaps?
> ```
> void no_long_double(long double) = delete;
> ```
The problem is that these declarations could be called, right?  So are we 
catching something like:

``` void has_long_double(long double d);

has_long_double(1.0); 
```

The deleted types shouldn't result in code being generated, but default will 
absolutely result in code being generated. Though I guess I can't think of a 
situation where we would have a defaulted function that could take a `long 
double` anyway.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895

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

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

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA updated this revision to Diff 383359.
ZarkoCA added a comment.

- Fix formatting and change variable names to make better grammatical sense


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112642

Files:
  clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
  clang/lib/StaticAnalyzer/Core/MemRegion.cpp
  clang/test/Analysis/vfork.c

Index: clang/test/Analysis/vfork.c
===
--- clang/test/Analysis/vfork.c
+++ clang/test/Analysis/vfork.c
@@ -15,7 +15,7 @@
   case 0:
 // Ensure that modifying pid is ok.
 pid = 1; // no-warning
-// Ensure that calling whitelisted routines is ok.
+// Ensure that calling allowlisted routines is ok.
 switch (y) {
 case 0:
   execl("", "", 0); // no-warning
@@ -65,7 +65,7 @@
   case 0:
 // Ensure that writing pid is ok.
 pid = 1; // no-warning
-// Ensure that calling whitelisted routines is ok.
+// Ensure that calling allowlisted routines is ok.
 execl("", "", 0); // no-warning
 _exit(1); // no-warning
 break;
Index: clang/lib/StaticAnalyzer/Core/MemRegion.cpp
===
--- clang/lib/StaticAnalyzer/Core/MemRegion.cpp
+++ clang/lib/StaticAnalyzer/Core/MemRegion.cpp
@@ -974,7 +974,7 @@
 
 // First handle the globals defined in system headers.
 if (Ctx.getSourceManager().isInSystemHeader(D->getLocation())) {
-  // Whitelist the system globals which often DO GET modified, assume the
+  //  Allow the system globals which often DO GET modified, assume the
   // rest are immutable.
   if (D->getName().contains("errno"))
 sReg = getGlobalsRegion(MemRegion::GlobalSystemSpaceRegionKind);
Index: clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
===
--- clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
+++ clang/lib/StaticAnalyzer/Checkers/WebKit/ASTUtils.h
@@ -29,7 +29,7 @@
 /// values).
 ///
 /// For more context see Static Analyzer checkers documentation - specifically
-/// webkit.UncountedCallArgsChecker checker. Whitelist of transformations:
+/// webkit.UncountedCallArgsChecker checker. Allowed list of transformations:
 /// - constructors of ref-counted types (including factory methods)
 /// - getters of ref-counted types
 /// - member overloaded operators
Index: clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/VforkChecker.cpp
@@ -9,7 +9,7 @@
 //  This file defines vfork checker which checks for dangerous uses of vfork.
 //  Vforked process shares memory (including stack) with parent so it's
 //  range of actions is significantly limited: can't write variables,
-//  can't call functions not in whitelist, etc. For more details, see
+//  can't call functions not in the allowed list, etc. For more details, see
 //  http://man7.org/linux/man-pages/man2/vfork.2.html
 //
 //  This checker checks for prohibited constructs in vforked process.
@@ -44,13 +44,14 @@
 class VforkChecker : public Checker> {
   mutable std::unique_ptr BT;
-  mutable llvm::SmallSet VforkWhitelist;
+  mutable llvm::SmallSet VforkAllowlist;
   mutable const IdentifierInfo *II_vfork;
 
   static bool isChildProcess(const ProgramStateRef State);
 
   bool isVforkCall(const Decl *D, CheckerContext &C) const;
-  bool isCallWhitelisted(const IdentifierInfo *II, CheckerContext &C) const;
+  bool isCallExplicitelyAllowed(const IdentifierInfo *II,
+CheckerContext &C) const;
 
   void reportBug(const char *What, CheckerContext &C,
  const char *Details = nullptr) const;
@@ -93,9 +94,9 @@
 }
 
 // Returns true iff ok to call function after successful vfork.
-bool VforkChecker::isCallWhitelisted(const IdentifierInfo *II,
- CheckerContext &C) const {
-  if (VforkWhitelist.empty()) {
+bool VforkChecker::isCallExplicitelyAllowed(const IdentifierInfo *II,
+CheckerContext &C) const {
+  if (VforkAllowlist.empty()) {
 // According to manpage.
 const char *ids[] = {
   "_Exit",
@@ -112,10 +113,10 @@
 
 ASTContext &AC = C.getASTContext();
 for (const char **id = ids; *id; ++id)
-  VforkWhitelist.insert(&AC.Idents.get(*id));
+  VforkAllowlist.insert(&AC.Idents.get(*id));
   }
 
-  return VforkWhitelist.count(II);
+  return VforkAllowlist.count(II);
 }
 
 void VforkChecker::reportBug(const char *What, CheckerContext &C,
@@ -179,12 +180,13 @@
   C.addTransition(ChildState);
 }
 

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

[Zarko Todorovski via Phabricator via cfe-commits]

ZarkoCA marked 6 inline comments as done.
ZarkoCA added a comment.

In D112642#3093559 , @aaron.ballman 
wrote:

> Thank you for this! Mostly just bikeshedding on names (allowlist as a verb 
> sounds weird to me), feel free to take or leave the suggestions. LG aside 
> from the formatting nits.

Yes, it didn't seem that right to me either but thought I'd get the ball 
rolling with something for now. Thanks for the suggestions, it looks much 
better to me now.


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] D107049: [clang-repl] Re-implement clang-interpreter as a test case.

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev marked an inline comment as done.
v.g.vassilev added a comment.

In D107049#3096727 , @uabelho wrote:

> Hi,
>
> We're seeing a problem with this patch in our downstream (not public) 
> buildbots. With an asan-built compiler we see the following:
>
>   10:08:55 FAIL: Clang-Unit :: 
> Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException
>  (25832 of 79960)
>   10:08:55  TEST 'Clang-Unit :: 
> Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException'
>  FAILED 
>   10:08:55 Script:
>   10:08:55 --
>   10:08:55 
> /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests
>  --gtest_filter=InterpreterTest.CatchException
>   10:08:55 --
>   10:08:55 Note: Google Test filter = InterpreterTest.CatchException
>   10:08:55 [==] Running 1 test from 1 test suite.
>   10:08:55 [--] Global test environment set-up.
>   10:08:55 [--] 1 test from InterpreterTest
>   10:08:55 [ RUN  ] InterpreterTest.CatchException
>   10:08:55 libunwind: __unw_add_dynamic_fde: bad fde: FDE is really a CIE
>   10:08:55 libc++abi: terminating with uncaught exception of type 
> custom_exception
>   10:08:55  #0 0x0052072b backtrace 
> /repo/uabelho/flacc_6.144/compiler-rt/lib/asan/../sanitizer_common/sanitizer_common_interceptors.inc:4205:13
>   10:08:55  #1 0x01873774 
> llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
> /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:565:13
>   10:08:55  #2 0x0186c328 llvm::sys::RunSignalHandlers() 
> /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Signals.cpp:0:5
>   10:08:55  #3 0x018745a8 SignalHandler(int) 
> /repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/../lib/Support/Unix/Signals.inc:0:3
>   10:08:55  #4 0x7f61246da630 __restore_rt sigaction.c:0:0
>   10:08:55  #5 0x7f6122025387 raise (/lib64/libc.so.6+0x36387)
>   10:08:55  #6 0x7f6122026a78 abort (/lib64/libc.so.6+0x37a78)
>   10:08:55  #7 0x0cbdedd6 
> (/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbdedd6)
>   10:08:55  #8 0x0cbe4407 
> (/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbe4407)
>   10:08:55  #9 0x0cbdecb3 std::__terminate(void (*)()) crtstuff.c:0:0
>   10:08:55 #10 0x0cbdca26 
> (/repo/bbiswjenk/fem2s10-eiffel176/workspace/llvm/llvm-main-sanitize-asan/llvm/build-all-bbisdk-asan/tools/clang/unittests/Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests+0xcbdca26)
>   10:08:55 #11 0x0cbdc9c0 
> __cxxabiv1::exception_cleanup_func(_Unwind_Reason_Code, _Unwind_Exception*) 
> cxa_exception.cpp:0:0
>   10:08:55 #12 0x7f611ea00171 
>   10:08:55 
>   10:08:55 
>   10:08:55 Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90.. 
>   10:25:10 
>   10:25:10 1 warning(s) in tests
>   10:25:10 
>   10:25:10 Failed Tests (1):
>   10:25:10   Clang-Unit :: 
> Interpreter/ExceptionTests/./ClangReplInterpreterExceptionTests/InterpreterTest.CatchException
>   10:25:10 
>
> Does an asan-build run clean for everyone else?

Can you share your cmake config line, the target triple and architecture?


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

https://reviews.llvm.org/D107049

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

[PATCH] D98895: [X86][clang] Disable long double type for -mno-x87 option

[Andrew Savonichev via Phabricator via cfe-commits]

asavonic added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:9573
 
-  checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);
+  if (D.getFunctionDefinitionKind() != FunctionDefinitionKind::Declaration)
+checkTypeSupport(NewFD->getType(), D.getBeginLoc(), NewFD);

erichkeane wrote:
> asavonic wrote:
> > erichkeane wrote:
> > > Why are we not checking declarations here?  This doesn't seem right to 
> > > me.  At least in the offload languages, we still need to check 
> > > declarations.  Also, if something is a problem with a declaration, why is 
> > > it not a problem with defaulted/deleted?
> > > Why are we not checking declarations here?  This doesn't seem right to 
> > > me.  At least in the offload languages, we still need to check 
> > > declarations.
> > 
> > I assume that if if a function is declared and not used, then it is 
> > discarded and no code is generated for it. So it should not really matter 
> > if it uses an "unsupported" type.
> > This is important for `long double`, because there are C standard library 
> > functions that have `long double` arguments. We skip diagnostics for 
> > declarations to avoid errors from standard library headers when the type is 
> > actually not used.
> > 
> > > Also, if something is a problem with a declaration, why is it not a 
> > > problem with defaulted/deleted?
> > 
> > If we can expect that defaulted or deleted functions never result in a code 
> > with unsupported types, then we can exclude them as well. Something like 
> > this perhaps?
> > ```
> > void no_long_double(long double) = delete;
> > ```
> The problem is that these declarations could be called, right?  So are we 
> catching something like:
> 
> ``` void has_long_double(long double d);
> 
> has_long_double(1.0); 
> ```
> 
> The deleted types shouldn't result in code being generated, but default will 
> absolutely result in code being generated. Though I guess I can't think of a 
> situation where we would have a defaulted function that could take a `long 
> double` anyway.
> The problem is that these declarations could be called, right?  So are we 
> catching something like:
> 
> ``` void has_long_double(long double d);
> 
> has_long_double(1.0); 
> ```

Yes, we catch this when the function is called (see below). This is done in 
`Sema::DiagnoseUseOfDecl`.


> The deleted types shouldn't result in code being generated, but default will 
> absolutely result in code being generated. Though I guess I can't think of a 
> situation where we would have a defaulted function that could take a `long 
> double` anyway.

Thank you. I'll update the patch to exclude deleted functions.




Comment at: clang/test/Sema/x86-no-x87.c:35
+#endif
+  return ld_args(x, y);
+}

This is the case where a declaration is called.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98895

___
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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

In D108696#3095789 , @ChuanqiXu wrote:

> 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 ?

I'm not aware of any similar situations involving ``. Most 
stuff in `` is just library stuff, like 
`std::experimental::pmr`, where the two versions can happily coexist because we 
have namespaces. Coroutines are special (and really annoying) because they're a 
core-language feature with a "magic" relationship to a specific library header. 
(Core-language syntax doesn't respect namespaces.) The only other "magic" 
features like that are

- `typeid` looks for `std::type_info`
- `{}` looks for `std::initializer_list`
- `auto [a,b]` looks for `std::tuple_size` etc.
- `<=>` looks for `std::strong_ordering` etc.

None of these were ever prototyped in `` before being put into 
the Standard, so they never ran into this problem.


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] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

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

Address comments.


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

https://reviews.llvm.org/D110215

Files:
  clang/include/clang/Basic/Module.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaModule.cpp
  clang/test/CXX/module/module.linkage_specification/Inputs/h1.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
  clang/test/CXX/module/module.linkage_specification/Inputs/h5.h
  clang/test/CXX/module/module.linkage_specification/p1.cpp
  clang/test/CXX/module/module.linkage_specification/p2.cpp
  clang/test/CXX/module/module.linkage_specification/p3.cpp
  clang/test/CXX/module/module.linkage_specification/p4.cpp
  clang/test/CXX/module/module.linkage_specification/p5.cpp
  clang/test/CodeGenCXX/Inputs/module-extern-C.h
  clang/test/CodeGenCXX/module-extern-C.cpp

Index: clang/test/CodeGenCXX/module-extern-C.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/module-extern-C.cpp
@@ -0,0 +1,27 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm -triple %itanium_abi_triple -o - %s | FileCheck %s
+
+module;
+
+#include "Inputs/module-extern-C.h"
+
+export module x;
+
+// CHECK: define dso_local void @foo()
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+// CHECK: define dso_local void @bar()
+void bar() {
+  return;
+}
+// CHECK: define dso_local i32 @baz()
+int baz() {
+  return 3;
+}
+// CHECK: define dso_local double @double_func()
+double double_func() {
+  return 5.0;
+}
+}
Index: clang/test/CodeGenCXX/Inputs/module-extern-C.h
===
--- /dev/null
+++ clang/test/CodeGenCXX/Inputs/module-extern-C.h
@@ -0,0 +1,7 @@
+extern "C" void foo();
+extern "C" {
+void bar();
+int baz();
+double double_func();
+}
+extern "C++" class CPP;
Index: clang/test/CXX/module/module.linkage_specification/p5.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p5.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C++" int a = 5;
Index: clang/test/CXX/module/module.linkage_specification/p4.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p4.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h4.h"
+
+export module x;
+
+extern "C" struct C {
+  int a;
+  int b;
+  double d;
+};
Index: clang/test/CXX/module/module.linkage_specification/p3.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p3.cpp
@@ -0,0 +1,7 @@
+// This tests whether the global module would be created when the program don't declare it explicitly.
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+export module x;
+
+extern "C" void foo();
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p2.cpp
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h2.h"
+
+export module x;
+
+extern "C++" class CPP {};
Index: clang/test/CXX/module/module.linkage_specification/p1.cpp
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/p1.cpp
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -std=c++20 %s -verify
+// expected-no-diagnostics
+module;
+
+#include "Inputs/h1.h"
+
+export module x;
+
+extern "C" void foo() {
+  return;
+}
+
+extern "C" {
+void bar() {
+  return;
+}
+int baz() {
+  return 3;
+}
+double double_func() {
+  return 5.0;
+}
+}
+
+extern "C++" {
+void bar_cpp() {
+  return;
+}
+int baz_cpp() {
+  return 3;
+}
+double double_func_cpp() {
+  return 5.0;
+}
+}
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h5.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h5.h
@@ -0,0 +1 @@
+extern "C++" int a;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inputs/h4.h
@@ -0,0 +1 @@
+extern "C" struct C;
Index: clang/test/CXX/module/module.linkage_specification/Inputs/h2.h
===
--- /dev/null
+++ clang/test/CXX/module/module.linkage_specification/Inpu

[PATCH] D110215: [C++2a] [Module] Support extern C/C++ semantics

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu marked 7 inline comments as done.
ChuanqiXu added inline comments.



Comment at: clang/lib/Sema/SemaModule.cpp:722-723
+void Sema::PopGlobalModuleFragment() {
+  assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
+ "left the wrong module scope, which is not global module fragment");
+  ModuleScopes.pop_back();

aaron.ballman wrote:
> FWIW, this seems to be failing to compile according to the precommit CI.
> ```
> FAILED: tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o
> CCACHE_CPP2=yes CCACHE_HASHDIR=yes /usr/bin/ccache /usr/bin/clang++ 
> -DBUILD_EXAMPLES -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG 
> -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> -D__STDC_LIMIT_MACROS -Itools/clang/lib/Sema 
> -I/var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema 
> -I/var/lib/buildkite-agent/builds/llvm-project/clang/include 
> -Itools/clang/include -Iinclude 
> -I/var/lib/buildkite-agent/builds/llvm-project/llvm/include -gmlt -fPIC 
> -fvisibility-inlines-hidden -Werror=date-time 
> -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
> -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
> -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
> -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> -fdata-sections -fno-common -Woverloaded-virtual -Wno-nested-anon-types -O3 
> -DNDEBUG  -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT 
> tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -MF 
> tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o.d -o 
> tools/clang/lib/Sema/CMakeFiles/obj.clangSema.dir/SemaModule.cpp.o -c 
> /var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp
> /var/lib/buildkite-agent/builds/llvm-project/clang/lib/Sema/SemaModule.cpp:722:53:
>  error: member reference type 'clang::Module *' is a pointer; did you mean to 
> use '->'?
>   assert(!ModuleScopes.empty() && getCurrentModule().isGlobalModule() &&
>   ~~^
> ->
> /usr/include/assert.h:93:27: note: expanded from macro 'assert'
>  (static_cast  (expr) \
>   ^~~~
> 1 error generated.
> ```
Oh, my bad. I forgot to run tests under debug locally.


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

https://reviews.llvm.org/D110215

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

[PATCH] D109950: [Clang] Enable IC/IF mode for __ibm128

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews added a comment.

This has changed the behavior for -

  // Define __complex128 type corresponding to __float128 (as in GCC headers).
  typedef _Complex float __attribute__((mode(TC))) __complex128;
  
  void check() {
  // CHECK: alloca { fp128, fp128 }   <- Fails because alloca { x86_fp80, 
x86_fp80 } is generated instead
  __complex128 tmp;
  }

In TargetInfo.cpp, we used to check long double format to decide whether to 
return LongDouble or Float128. This patch doesn't, and so the return value has 
changed from FloatModeKind::Float128 to clang::FloatModeKind::LongDouble 
(ExplicitType).  Should we be checking the format?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109950

___
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 383378.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.

Resolve the 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 &Include : 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 &Includes, const SourceManager &SM);
 
 /// Retrieves headers that are referenced from the main file but not used.
+/// In unclear cases, headers are not marked as unused.
 std::vector
-getUnused(const IncludeStructure &Includes,
+getUnused(ParsedAST &AST,
   const llvm::DenseSet &ReferencedFiles);
 
 std::vector computeUnusedIncludes(ParsedAST &AST);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleane

[clang-tools-extra] f47564e - [clangd] IncludeCleaner: Skip non self-contained headers

[Kirill Bobyrev via cfe-commits]

Author: Kirill Bobyrev
Date: 2021-10-29T17:57:31+02:00
New Revision: f47564ea87a502f5fc6320ba5b57e91186751b12

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

LOG: [clangd] IncludeCleaner: Skip non self-contained headers

Headers without include guards might have side effects or can be the files we
don't want to consider (e.g. tablegen ".inc" files). Skip them when translating
headers to the HeaderIDs that we will consider as unused.

Reviewed By: sammccall

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

Added: 


Modified: 
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

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 240eb864d087c..eb593ad86ea00 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/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"
@@ -186,12 +189,28 @@ void findReferencedMacros(ParsedAST &AST, 
ReferencedLocations &Result) {
   }
 }
 
-// FIXME(kirillbobyrev): We currently do not support the umbrella headers.
-// Standard Library headers are typically umbrella headers, and system headers
-// are likely to be the Standard Library headers. Until we have a good support
-// for umbrella headers and Standard Library headers, don't warn about them.
-bool mayConsiderUnused(const Inclusion *Inc) {
-  return Inc->Written.front() != '<';
+bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST) {
+  // FIXME(kirillbobyrev): We currently do not support the umbrella headers.
+  // Standard Library headers are typically umbrella headers, and system
+  // headers are likely to be the Standard Library headers. Until we have a
+  // good support for umbrella headers and Standard Library headers, don't warn
+  // about them.
+  if (Inc.Written.front() == '<')
+return false;
+  // Headers without include guards have side effects and are not
+  // self-contained, skip them.
+  assert(Inc.HeaderID);
+  auto FE = AST.getSourceManager().getFileManager().getFile(
+  AST.getIncludeStructure().getRealPath(
+  static_cast(*Inc.HeaderID)));
+  assert(FE);
+  if 
(!AST.getPreprocessor().getHeaderSearchInfo().isFileMultipleIncludeGuarded(
+  *FE)) {
+dlog("{0} doesn't have header guard and will not be considered unused",
+ (*FE)->getName());
+return false;
+  }
+  return true;
 }
 
 } // namespace
@@ -224,21 +243,23 @@ findReferencedFiles(const llvm::DenseSet 
&Locs,
 }
 
 std::vector
-getUnused(const IncludeStructure &Includes,
+getUnused(ParsedAST &AST,
   const llvm::DenseSet &ReferencedFiles) {
   trace::Span Tracer("IncludeCleaner::getUnused");
   std::vector Unused;
-  for (const Inclusion &MFI : Includes.MainFileIncludes) {
-// FIXME: Skip includes that are not self-contained.
-if (!MFI.HeaderID) {
-  elog("File {0} not found.", MFI.Written);
+  for (const Inclusion &MFI : AST.getIncludeStructure().MainFileIncludes) {
+if (!MFI.HeaderID)
   continue;
-}
 auto IncludeID = static_cast(*MFI.HeaderID);
-if (!ReferencedFiles.contains(IncludeID))
+bool Used = ReferencedFiles.contains(IncludeID);
+if (!Used && !mayConsiderUnused(MFI, AST)) {
+  dlog("{0} was not used, but is not eligible to be diagnosed as unused",
+   MFI.Written);
+  continue;
+}
+if (!Used)
   Unused.push_back(&MFI);
-dlog("{0} is {1}", MFI.Written,
- ReferencedFiles.contains(IncludeID) ? "USED" : "UNUSED");
+dlog("{0} is {1}", MFI.Written, Used ? "USED" : "UNUSED");
   }
   return Unused;
 }
@@ -275,9 +296,15 @@ std::vector 
computeUnusedIncludes(ParsedAST &AST) {
   const auto &SM = AST.getSourceManager();
 
   auto Refs = findReferencedLocations(AST);
-  auto ReferencedFiles = translateToHeaderIDs(findReferencedFiles(Refs, SM),
-  AST.getIncludeStructure(), SM);
-  return getUnused(AST.getIncludeStructure(), ReferencedFiles);
+  // FIXME(kirillbobyrev): Attribute the symbols from non self-contained
+  // headers to their parents while the information about respective
+  // SourceLocations and FileIDs is not lost. It is no

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

[Kirill Bobyrev 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 rGf47564ea87a5: [clangd] IncludeCleaner: Skip non 
self-contained headers (authored by kbobyrev).

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 &Include : 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 &Includes, const SourceManager &SM);
 
 /// Retrieves headers that are referenced from the main file but not used.
+/// In unclear cases, headers are not marked as unused.
 std::vector
-getUnused(const IncludeStructure &Includes,
+getUnused(ParsedAST &AST,
   const llvm::DenseSet &ReferencedFiles);
 
 std::vector computeUnusedIncludes(ParsedAST &AST);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp

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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D108696#3096822 , @Quuxplusone 
wrote:

> In D108696#3095789 , @ChuanqiXu 
> wrote:
>
>> 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 ?
>
> I'm not aware of any similar situations involving ``. Most 
> stuff in `` is just library stuff, like 
> `std::experimental::pmr`, where the two versions can happily coexist because 
> we have namespaces. Coroutines are special (and really annoying) because 
> they're a core-language feature with a "magic" relationship to a specific 
> library header. (Core-language syntax doesn't respect namespaces.) The only 
> other "magic" features like that are
>
> - `typeid` looks for `std::type_info`
> - `{}` looks for `std::initializer_list`
> - `auto [a,b]` looks for `std::tuple_size` etc.
> - `<=>` looks for `std::strong_ordering` etc.
>
> None of these were ever prototyped in `` before being put 
> into the Standard, so they never ran into this problem.

Oh, got it. So now is the time to do policy decision and the community didn't 
meet similar problem before. Here is my reason for not supporting both:

- It is complex to implement.
- It would be removed in recent future. Complexity is not a very strong reason 
to refuse a need. But it is frustrating to implement a feature which would be 
removed in a predictable recent future.
- From the perspective of users, it shouldn't be hard to switch from 
std::experimental::coro* to std::coro*. I write coroutine codes in production 
too. AFAIK, it should be easy. (I understand that I couldn't stand for all the 
users)

Here are my reasons. But if we couldn't get in consensus. I guess we could only 
post this to cfe-dev to ask more people for involving in.


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] D112491: Add `LambdaCapture`-related matchers.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

This looks really sensible to me too. Some naming bikeshedding, but my main 
question is migration.

Supporting two things is indeed annoying and confusing. I agree it's not worth 
keeping the old way forever just to avoid writing `refersToVarDecl`.
The question is: how soon can we get rid of it? We should consider whether we 
can do it in this patch: just replace the old matcher with this one.

AFAICT the old matcher has no uses in-tree beyond tests/registration. FWIW it 
has no uses in our out-of-tree repo either (which generally is a heavy matchers 
user).
I'm sure it has users somewhere, but probably few, and the mechanical 
difficulty of updating them is low. Given that I think we should just break 
them, rather than deal with overloading and deprecation warnings and future 
cleanup just to put off breaking them for one more release cycle.

This is a tradeoff, to me it seems but reasonable people can disagree on the 
importance of stability. Aaron, happy to go with whatever you decide.




Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4632
+/// refersToVarDecl(hasName("x")) matches `int x` and `x = 1`.
+AST_MATCHER_P(LambdaCapture, refersToVarDecl, internal::Matcher,
+  InnerMatcher) {

I think `capturesVar` may be a clearer/more direct name.

For example given `int y; [x(y)] { return x; }` I would naively expect 
`refersToVarDecl(hasName("y"))` to match the capture.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:4652
+///   matches `[this]() { return cc; }`.
+AST_MATCHER(LambdaCapture, refersToThis) { return Node.capturesThis(); }
+

Again, why `refersToThis` rather than `capturesThis`, which is more specific 
and matches the AST?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

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

[PATCH] D112765: [AST] injected-class-name is not a redecl, even in template specializations

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1841-1842
+   /*DelayTypeCreation=*/IsInjectedClassName);
+  if (IsInjectedClassName)
+SemaRef.Context.getTypeDeclType(Record, cast(Owner));
 

hokein wrote:
> aaron.ballman wrote:
> > Why is this call needed? (It seems strange to me that we call it and ignore 
> > the return value.)
> my understanding is
>   - `getTypeDeclType` is more than a getter, it also has a side-effort -- if 
> the type of the passed `Record` is empty, it creates a type, and propagates 
> the type to `Record->TypeForDecl`;
>   - from the above line, we delay the type creation when 
> `IsInjectedClassName` is true;
>   - so we need to create a type for the `Record` by invoking 
> `getTypeDeclType`;
> 
> might be worth a comment.
> 
That's exactly right. The injected-class-name is an independent decl but yields 
the same RecordType, and getTypeDeclType has a hacky interface so we can force 
this link.
(It wasn't needed in the old version of the code, because when it was a redecl 
getTypeDeclType would yield the same type in any case)

I should have mentioned this is all derived from the injected-class-name 
handling in Sema::ActOnStartCXXMemberDeclarations(). It lacks comments, but 
I'll add a few while here.



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1910
 
+  if (IsInjectedClassName)
+assert(Record->isInjectedClassName() && "Broken injected-class-name");

hokein wrote:
> it is unclear to me what's the intention of the assertion.
We needed to do a few weird things (and to assume some things about the input) 
to initialize the injected-class-name, it verifies we got them all right.

(This assertion is also in ActOnStartCXXMemberDeclarations())


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112765

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

[clang] 72c3736 - [OpenMP] Add triple to run lines to avoid message differences

[Mike Rice via cfe-commits]

Author: Mike Rice
Date: 2021-10-29T09:20:40-07:00
New Revision: 72c373644fc397e9443200865b58514a3d6cf69d

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

LOG: [OpenMP] Add triple to run lines to avoid message differences

Diagnostics with function types can show calling conventions on some platforms.
Just choose one to prevent that.

Added: 


Modified: 
clang/test/OpenMP/declare_variant_clauses_messages.cpp

Removed: 




diff  --git a/clang/test/OpenMP/declare_variant_clauses_messages.cpp 
b/clang/test/OpenMP/declare_variant_clauses_messages.cpp
index 648a1c1201c4b..e4db5b4e44eda 100644
--- a/clang/test/OpenMP/declare_variant_clauses_messages.cpp
+++ b/clang/test/OpenMP/declare_variant_clauses_messages.cpp
@@ -1,8 +1,8 @@
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=51 -DOMP51 -std=c++11 -o 
- %s
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=51 -DOMP51 -std=c++11 \
+// RUN: %clang_cc1 -verify -triple x86_64-unknown-linux -fopenmp 
-fopenmp-version=51 -DOMP51 -std=c++11 -o - %s
+// RUN: %clang_cc1 -verify -triple x86_64-unknown-linux -fopenmp 
-fopenmp-version=51 -DOMP51 -std=c++11 \
 // RUN:  -DNO_INTEROP_T_DEF -o - %s
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 -DOMP50 -std=c++11 -o 
- %s
-// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=51 -DOMP51 -DC -x c -o - 
%s
+// RUN: %clang_cc1 -verify -triple x86_64-unknown-linux -fopenmp 
-fopenmp-version=50 -DOMP50 -std=c++11 -o - %s
+// RUN: %clang_cc1 -verify -triple x86_64-unknown-linux -fopenmp 
-fopenmp-version=51 -DOMP51 -DC -x c -o - %s
 
 #ifdef NO_INTEROP_T_DEF
 void foo_v1(float *, void *);



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

[PATCH] D112768: [ARM] implement support for TLS register based stack protector

[Ard Biesheuvel via Phabricator via cfe-commits]

ardb updated this revision to Diff 383390.
ardb edited the summary of this revision.
ardb added a comment.

- split off LOAD_STACK_GUARD conversion
- deal with guard offsets >= 4096 bytes
- reject offsets < 0 or >= 1 MiB
- add backend test to check that the MRC/LDR sequence is emitted twice


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112768

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/Driver/stack-protector-guard.c
  llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
  llvm/lib/Target/ARM/ARMInstrInfo.cpp
  llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
  llvm/test/CodeGen/ARM/stack-guard-tls.ll

Index: llvm/test/CodeGen/ARM/stack-guard-tls.ll
===
--- /dev/null
+++ llvm/test/CodeGen/ARM/stack-guard-tls.ll
@@ -0,0 +1,38 @@
+; RUN: split-file %s %t
+; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
+; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
+; RUN: llc %t/a2.ll -mtriple=armv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-SMALL %s
+; RUN: llc %t/a2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-SMALL %s
+; RUN: llc %t/b2.ll -mtriple=armv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-LARGE %s
+; RUN: llc %t/b2.ll -mtriple=thumbv7-unknown-linux-gnueabihf -o - | \
+; RUN: FileCheck --check-prefix=CHECK --check-prefix=CHECK-LARGE %s
+
+;--- main.ll
+declare void @baz(i32*)
+
+define void @foo(i64 %t) sspstrong {
+  %vla = alloca i32, i64 %t, align 4
+  call void @baz(i32* nonnull %vla)
+  ret void
+}
+!llvm.module.flags = !{!1, !2}
+!1 = !{i32 2, !"stack-protector-guard", !"tls"}
+
+;--- a.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 1296}
+
+;--- b.ll
+!2 = !{i32 2, !"stack-protector-guard-offset", i32 4296}
+
+; CHECK: mrc p15, #0, [[REG1:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG1]], [[REG1]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG1]], {{\[}}[[REG1]], #200]
+; CHECK: bl baz
+; CHECK: mrc p15, #0, [[REG2:r[0-9]+]], c13, c0, #3
+; CHECK-SMALL-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #1296]
+; CHECK-LARGE-NEXT: add{{(\.w)?}} [[REG2]], [[REG2]], #4096
+; CHECK-LARGE-NEXT: ldr{{(\.w)?}} [[REG2]], {{\[}}[[REG2]], #200]
Index: llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
===
--- llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
+++ llvm/lib/Target/ARM/Thumb2InstrInfo.cpp
@@ -250,6 +250,14 @@
 void Thumb2InstrInfo::expandLoadStackGuard(
 MachineBasicBlock::iterator MI) const {
   MachineFunction &MF = *MI->getParent()->getParent();
+  MachineBasicBlock &MBB = *MI->getParent();
+  Module &M = *MBB.getParent()->getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::t2MRC, ARM::t2LDRi12);
+return;
+  }
+
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
 
Index: llvm/lib/Target/ARM/ARMInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMInstrInfo.cpp
@@ -95,6 +95,13 @@
   MachineFunction &MF = *MI->getParent()->getParent();
   const ARMSubtarget &Subtarget = MF.getSubtarget();
   const TargetMachine &TM = MF.getTarget();
+  MachineBasicBlock &MBB = *MI->getParent();
+  Module &M = *MBB.getParent()->getFunction().getParent();
+
+  if (M.getStackProtectorGuard() == "tls") {
+expandLoadStackGuardBase(MI, ARM::MRC, ARM::LDRi12);
+return;
+  }
 
   const GlobalValue *GV =
   cast((*MI->memoperands_begin())->getValue());
@@ -117,7 +124,6 @@
 return;
   }
 
-  MachineBasicBlock &MBB = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg = MI->getOperand(0).getReg();
   MachineInstrBuilder MIB;
Index: llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
===
--- llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
+++ llvm/lib/Target/ARM/ARMBaseInstrInfo.cpp
@@ -4891,40 +4891,67 @@
   MachineBasicBlock &MBB = *MI->getParent();
   DebugLoc DL = MI->getDebugLoc();
   Register Reg = MI->getOperand(0).getReg();
-  const GlobalValue *GV =
-  cast((*MI->memoperands_begin())->getValue());
-  bool IsIndirect = Subtarget.isGVIndirectSymbol(GV);
   MachineInstrBuilder MIB;
+  unsigned int Offset = 0;
 
-  unsigned TargetFlags = ARMII::MO_NO_FLAG;
-  if (Subtarget.isTargetMachO()) {
-TargetFlags |= ARMII::MO_NONLAZY;
-  } else if (Subtarget.isTargetCOFF()) {
-if (GV->hasDLLImportStorageClass())
-  TargetFlags |= ARMII::MO_DLLIMPORT;
-else if (IsIndirect)
-  TargetFlags |= ARMII::MO_COFFSTUB;
-  } else if (Subtarget.isGVInGOT(GV)) {
-TargetFlags |= ARMII::MO_GOT;
-  }
+  if (LoadImmOpc == ARM::MR

[PATCH] D111654: [analyzer] Retrieve a value from list initialization of multi-dimensional array declaration.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 383396.
ASDenysPetrov added a comment.

- Added support of aliased types.
- Added tests aliased types.
- Added tests with initialization values in parentheses.




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

https://reviews.llvm.org/D111654

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/initialization.c
  clang/test/Analysis/initialization.cpp

Index: clang/test/Analysis/initialization.cpp
===
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -14,13 +14,6 @@
   clang_analyzer_eval(sarr[i].a); // expected-warning{{UNKNOWN}}
 }
 
-int const arr[2][2] = {};
-void arr2init() {
-  int i = 1;
-  // FIXME: Should recognize that it is 0.
-  clang_analyzer_eval(arr[i][0]); // expected-warning{{UNKNOWN}}
-}
-
 int const glob_arr1[3] = {};
 void glob_array_index1() {
   clang_analyzer_eval(glob_arr1[0] == 0); // expected-warning{{TRUE}}
@@ -60,23 +53,18 @@
   return glob_arr3[0]; // no-warning (garbage or undefined)
 }
 
-// TODO: Support multidimensional array.
 int const glob_arr4[4][2] = {};
 void glob_array_index2() {
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr4[1][0] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr4[1][1] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr4[0][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr4[1][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr4[1][1] == 0); // expected-warning{{TRUE}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index3() {
   int idx = -42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = glob_arr4[1][idx]; // no-warning
+  auto x = glob_arr4[1][idx]; // expected-warning{{garbage or undefined}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index4() {
   const int *ptr = glob_arr4[1];
   int idx = -42;
@@ -84,28 +72,18 @@
   auto x = ptr[idx]; // no-warning
 }
 
-// TODO: Support multidimensional array.
 int const glob_arr5[4][2] = {{1}, 3, 4, 5};
 void glob_array_index3() {
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[0][0] == 1); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[0][1] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[1][0] == 3); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[1][1] == 4); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[2][0] == 5); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[2][1] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[3][0] == 0); // expected-warning{{UNKNOWN}}
-  // FIXME: Should be TRUE.
-  clang_analyzer_eval(glob_arr5[3][1] == 0); // expected-warning{{UNKNOWN}}
+  clang_analyzer_eval(glob_arr5[0][0] == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[0][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[1][0] == 3); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[1][1] == 4); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[2][0] == 5); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[2][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[3][0] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr5[3][1] == 0); // expected-warning{{TRUE}}
 }
 
-// TODO: Support multidimensional array.
 void glob_ptr_index2() {
   int const *ptr = glob_arr5[1];
   // FIXME: Should be TRUE.
@@ -120,19 +98,16 @@
   clang_analyzer_eval(ptr[4] == 0); // expected-warning{{UNKNOWN}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index5() {
   int idx = -42;
-  // FIXME: Should warn {{garbage or undefined}}.
-  auto x = glob_arr5[1][idx]; // no-warning
+  auto x = glob_arr5[1][idx]; // expected-warning{{garbage or undefined}}
 }
 
-// TODO: Support multidimensional array.
 void glob_invalid_index6() {
   int const *ptr = &glob_arr5[1][0];
   int idx = 42;
   // FIXME: Should warn {{garbage or undefined}}.
-  auto x = ptr[idx]; // // no-warning
+  auto x = ptr[idx]; // no-warning
 }
 
 extern const int glob_arr_no_init[10];
@@ -253,3 +228,31 @@
   clang_analyzer_eval(glob_ptr12[2] == 'c');  // expected-warning{{TRUE}}
   clang_analyzer_eval(glob_ptr12[3] == '\0'); // expected-warning{{TRUE}}
 }
+
+typedef int Int;
+typedef Int const CInt;
+typedef CInt Arr[2];
+typedef Arr Arr2[4];
+Arr2 glob_arr8 = {{1}, 3, 4, 5}; // const int[4][2]
+void glob_array_typedef1() {
+  clang_analyzer_eval(glob_arr8[0][0] == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr8[0][1] == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr8[1][0]

[clang] 1deccd0 - [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

[Denys Petrov via cfe-commits]

Author: Denys Petrov
Date: 2021-10-29T19:44:37+03:00
New Revision: 1deccd05ba8a326e57a513002f482ed0924b1fd8

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

LOG: [analyzer] Retrieve a character from StringLiteral as an initializer for 
constant arrays.

Summary: Assuming that values of constant arrays never change, we can retrieve 
values for specific position(index) right from the initializer, if presented. 
Retrieve a character code by index from StringLiteral which is an initializer 
of constant arrays in global scope.

This patch has a known issue of getting access to characters past the end of 
the literal. The declaration, in which the literal is used, is an implicit cast 
of kind `array-to-pointer`. The offset should be in literal length's bounds. 
This should be distinguished from the states in the Standard C++20 
[dcl.init.string] 9.4.2.3. Example:
  const char arr[42] = "123";
  char c = arr[41]; // OK
  const char * const str = "123";
  char c = str[41]; // NOK

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

Added: 


Modified: 
clang/lib/StaticAnalyzer/Core/RegionStore.cpp
clang/test/Analysis/initialization.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp 
b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
index 5565e9cfd08cf..79d2fc76a42f1 100644
--- a/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -441,6 +441,8 @@ class RegionStoreManager : public StoreManager {
   RegionBindingsConstRef B, const VarRegion *VR, const ElementRegion *R);
   Optional getSValFromInitListExpr(const InitListExpr *ILE,
  uint64_t Offset, QualType ElemT);
+  SVal getSValFromStringLiteral(const StringLiteral *SL, uint64_t Offset,
+QualType ElemT);
 
 public: // Part of public interface to class.
 
@@ -1701,10 +1703,16 @@ Optional 
RegionStoreManager::getConstantValFromConstArrayInitializer(
   // From here `Offset` is in the bounds.
 
   // Handle InitListExpr.
+  // Example:
+  //   const char arr[] = { 1, 2, 3 };
   if (const auto *ILE = dyn_cast(Init))
 return getSValFromInitListExpr(ILE, Offset, R->getElementType());
 
-  // FIXME: Handle StringLiteral.
+  // Handle StringLiteral.
+  // Example:
+  //   const char arr[] = "abc";
+  if (const auto *SL = dyn_cast(Init))
+return getSValFromStringLiteral(SL, Offset, R->getElementType());
 
   // FIXME: Handle CompoundLiteralExpr.
 
@@ -1716,6 +1724,15 @@ RegionStoreManager::getSValFromInitListExpr(const 
InitListExpr *ILE,
 uint64_t Offset, QualType ElemT) {
   assert(ILE && "InitListExpr should not be null");
 
+  // C++20 [dcl.init.string] 9.4.2.1:
+  //   An array of ordinary character type [...] can be initialized by [...]
+  //   an appropriately-typed string-literal enclosed in braces.
+  // Example:
+  //   const char arr[] = { "abc" };
+  if (ILE->isStringLiteralInit())
+if (const auto *SL = dyn_cast(ILE->getInit(0)))
+  return getSValFromStringLiteral(SL, Offset, ElemT);
+
   // C++20 [expr.add] 9.4.17.5 (excerpt):
   //   i-th array element is value-initialized for each k < i ≤ n,
   //   where k is an expression-list size and n is an array extent.
@@ -1728,6 +1745,42 @@ RegionStoreManager::getSValFromInitListExpr(const 
InitListExpr *ILE,
   return svalBuilder.getConstantVal(E);
 }
 
+/// Returns an SVal, if possible, for the specified position in a string
+/// literal.
+///
+/// \param SL The given string literal.
+/// \param Offset The unsigned offset. E.g. for the expression
+///   `char x = str[42];` an offset should be 42.
+///   E.g. for the string "abc" offset:
+///   - 1 returns SVal{b}, because it's the second position in the string.
+///   - 42 returns SVal{0}, because there's no explicit value at this
+/// position in the string.
+/// \param ElemT The type of the result SVal expression.
+///
+/// NOTE: We return `0` for every offset >= the literal length for array
+/// declarations, like:
+///   const char str[42] = "123"; // Literal length is 4.
+///   char c = str[41];   // Offset is 41.
+/// FIXME: Nevertheless, we can't do the same for pointer declaraions, like:
+///   const char * const str = "123"; // Literal length is 4.
+///   char c = str[41];   // Offset is 41. Returns `0`, but Undef
+///   // expected.
+/// It should be properly handled before reaching this point.
+/// The main problem is that we can't distinguish between these declarations,
+/// because in case of array we can get the Decl from VarRegion, but in case
+/// of pointer the region is a StringRegion, which doesn't contain a Decl.
+/// Possible solution could be 

[PATCH] D107339: [analyzer] Retrieve a character from StringLiteral as an initializer for constant arrays.

[Denys Petrov via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1deccd05ba8a: [analyzer] Retrieve a character from 
StringLiteral as an initializer for… (authored by ASDenysPetrov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107339

Files:
  clang/lib/StaticAnalyzer/Core/RegionStore.cpp
  clang/test/Analysis/initialization.cpp

Index: clang/test/Analysis/initialization.cpp
===
--- clang/test/Analysis/initialization.cpp
+++ clang/test/Analysis/initialization.cpp
@@ -146,3 +146,110 @@
 void struct_arr_index1() {
   clang_analyzer_eval(S2::arr_no_init[2]); // expected-warning{{UNKNOWN}}
 }
+
+char const glob_arr6[5] = "123";
+void glob_array_index5() {
+  clang_analyzer_eval(glob_arr6[0] == '1');  // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr6[1] == '2');  // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr6[2] == '3');  // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr6[3] == '\0'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr6[4] == '\0'); // expected-warning{{TRUE}}
+}
+
+void glob_ptr_index3() {
+  char const *ptr = glob_arr6;
+  clang_analyzer_eval(ptr[-42] == '\0'); // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[0] == '1');// expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[1] == '2');// expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[2] == '3');// expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[3] == '\0');   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[4] == '\0');   // expected-warning{{TRUE}}
+  clang_analyzer_eval(ptr[5] == '\0');   // expected-warning{{UNDEFINED}}
+  clang_analyzer_eval(ptr[6] == '\0');   // expected-warning{{UNDEFINED}}
+}
+
+void glob_invalid_index7() {
+  int idx = -42;
+  auto x = glob_arr6[idx]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_invalid_index8() {
+  const char *ptr = glob_arr6;
+  int idx = 42;
+  auto x = ptr[idx]; // expected-warning{{garbage or undefined}}
+}
+
+char const glob_arr7[5] = {"123"};
+void glob_array_index6() {
+  clang_analyzer_eval(glob_arr7[0] == '1');  // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr7[1] == '2');  // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr7[2] == '3');  // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr7[3] == '\0'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_arr7[4] == '\0'); // expected-warning{{TRUE}}
+}
+
+void glob_invalid_index9() {
+  int idx = -42;
+  auto x = glob_arr7[idx]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_invalid_index10() {
+  const char *ptr = glob_arr7;
+  int idx = 42;
+  auto x = ptr[idx]; // expected-warning{{garbage or undefined}}
+}
+
+char const *const glob_ptr8 = "123";
+void glob_ptr_index4() {
+  clang_analyzer_eval(glob_ptr8[0] == '1');  // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr8[1] == '2');  // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr8[2] == '3');  // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr8[3] == '\0'); // expected-warning{{TRUE}}
+  // FIXME: Should be UNDEFINED.
+  // We should take into account a declaration in which the literal is used.
+  clang_analyzer_eval(glob_ptr8[4] == '\0'); // expected-warning{{TRUE}}
+}
+
+void glob_invalid_index11() {
+  int idx = -42;
+  auto x = glob_ptr8[idx]; // expected-warning{{garbage or undefined}}
+}
+
+void glob_invalid_index12() {
+  int idx = 42;
+  // FIXME: Should warn {{garbage or undefined}}
+  // We should take into account a declaration in which the literal is used.
+  auto x = glob_ptr8[idx]; // no-warning
+}
+
+const char16_t *const glob_ptr9 = u"абв";
+void glob_ptr_index5() {
+  clang_analyzer_eval(glob_ptr9[0] == u'а'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr9[1] == u'б'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr9[2] == u'в'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr9[3] == '\0'); // expected-warning{{TRUE}}
+}
+
+const char32_t *const glob_ptr10 = U"\U0001F607\U0001F608\U0001F609";
+void glob_ptr_index6() {
+  clang_analyzer_eval(glob_ptr10[0] == U'\U0001F607'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr10[1] == U'\U0001F608'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr10[2] == U'\U0001F609'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr10[3] == '\0');  // expected-warning{{TRUE}}
+}
+
+const wchar_t *const glob_ptr11 = L"\123\u0041\xFF";
+void glob_ptr_index7() {
+  clang_analyzer_eval(glob_ptr11[0] == L'\123');   // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr11[1] == L'\u0041'); // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr11[2] == L'\xFF');   // expected-warning{{TRUE}}
+  clang_analyzer_eval(glob_ptr11[3] == L'\0'); // expected-warning{{TRUE}}
+}
+
+const char *const glob_ptr12 = u8"a

[PATCH] D109128: [VFS] Use original path when falling back to external FS

[Keith Smiley via Phabricator via cfe-commits]

keith updated this revision to Diff 383406.
keith added a comment.

Remove broken test for now

Turns out this fails on main as well, so I'll punt this to another change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128

Files:
  clang/test/VFS/relative-path-errors.c
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp
  llvm/unittests/Support/VirtualFileSystemTest.cpp

Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -1627,6 +1627,114 @@
   EXPECT_EQ(0, NumDiagnostics);
 }
 
+TEST_F(VFSFromYAMLTest, ReturnsRequestedPathVFSMiss) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/a", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  ASSERT_FALSE(BaseFS->setCurrentWorkingDirectory("//root/foo"));
+  auto RemappedFS = vfs::RedirectingFileSystem::create(
+  {}, /*UseExternalNames=*/false, *BaseFS);
+
+  auto OpenedF = RemappedFS->openFileForRead("a");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("a", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("a", OpenedS->getName());
+  EXPECT_FALSE(OpenedS->IsVFSMapped);
+
+  auto DirectS = RemappedFS->status("a");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("a", DirectS->getName());
+  EXPECT_FALSE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsExternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': true,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("realname", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("realname", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  auto DirectS = FS->status("vfsname");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("realname", DirectS->getName());
+  EXPECT_TRUE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
+TEST_F(VFSFromYAMLTest, ReturnsInternalPathVFSHit) {
+  IntrusiveRefCntPtr BaseFS(
+  new vfs::InMemoryFileSystem);
+  BaseFS->addFile("//root/foo/realname", 0,
+  MemoryBuffer::getMemBuffer("contents of a"));
+  auto FS =
+  getFromYAMLString("{ 'use-external-names': false,\n"
+"  'roots': [\n"
+"{\n"
+"  'type': 'directory',\n"
+"  'name': '//root/foo',\n"
+"  'contents': [ {\n"
+"  'type': 'file',\n"
+"  'name': 'vfsname',\n"
+"  'external-contents': 'realname'\n"
+"}\n"
+"  ]\n"
+"}]}",
+BaseFS);
+  ASSERT_FALSE(FS->setCurrentWorkingDirectory("//root/foo"));
+
+  auto OpenedF = FS->openFileForRead("vfsname");
+  ASSERT_FALSE(OpenedF.getError());
+  llvm::ErrorOr Name = (*OpenedF)->getName();
+  ASSERT_FALSE(Name.getError());
+  EXPECT_EQ("vfsname", Name.get());
+
+  auto OpenedS = (*OpenedF)->status();
+  ASSERT_FALSE(OpenedS.getError());
+  EXPECT_EQ("vfsname", OpenedS->getName());
+  EXPECT_TRUE(OpenedS->IsVFSMapped);
+
+  auto DirectS = FS->status("vfsname");
+  ASSERT_FALSE(DirectS.getError());
+  EXPECT_EQ("vfsname", DirectS->getName());
+  EXPECT_TRUE(DirectS->IsVFSMapped);
+
+  EXPECT_EQ(0, NumDiagnostics);
+}
+
 TEST_F(VFSFromYAMLTest, CaseInsensitive) {
   IntrusiveRefCntPtr Lower(new DummyFileSystem());
   Lower->addRegularFile("//root/foo/bar/a");
Index: llvm/lib/Support/VirtualFileSystem.cp

[PATCH] D109128: [VFS] Use original path when falling back to external FS

[Keith Smiley via Phabricator via cfe-commits]

keith added a comment.

@dexonsmith turns out the test I was adding is broken on main today too. If 
it's alright with you I will punt that to another diff to not increase the 
scope of this one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109128

___
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

[Tom Stellard via Phabricator via cfe-commits]

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

@carlosgalvezp Did you write this patch or did you get it from someone else?


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] D112059: Fix inline builtin handling in case of redefinition

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers accepted this revision.
nickdesaulniers added a comment.

thanks again for all of the work that went into this!


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] D112730: [clang-tidy] Add AUTOSAR module

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

In D112730#3097091 , @tstellar wrote:

> @carlosgalvezp Did you write this patch or did you get it from someone else?

@tstellar I wrote it myself (following the existing pattern for the other 
modules in the llvm-project repo).

Just to clarify, I mentioned the above repo to point out about the license, but 
I don't plan


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

[clang] bd8a950 - [clang][driver] Fix multiarch output name with -Wl arg

[Keith Smiley via cfe-commits]

Author: Keith Smiley
Date: 2021-10-29T10:09:38-07:00
New Revision: bd8a9507ef8c3a493c15828f7f2fbc241a9e5c93

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

LOG: [clang][driver] Fix multiarch output name with -Wl arg

Previously if you passed a `-Wl,-foo` _before_ the source filename, the
first `InputInfos`, which is used for the base input name would be an
`InputArg` kind, which would never have a base input name. Now we use
that by default, but pick the first `InputInfo` that is of kind
`Filename` to get the name from if there is one.

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

Added: 


Modified: 
clang/lib/Driver/Driver.cpp
clang/test/Driver/darwin-dsymutil.c

Removed: 




diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index cd13a6d8b830..5b64ca8657d0 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4697,8 +4697,14 @@ InputInfo Driver::BuildJobsForActionNoCache(
 CachedResults, A->getOffloadingDeviceKind()));
   }
 
-  // Always use the first input as the base input.
+  // Always use the first file input as the base input.
   const char *BaseInput = InputInfos[0].getBaseInput();
+  for (auto &Info : InputInfos) {
+if (Info.isFilename()) {
+  BaseInput = Info.getBaseInput();
+  break;
+}
+  }
 
   // ... except dsymutil actions, which use their actual input as the base
   // input.

diff  --git a/clang/test/Driver/darwin-dsymutil.c 
b/clang/test/Driver/darwin-dsymutil.c
index 922b5d27f438..9f77e7237d02 100644
--- a/clang/test/Driver/darwin-dsymutil.c
+++ b/clang/test/Driver/darwin-dsymutil.c
@@ -51,6 +51,14 @@
 // CHECK-MULTIARCH-OUTPUT-NAME: "x86_64-apple-darwin10" - "darwin::Linker", 
inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.o"], output: 
"{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out"
 // CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin10" - "darwin::Linker", 
inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-arm64.o"], output: 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"
 // CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin10" - "darwin::Lipo", 
inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out", 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"], output: "a.out"
+//
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -Wl,-foo -arch x86_64 -arch arm64 -ccc-print-bindings %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG < %t %s
+//
+// CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG: "x86_64-apple-darwin10" - 
"darwin::Linker", inputs: [(input arg), 
"{{.*}}{{/|\\}}darwin-dsymutil-x86_64.o"], output: 
"{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out"
+// CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG: "arm64-apple-darwin10" - 
"darwin::Linker", inputs: [(input arg), 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.o"], output: 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"
+// CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG: "arm64-apple-darwin10" - 
"darwin::Lipo", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out", 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"], output: "a.out"
 
 // Check that we only use dsymutil when needed.
 //



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

[PATCH] D112767: [clang][driver] Fix multiarch output name with -Wl arg

[Keith Smiley via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGbd8a9507ef8c: [clang][driver] Fix multiarch output name with 
-Wl arg (authored by keith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112767

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/darwin-dsymutil.c


Index: clang/test/Driver/darwin-dsymutil.c
===
--- clang/test/Driver/darwin-dsymutil.c
+++ clang/test/Driver/darwin-dsymutil.c
@@ -51,6 +51,14 @@
 // CHECK-MULTIARCH-OUTPUT-NAME: "x86_64-apple-darwin10" - "darwin::Linker", 
inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.o"], output: 
"{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out"
 // CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin10" - "darwin::Linker", 
inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-arm64.o"], output: 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"
 // CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin10" - "darwin::Lipo", 
inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out", 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"], output: "a.out"
+//
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -Wl,-foo -arch x86_64 -arch arm64 -ccc-print-bindings %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG < %t %s
+//
+// CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG: "x86_64-apple-darwin10" - 
"darwin::Linker", inputs: [(input arg), 
"{{.*}}{{/|\\}}darwin-dsymutil-x86_64.o"], output: 
"{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out"
+// CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG: "arm64-apple-darwin10" - 
"darwin::Linker", inputs: [(input arg), 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.o"], output: 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"
+// CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG: "arm64-apple-darwin10" - 
"darwin::Lipo", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out", 
"{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"], output: "a.out"
 
 // Check that we only use dsymutil when needed.
 //
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4697,8 +4697,14 @@
 CachedResults, A->getOffloadingDeviceKind()));
   }
 
-  // Always use the first input as the base input.
+  // Always use the first file input as the base input.
   const char *BaseInput = InputInfos[0].getBaseInput();
+  for (auto &Info : InputInfos) {
+if (Info.isFilename()) {
+  BaseInput = Info.getBaseInput();
+  break;
+}
+  }
 
   // ... except dsymutil actions, which use their actual input as the base
   // input.


Index: clang/test/Driver/darwin-dsymutil.c
===
--- clang/test/Driver/darwin-dsymutil.c
+++ clang/test/Driver/darwin-dsymutil.c
@@ -51,6 +51,14 @@
 // CHECK-MULTIARCH-OUTPUT-NAME: "x86_64-apple-darwin10" - "darwin::Linker", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out"
 // CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin10" - "darwin::Linker", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-arm64.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"
 // CHECK-MULTIARCH-OUTPUT-NAME: "arm64-apple-darwin10" - "darwin::Lipo", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out", "{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"], output: "a.out"
+//
+// RUN: %clang -target x86_64-apple-darwin10 \
+// RUN:   -Wl,-foo -arch x86_64 -arch arm64 -ccc-print-bindings %s 2> %t
+// RUN: FileCheck --check-prefix=CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG < %t %s
+//
+// CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG: "x86_64-apple-darwin10" - "darwin::Linker", inputs: [(input arg), "{{.*}}{{/|\\}}darwin-dsymutil-x86_64.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out"
+// CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG: "arm64-apple-darwin10" - "darwin::Linker", inputs: [(input arg), "{{.*}}{{/|\\}}darwin-dsymutil-arm64.o"], output: "{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"
+// CHECK-MULTIARCH-OUTPUT-NAME-WITH-ARG: "arm64-apple-darwin10" - "darwin::Lipo", inputs: ["{{.*}}{{/|\\}}darwin-dsymutil-x86_64.out", "{{.*}}{{/|\\}}darwin-dsymutil-arm64.out"], output: "a.out"
 
 // Check that we only use dsymutil when needed.
 //
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4697,8 +4697,14 @@
 CachedResults, A->getOffloadingDeviceKind()));
   }
 
-  // Always use the first input as the base input.
+  // Always use the first file input as the base input.
   const char *BaseInput = InputInfos[0].getBaseInput();
+  for (auto &Info : InputInfos) {
+if (Info.isFilename()) {
+  BaseInput = Info.getBaseInput();
+  break;
+}
+  }
 
   // ... except dsymutil actions, which use their actual input as the base
   // input.
___
cfe-commits mailing l

[clang] c001775 - [clang] Inclusive language: change error message to use allowlist

[Zarko Todorovski via cfe-commits]

Author: Zarko Todorovski
Date: 2021-10-29T13:12:46-04:00
New Revision: c001775a3afb124b9b19ff5a45a68d4baf130077

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

LOG: [clang] Inclusive language: change error message to use allowlist

Reviewed By: vitalybuka

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

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticDriverKinds.td
clang/lib/Driver/SanitizerArgs.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index e013cf25db809..b823ade0eafb5 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -187,8 +187,8 @@ def err_drv_invalid_argument_to_option : Error<
   "invalid argument '%0' to -%1">;
 def err_drv_malformed_sanitizer_ignorelist : Error<
   "malformed sanitizer ignorelist: '%0'">;
-def err_drv_malformed_sanitizer_coverage_whitelist : Error<
-  "malformed sanitizer coverage whitelist: '%0'">;
+def err_drv_malformed_sanitizer_coverage_allowlist : Error<
+  "malformed sanitizer coverage allowlist: '%0'">;
 def err_drv_malformed_sanitizer_coverage_ignorelist : Error<
   "malformed sanitizer coverage ignorelist: '%0'">;
 def err_drv_duplicate_config : Error<

diff  --git a/clang/lib/Driver/SanitizerArgs.cpp 
b/clang/lib/Driver/SanitizerArgs.cpp
index 44692e131d3e1..6f426c6cad69c 100644
--- a/clang/lib/Driver/SanitizerArgs.cpp
+++ b/clang/lib/Driver/SanitizerArgs.cpp
@@ -756,7 +756,7 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC,
 parseSpecialCaseListArg(
 D, Args, CoverageAllowlistFiles,
 options::OPT_fsanitize_coverage_allowlist, OptSpecifier(),
-clang::diag::err_drv_malformed_sanitizer_coverage_whitelist);
+clang::diag::err_drv_malformed_sanitizer_coverage_allowlist);
 parseSpecialCaseListArg(
 D, Args, CoverageIgnorelistFiles,
 options::OPT_fsanitize_coverage_ignorelist, OptSpecifier(),



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

[PATCH] D112627: [clang] Inclusive language: change error message to use allowlist

[Zarko Todorovski 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 rGc001775a3afb: [clang] Inclusive language: change error 
message to use allowlist (authored by ZarkoCA).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112627

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/lib/Driver/SanitizerArgs.cpp


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -756,7 +756,7 @@
 parseSpecialCaseListArg(
 D, Args, CoverageAllowlistFiles,
 options::OPT_fsanitize_coverage_allowlist, OptSpecifier(),
-clang::diag::err_drv_malformed_sanitizer_coverage_whitelist);
+clang::diag::err_drv_malformed_sanitizer_coverage_allowlist);
 parseSpecialCaseListArg(
 D, Args, CoverageIgnorelistFiles,
 options::OPT_fsanitize_coverage_ignorelist, OptSpecifier(),
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -187,8 +187,8 @@
   "invalid argument '%0' to -%1">;
 def err_drv_malformed_sanitizer_ignorelist : Error<
   "malformed sanitizer ignorelist: '%0'">;
-def err_drv_malformed_sanitizer_coverage_whitelist : Error<
-  "malformed sanitizer coverage whitelist: '%0'">;
+def err_drv_malformed_sanitizer_coverage_allowlist : Error<
+  "malformed sanitizer coverage allowlist: '%0'">;
 def err_drv_malformed_sanitizer_coverage_ignorelist : Error<
   "malformed sanitizer coverage ignorelist: '%0'">;
 def err_drv_duplicate_config : Error<


Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -756,7 +756,7 @@
 parseSpecialCaseListArg(
 D, Args, CoverageAllowlistFiles,
 options::OPT_fsanitize_coverage_allowlist, OptSpecifier(),
-clang::diag::err_drv_malformed_sanitizer_coverage_whitelist);
+clang::diag::err_drv_malformed_sanitizer_coverage_allowlist);
 parseSpecialCaseListArg(
 D, Args, CoverageIgnorelistFiles,
 options::OPT_fsanitize_coverage_ignorelist, OptSpecifier(),
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -187,8 +187,8 @@
   "invalid argument '%0' to -%1">;
 def err_drv_malformed_sanitizer_ignorelist : Error<
   "malformed sanitizer ignorelist: '%0'">;
-def err_drv_malformed_sanitizer_coverage_whitelist : Error<
-  "malformed sanitizer coverage whitelist: '%0'">;
+def err_drv_malformed_sanitizer_coverage_allowlist : Error<
+  "malformed sanitizer coverage allowlist: '%0'">;
 def err_drv_malformed_sanitizer_coverage_ignorelist : Error<
   "malformed sanitizer coverage ignorelist: '%0'">;
 def err_drv_duplicate_config : Error<
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112835: [clangd] Record time spent in tidy checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kadircet 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/D112835

Files:
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -21,6 +21,7 @@
 #include "index/MemIndex.h"
 #include "support/Context.h"
 #include "support/Path.h"
+#include "support/TestTracer.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticSema.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -305,6 +306,7 @@
"modernize-use-trailing-return-type,"
"misc-no-recursion");
   TU.ExtraArgs.push_back("-Wno-unsequenced");
+  trace::TestTracer Tracer;
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
   ifTidyChecks(UnorderedElementsAre(
@@ -336,6 +338,11 @@
"function 'foo' is within a recursive call chain"),
   Diag(Test.range("bar"),
"function 'bar' is within a recursive call chain";
+  if (CLANGD_TIDY_CHECKS) {
+EXPECT_THAT(Tracer.takeMetric("tidy_check_latency",
+  "modernize-use-trailing-return-type"),
+Not(IsEmpty()));
+  }
 }
 
 TEST(DiagnosticsTest, ClangTidyEOF) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -29,6 +29,7 @@
 #include "support/Trace.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
@@ -54,7 +55,9 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 #include 
@@ -341,7 +344,10 @@
   //ancestors outside this scope).
   // In practice almost all checks work well without modifications.
   std::vector> CTChecks;
-  ast_matchers::MatchFinder CTFinder;
+  llvm::StringMap CTCheckTimings;
+  ast_matchers::MatchFinder::MatchFinderOptions CTFinderOpts;
+  CTFinderOpts.CheckProfiling.emplace(CTCheckTimings);
+  ast_matchers::MatchFinder CTFinder(CTFinderOpts);
   llvm::Optional CTContext;
   llvm::Optional FixIncludes;
   // No need to run clang-tidy or IncludeFixerif we are not going to surface
@@ -492,6 +498,10 @@
 // (The preprocessor part ran already, via PPCallbacks).
 trace::Span Tracer("ClangTidyMatch");
 CTFinder.matchAST(Clang->getASTContext());
+static constexpr trace::Metric TidyCheckLatencies(
+"tidy_check_latency", trace::Metric::Distribution, "check_name");
+for (auto &Elem : CTCheckTimings)
+  TidyCheckLatencies.record(Elem.second.getWallTime(), Elem.first());
   }
 
   // XXX: This is messy: clang-tidy checks flush some diagnostics at EOF.


Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -21,6 +21,7 @@
 #include "index/MemIndex.h"
 #include "support/Context.h"
 #include "support/Path.h"
+#include "support/TestTracer.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticSema.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -305,6 +306,7 @@
"modernize-use-trailing-return-type,"
"misc-no-recursion");
   TU.ExtraArgs.push_back("-Wno-unsequenced");
+  trace::TestTracer Tracer;
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
   ifTidyChecks(UnorderedElementsAre(
@@ -336,6 +338,11 @@
"function 'foo' is within a recursive call chain"),
   Diag(Test.range("bar"),
"function 'bar' is within a recursive call chain";
+  if (CLANGD_TIDY_CHECKS) {
+EXPECT_THAT(Tracer.takeMetric("tidy_check_latency",
+  "modernize-use-trailing-return-type"),
+Not(IsEmpty()));
+  }
 }
 
 TEST(DiagnosticsTest, ClangTidyEOF) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@

[PATCH] D109950: [Clang] Enable IC/IF mode for __ibm128

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Oh, yes, I think this should be preserving the old logic there and just adding 
a new clause for explicit requests for ibm128, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109950

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

[PATCH] D112835: [clangd] Record time spent in tidy checks

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

My only concern with this is the overhead of the timing itself.
We look up the hash bucket, turn timers on/off...

If I'm understanding right we do this once per AST node per registered matcher. 
Are you able to see any performance difference with this tracing on vs off?

If there's a performance penalty but our goal is to gather enough statistics to 
be useful, we could consider only doing it every N reparses (or just with a 
certain probability).
But even then, if it's slow maybe we should only be running it in cases where 
we know we're sending the stats somewhere useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112835

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

[PATCH] D111477: DO NOT SUBMIT: workaround for context-sensitive use of non-type-template-parameter integer suffixes

[Adrian Prantl via Phabricator via cfe-commits]

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

I think this is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111477

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

[PATCH] D8467: C++14: Disable sized deallocation by default due to ABI breakage

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D8467#3095610 , @zixuan-wu wrote:

> Hi, I am wondering could -fsized-deallocation this be enabled by default 
> nowadays in 2021?

Yes, I think so. This should be a matter of adjusting the default in the 
driver. We should not bring back the code that defines the sized deallocation 
thunk, so a straight revert of this patch doesn't make sense.

I don't have time to work on this at the moment, but I would approve a patch.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D8467

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

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

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/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] D109402: Make 'align-mismatch' warning work without an associated function declaration

[Ravi via Phabricator via cfe-commits]

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

Looks good to me! Thanks!


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

https://reviews.llvm.org/D109402

___
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

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

dexonsmith updated this revision to Diff 383446.
dexonsmith edited the summary of this revision.
dexonsmith added a comment.

Squashed in the changes from https://reviews.llvm.org/D112786.


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

https://reviews.llvm.org/D112289

Files:
  clang/include/clang/Basic/JsonSupport.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/unittests/Basic/FileManagerTest.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp
  clang/unittests/Tooling/RefactoringTest.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/GraphWriter.cpp
  llvm/lib/Support/Path.cpp
  llvm/tools/lli/lli.cpp

Index: llvm/tools/lli/lli.cpp
===
--- llvm/tools/lli/lli.cpp
+++ llvm/tools/lli/lli.cpp
@@ -355,13 +355,12 @@
   return false;
 
 std::string CacheSubdir = ModID.substr(PrefixLength);
-#if defined(_WIN32)
-// Transform "X:\foo" => "/X\foo" for convenience.
-if (isalpha(CacheSubdir[0]) && CacheSubdir[1] == ':') {
+// Transform "X:\foo" => "/X\foo" for convenience on Windows.
+if (is_style_windows(llvm::sys::path::Style::native) &&
+isalpha(CacheSubdir[0]) && CacheSubdir[1] == ':') {
   CacheSubdir[1] = CacheSubdir[0];
   CacheSubdir[0] = '/';
 }
-#endif
 
 CacheName = CacheDir + CacheSubdir;
 size_t pos = CacheName.rfind('.');
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -64,7 +64,7 @@
 if (path.empty())
   return path;
 
-if (real_style(style) == Style::windows) {
+if (is_style_windows(style)) {
   // C:
   if (path.size() >= 2 &&
   std::isalpha(static_cast(path[0])) && path[1] == ':')
@@ -96,7 +96,7 @@
 
 size_t pos = str.find_last_of(separators(style), str.size() - 1);
 
-if (real_style(style) == Style::windows) {
+if (is_style_windows(style)) {
   if (pos == StringRef::npos)
 pos = str.find_last_of(':', str.size() - 2);
 }
@@ -111,7 +111,7 @@
   // directory in str, it returns StringRef::npos.
   size_t root_dir_start(StringRef str, Style style) {
 // case "c:/"
-if (real_style(style) == Style::windows) {
+if (is_style_windows(style)) {
   if (str.size() > 2 && str[1] == ':' && is_separator(str[2], style))
 return 2;
 }
@@ -257,7 +257,7 @@
 // Root dir.
 if (was_net ||
 // c:/
-(real_style(S) == Style::windows && Component.endswith(":"))) {
+(is_style_windows(S) && Component.endswith(":"))) {
   Component = Path.substr(Position, 1);
   return *this;
 }
@@ -346,7 +346,7 @@
   if (b != e) {
 bool has_net =
 b->size() > 2 && is_separator((*b)[0], style) && (*b)[1] == (*b)[0];
-bool has_drive = (real_style(style) == Style::windows) && b->endswith(":");
+bool has_drive = is_style_windows(style) && b->endswith(":");
 
 if (has_net || has_drive) {
   if ((++pos != e) && is_separator((*pos)[0], style)) {
@@ -371,7 +371,7 @@
   if (b != e) {
 bool has_net =
 b->size() > 2 && is_separator((*b)[0], style) && (*b)[1] == (*b)[0];
-bool has_drive = (real_style(style) == Style::windows) && b->endswith(":");
+bool has_drive = is_style_windows(style) && b->endswith(":");
 
 if (has_net || has_drive) {
   // just {C:,//net}, return the first component.
@@ -388,7 +388,7 @@
   if (b != e) {
 bool has_net =
 b->size() > 2 && is_separator((*b)[0], style) && (*b)[1] == (*b)[0];
-bool has_drive = (real_style(style) == Style::windows) && b->endswith(":");
+bool has_drive = is_style_windows(style) && b->endswith(":");
 
 if ((has_net || has_drive) &&
 // {C:,//net}, skip to the next component.
@@ -495,7 +495,7 @@
 static bool starts_with(StringRef Path, StringRef Prefix,
 Style style = Style::native) {
   // Windows prefix matching : case and separator insensitive
-  if (real_style(style) == Style::windows) {
+  if (is_style_windows(style)) {
 if (Path.size() < Prefix.size())
   return false;
 for (size_t I = 0, E = Prefix.size(); I != E; ++I) {
@@ -546,7 +546,7 @@
 void native(SmallVectorImpl &Path, Style style) {
   if (Path.empty())
 return;
-  if (real_style(style) == Style::windows) {
+  if (is_style_windows(style)) {
 std::replace(Path.begin(), Path.end(), '/', '\\');
 if (Path[0] == '~' && (Path.size() == 1 || is_separator(Path[1], style))) {
   SmallString<128> PathHome;
@@ -560,7 +560,7 @@
 }
 
 std::string convert_to_slash(StringRef path, Style style) {
-  if (real_style(style) != Style::windows)
+  if (is_style_posix(style))
 return std::string(path);
 
   std::string s = path.str();
@@ -595,13 +595,13 @@
 bool is_separator(char value, Style style

[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for this! I have some concerns that I'd like to talk out to hopefully 
make myself feel more comfortable with this.

My understanding of the utility from -Wformat warnings comes from the fact that 
the function receiving the format string has no way to validate that the 
variadic arguments passed have the correct types when compared to the format 
string. However, in both of the new cases you propose to support, that type 
information is present in the function signature (you may have to go through a 
bunch of template instantiations in the variadic template cases, but you get 
there eventually). Having the types in the signatures changes something I think 
might be pretty fundamental to the way the format string checker works -- it 
tries to figure out types *after default argument promotions* because those 
promotions are a lossy operation. However, when the types are specified in the 
signature, those default argument promotions no longer happen. The type passed 
for a `%f` may actually be a `float` rather than promoted to a `double`, the 
type for a `char` may actually be `char` rather than `int`, etc. I'm not 
convinced this is as simple as "now we allow non-vararg functions" for the 
implementation. I think we need some more extensive testing to prove that the 
diagnostics actually make sense or not.

In terms of the specific cases allowed, I think I am happier about variadic 
templates than I am about fixed-signature functions. For the variadic template, 
I think supporting -Wformat would mean users don't have to reimplement similar 
logic to what that check already handles even though they can do it themselves. 
But for a fixed-signature function, I'm not certain I see what the value add is 
-- the only valid format strings such a function could accept would have to be 
fixed to the function signature anyway. Can you explain what use cases you have 
for that situation?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112579

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

[PATCH] D112765: [AST] injected-class-name is not a redecl, even in template specializations

[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.

Aside from the cast request, this LGTM, thank you!




Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1841-1842
+   /*DelayTypeCreation=*/IsInjectedClassName);
+  if (IsInjectedClassName)
+SemaRef.Context.getTypeDeclType(Record, cast(Owner));
 

sammccall wrote:
> hokein wrote:
> > aaron.ballman wrote:
> > > Why is this call needed? (It seems strange to me that we call it and 
> > > ignore the return value.)
> > my understanding is
> >   - `getTypeDeclType` is more than a getter, it also has a side-effort -- 
> > if the type of the passed `Record` is empty, it creates a type, and 
> > propagates the type to `Record->TypeForDecl`;
> >   - from the above line, we delay the type creation when 
> > `IsInjectedClassName` is true;
> >   - so we need to create a type for the `Record` by invoking 
> > `getTypeDeclType`;
> > 
> > might be worth a comment.
> > 
> That's exactly right. The injected-class-name is an independent decl but 
> yields the same RecordType, and getTypeDeclType has a hacky interface so we 
> can force this link.
> (It wasn't needed in the old version of the code, because when it was a 
> redecl getTypeDeclType would yield the same type in any case)
> 
> I should have mentioned this is all derived from the injected-class-name 
> handling in Sema::ActOnStartCXXMemberDeclarations(). It lacks comments, but 
> I'll add a few while here.
Ah, thank you for the explanation! I think a `(void)` on the result will also 
help make it clear that we intentionally are ignoring that result despite it 
looking like a getter.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112765

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

[PATCH] D112101: [AST] Fix the EndLoc calculation for ObjCObjectPointer

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

I think this is still not quite right: if the `ObjCObjectPointerType` has a 
valid star location, we should use that. We should only be falling back on the 
inner location's end when there is no star. For example:

  @interface X @end;
  using ObjectPointer = X*;

... should have an end loc that points at the `*`, not the `X`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112101

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

[clang] 9902362 - Support: Use sys::path::is_style_{posix, windows}() in a few places

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

Author: Duncan P. N. Exon Smith
Date: 2021-10-29T12:09:41-07:00
New Revision: 99023627010bbfefb71e25a2b4d056de1cbd354e

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

LOG: Support: Use sys::path::is_style_{posix,windows}() in a few places

Use the new sys::path::is_style_posix() and is_style_windows() in a few
places that need to detect the system's native path style.

In llvm/lib/Support/Path.cpp, this patch removes most uses of the
private `real_style()`, where is_style_posix() and is_style_windows()
are just a little tidier.

Elsewhere, this removes `_WIN32` macro checks. Added a FIXME to a
FileManagerTest that seemed fishy, but maintained the existing
behaviour.

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

Added: 


Modified: 
clang/include/clang/Basic/JsonSupport.h
clang/lib/Basic/FileManager.cpp
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChain.cpp
clang/lib/Lex/PPDirectives.cpp
clang/unittests/Basic/FileManagerTest.cpp
clang/unittests/Driver/ToolChainTest.cpp
clang/unittests/Lex/HeaderSearchTest.cpp
clang/unittests/Tooling/RefactoringTest.cpp
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/GraphWriter.cpp
llvm/lib/Support/Path.cpp
llvm/tools/lli/lli.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/JsonSupport.h 
b/clang/include/clang/Basic/JsonSupport.h
index 6cd3f4d57b846..2ccb08e4bdaa2 100644
--- a/clang/include/clang/Basic/JsonSupport.h
+++ b/clang/include/clang/Basic/JsonSupport.h
@@ -12,6 +12,7 @@
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/SourceManager.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
 
@@ -98,18 +99,19 @@ inline void printSourceLocationAsJson(raw_ostream &Out, 
SourceLocation Loc,
 if (AddBraces)
   Out << "{ ";
 std::string filename(PLoc.getFilename());
-#ifdef _WIN32
-// Remove forbidden Windows path characters
-auto RemoveIt =
-std::remove_if(filename.begin(), filename.end(), [](auto Char) {
-  static const char ForbiddenChars[] = "<>*?\"|";
-  return std::find(std::begin(ForbiddenChars), 
std::end(ForbiddenChars),
-   Char) != std::end(ForbiddenChars);
-});
-filename.erase(RemoveIt, filename.end());
-// Handle windows-specific path delimiters.
-std::replace(filename.begin(), filename.end(), '\\', '/');
-#endif
+if (is_style_windows(llvm::sys::path::Style::native)) {
+  // Remove forbidden Windows path characters
+  auto RemoveIt =
+  std::remove_if(filename.begin(), filename.end(), [](auto Char) {
+static const char ForbiddenChars[] = "<>*?\"|";
+return std::find(std::begin(ForbiddenChars),
+ std::end(ForbiddenChars),
+ Char) != std::end(ForbiddenChars);
+  });
+  filename.erase(RemoveIt, filename.end());
+  // Handle windows-specific path delimiters.
+  std::replace(filename.begin(), filename.end(), '\\', '/');
+}
 Out << "\"line\": " << PLoc.getLine()
 << ", \"column\": " << PLoc.getColumn()
 << ", \"file\": \"" << filename << "\"";

diff  --git a/clang/lib/Basic/FileManager.cpp b/clang/lib/Basic/FileManager.cpp
index c4eae6acd7b04..1cb52d5594d59 100644
--- a/clang/lib/Basic/FileManager.cpp
+++ b/clang/lib/Basic/FileManager.cpp
@@ -123,16 +123,16 @@ FileManager::getDirectoryRef(StringRef DirName, bool 
CacheFailure) {
   DirName != llvm::sys::path::root_path(DirName) &&
   llvm::sys::path::is_separator(DirName.back()))
 DirName = DirName.substr(0, DirName.size()-1);
-#ifdef _WIN32
-  // Fixing a problem with "clang C:test.c" on Windows.
-  // Stat("C:") does not recognize "C:" as a valid directory
-  std::string DirNameStr;
-  if (DirName.size() > 1 && DirName.back() == ':' &&
-  DirName.equals_insensitive(llvm::sys::path::root_name(DirName))) {
-DirNameStr = DirName.str() + '.';
-DirName = DirNameStr;
+  if (is_style_windows(llvm::sys::path::Style::native)) {
+// Fixing a problem with "clang C:test.c" on Windows.
+// Stat("C:") does not recognize "C:" as a valid directory
+std::string DirNameStr;
+if (DirName.size() > 1 && DirName.back() == ':' &&
+DirName.equals_insensitive(llvm::sys::path::root_name(DirName))) {
+  DirNameStr = DirName.str() + '.';
+  DirName = DirNameStr;
+}
   }
-#endif
 
   ++NumDirLookups;
 

diff  --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp
index 5b64ca8657d0e..2ddb753660e40 100644
--- a/clang/lib/Driver/Driver.cpp
+++ b/clang/lib/Driver/Driver.cpp
@@ -4899,11 +4899,11 @@ const char *Driver::GetNamedOutputPath(Compilation &C, 
const JobActi

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

[Duncan P. N. Exon Smith 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 rG99023627010b: Support: Use 
sys::path::is_style_{posix,windows}() in a few places (authored by dexonsmith).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112289

Files:
  clang/include/clang/Basic/JsonSupport.h
  clang/lib/Basic/FileManager.cpp
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChain.cpp
  clang/lib/Lex/PPDirectives.cpp
  clang/unittests/Basic/FileManagerTest.cpp
  clang/unittests/Driver/ToolChainTest.cpp
  clang/unittests/Lex/HeaderSearchTest.cpp
  clang/unittests/Tooling/RefactoringTest.cpp
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/GraphWriter.cpp
  llvm/lib/Support/Path.cpp
  llvm/tools/lli/lli.cpp

Index: llvm/tools/lli/lli.cpp
===
--- llvm/tools/lli/lli.cpp
+++ llvm/tools/lli/lli.cpp
@@ -355,13 +355,12 @@
   return false;
 
 std::string CacheSubdir = ModID.substr(PrefixLength);
-#if defined(_WIN32)
-// Transform "X:\foo" => "/X\foo" for convenience.
-if (isalpha(CacheSubdir[0]) && CacheSubdir[1] == ':') {
+// Transform "X:\foo" => "/X\foo" for convenience on Windows.
+if (is_style_windows(llvm::sys::path::Style::native) &&
+isalpha(CacheSubdir[0]) && CacheSubdir[1] == ':') {
   CacheSubdir[1] = CacheSubdir[0];
   CacheSubdir[0] = '/';
 }
-#endif
 
 CacheName = CacheDir + CacheSubdir;
 size_t pos = CacheName.rfind('.');
Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -64,7 +64,7 @@
 if (path.empty())
   return path;
 
-if (real_style(style) == Style::windows) {
+if (is_style_windows(style)) {
   // C:
   if (path.size() >= 2 &&
   std::isalpha(static_cast(path[0])) && path[1] == ':')
@@ -96,7 +96,7 @@
 
 size_t pos = str.find_last_of(separators(style), str.size() - 1);
 
-if (real_style(style) == Style::windows) {
+if (is_style_windows(style)) {
   if (pos == StringRef::npos)
 pos = str.find_last_of(':', str.size() - 2);
 }
@@ -111,7 +111,7 @@
   // directory in str, it returns StringRef::npos.
   size_t root_dir_start(StringRef str, Style style) {
 // case "c:/"
-if (real_style(style) == Style::windows) {
+if (is_style_windows(style)) {
   if (str.size() > 2 && str[1] == ':' && is_separator(str[2], style))
 return 2;
 }
@@ -257,7 +257,7 @@
 // Root dir.
 if (was_net ||
 // c:/
-(real_style(S) == Style::windows && Component.endswith(":"))) {
+(is_style_windows(S) && Component.endswith(":"))) {
   Component = Path.substr(Position, 1);
   return *this;
 }
@@ -346,7 +346,7 @@
   if (b != e) {
 bool has_net =
 b->size() > 2 && is_separator((*b)[0], style) && (*b)[1] == (*b)[0];
-bool has_drive = (real_style(style) == Style::windows) && b->endswith(":");
+bool has_drive = is_style_windows(style) && b->endswith(":");
 
 if (has_net || has_drive) {
   if ((++pos != e) && is_separator((*pos)[0], style)) {
@@ -371,7 +371,7 @@
   if (b != e) {
 bool has_net =
 b->size() > 2 && is_separator((*b)[0], style) && (*b)[1] == (*b)[0];
-bool has_drive = (real_style(style) == Style::windows) && b->endswith(":");
+bool has_drive = is_style_windows(style) && b->endswith(":");
 
 if (has_net || has_drive) {
   // just {C:,//net}, return the first component.
@@ -388,7 +388,7 @@
   if (b != e) {
 bool has_net =
 b->size() > 2 && is_separator((*b)[0], style) && (*b)[1] == (*b)[0];
-bool has_drive = (real_style(style) == Style::windows) && b->endswith(":");
+bool has_drive = is_style_windows(style) && b->endswith(":");
 
 if ((has_net || has_drive) &&
 // {C:,//net}, skip to the next component.
@@ -495,7 +495,7 @@
 static bool starts_with(StringRef Path, StringRef Prefix,
 Style style = Style::native) {
   // Windows prefix matching : case and separator insensitive
-  if (real_style(style) == Style::windows) {
+  if (is_style_windows(style)) {
 if (Path.size() < Prefix.size())
   return false;
 for (size_t I = 0, E = Prefix.size(); I != E; ++I) {
@@ -546,7 +546,7 @@
 void native(SmallVectorImpl &Path, Style style) {
   if (Path.empty())
 return;
-  if (real_style(style) == Style::windows) {
+  if (is_style_windows(style)) {
 std::replace(Path.begin(), Path.end(), '/', '\\');
 if (Path[0] == '~' && (Path.size() == 1 || is_separator(Path[1], style))) {
   SmallString<128> PathHome;
@@ -560,7 +560,7 @@
 }
 
 std::string convert_to_slash(StringRef path, Style style) {
-  if (real_style(style) != Style::windows)
+  if (is_style_posix(style))
 return std::strin

[PATCH] D112765: [AST] injected-class-name is not a redecl, even in template specializations

[Haojian Wu via Phabricator via cfe-commits]

hokein accepted this revision.
hokein added a comment.

looks good, thanks.




Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1685
-  // We don't want to visit injected-class-names in this traversal.
-  if (cast(RD)->isInjectedClassName())
-continue;

I would add a  assertion here: 
`assert(!cast(RD)->isInjectedClassName())`.



Comment at: clang/lib/AST/ASTDumper.cpp:94
 auto *Redecl = dyn_cast(RedeclWithBadType);
-if (!Redecl) {
-  // Found the injected-class-name for a class template. This will be 
dumped

`assert(Redecl)`



Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:1910
 
+  if (IsInjectedClassName)
+assert(Record->isInjectedClassName() && "Broken injected-class-name");

sammccall wrote:
> hokein wrote:
> > it is unclear to me what's the intention of the assertion.
> We needed to do a few weird things (and to assume some things about the 
> input) to initialize the injected-class-name, it verifies we got them all 
> right.
> 
> (This assertion is also in ActOnStartCXXMemberDeclarations())
ok, I see. didn't know this is derived from `ActOnStartCXXMemberDeclarations`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112765

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

[PATCH] D112491: Add `LambdaCapture`-related matchers.

[James King via Phabricator via cfe-commits]

jcking1034 updated this revision to Diff 383458.
jcking1034 marked 2 inline comments as done.
jcking1034 added a comment.

Update matcher names for clarity.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112491

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/AST/ASTTypeTraits.h
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/AST/ASTTypeTraits.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
  clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2237,6 +2237,66 @@
  varDecl(hasName("ss"), hasTypeLoc(elaboratedTypeLoc();
 }
 
+TEST_P(ASTMatchersTest, LambdaCaptureTest) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture();
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(matches("int main() { int cc; auto f = [&cc](){ return cc; }; }",
+  matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [=](){ return cc; }; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc; auto f = [&](){ return cc; }; }", matcher));
+}
+
+TEST_P(ASTMatchersTest, LambdaCaptureTest_BindsToCaptureWithInitializer) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(
+  varDecl(hasName("cc"), hasInitializer(integerLiteral(equals(1;
+  EXPECT_TRUE(
+  matches("int main() { auto lambda = [cc = 1] {return cc;}; }", matcher));
+  EXPECT_TRUE(
+  matches("int main() { int cc = 2; auto lambda = [cc = 1] {return cc;}; }",
+  matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureReferringToVarDecl) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(
+  hasAnyCapture(lambdaCapture(capturesVar(varDecl(hasName("cc"));
+  EXPECT_FALSE(matches("int main() { auto f = [](){ return 5; }; }", matcher));
+  EXPECT_FALSE(matches("int main() { int xx; auto f = [xx](){ return xx; }; }",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest,
+   LambdaCaptureTest_DoesNotBindToCaptureWithInitializerAndDifferentName) {
+  if (!GetParam().isCXX14OrLater()) {
+return;
+  }
+  EXPECT_FALSE(matches(
+  "int main() { auto lambda = [xx = 1] {return xx;}; }",
+  lambdaExpr(hasAnyCapture(lambdaCapture(capturesVar(varDecl(
+  hasName("cc"), hasInitializer(integerLiteral(equals(1));
+}
+
 TEST(ASTMatchersTestObjC, ObjCMessageExpr) {
   // Don't find ObjCMessageExpr where none are present.
   EXPECT_TRUE(notMatchesObjC("", objcMessageExpr(anything(;
Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -4445,5 +4445,42 @@
   cxxRecordDecl(hasName("Derived"),
 hasDirectBase(hasType(cxxRecordDecl(hasName("Base")));
 }
+
+TEST_P(ASTMatchersTest, RefersToThis) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(lambdaCapture(capturesThis(;
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [this](){ return "
+  "cc; }; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [=](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_TRUE(matches("class C { int cc; int f() { auto l = [&](){ return cc; "
+  "}; return l(); } };",
+  matcher));
+  EXPECT_FALSE(matches("class C { int cc; int f() { auto l = [cc](){ return "
+   "cc; }; return l(); } };",
+   matcher));
+  EXPECT_FALSE(matches("class C { int this; int f() { auto l = [this](){ "
+   "return this; }; return l(); } };",
+   matcher));
+}
+
+TEST_P(ASTMatchersTest, IsImplicit_LambdaCapture) {
+  if (!GetParam().isCXX11OrLater()) {
+return;
+  }
+  auto matcher = lambdaExpr(hasAnyCapture(
+  l