[PATCH] D141280: [clang][wip] Build UsingType for elaborated type specifiers.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

though this is a hack implementation, the result seems promising to me, so I 
post it for initial thoughts.




Comment at: clang/include/clang/Sema/Sema.h:3307
 
   Decl *ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
  SourceLocation KWLoc, CXXScopeSpec &SS, IdentifierInfo *Name,

This is the key place where it returns the underlying decl.

Alternatively, we could return the UsingDecl as the result (there are only 4 
places using it, might be possible to check/verify/adjust all caller place, I 
feel a bit scary about it, as the name indicates, caller might assume the 
return Decl is a `TagDecl`, e.g. the 
[case](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaTemplate.cpp#L10195))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141280

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

[PATCH] D140462: [clangd] Add schema for `.clangd` config

[Edwin Kofler via Phabricator via cfe-commits]

hyperupcall updated this revision to Diff 487706.
hyperupcall added a comment.

Improve schema correctness and disable `additionalProperties`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140462

Files:
  clang-tools-extra/clangd/schema/config.json

Index: clang-tools-extra/clangd/schema/config.json
===
--- /dev/null
+++ clang-tools-extra/clangd/schema/config.json
@@ -0,0 +1,272 @@
+{
+  "$schema": "http://json-schema.org/draft-07/schema";,
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"If": {
+  "description": "Conditions in the If block restrict when a fragment applies.",
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"PathMatch": {
+  "description": "The file being processed must fully match a regular expression.",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+},
+"PathExclude": {
+  "description": "The file being processed must not fully match a regular expression.",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+}
+  }
+},
+"CompileFlags": {
+  "description": "Affects how a source file is parsed.",
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"Add": {
+  "description": "List of flags to append to the compile command.",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+},
+"Remove": {
+  "description": "List of flags to remove from the compile command",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  ]
+},
+"CompilationDatabase": {
+  "description": "Directory to search for compilation database (compile_commands.json etc).",
+  "oneOf": [
+{
+  "type": "string"
+},
+{
+  "enum": [
+"Ancestors",
+"None"
+  ]
+}
+  ]
+},
+"Compiler": {
+  "description": "String to replace the executable name in the compile flags. The name controls flag parsing (clang vs clang-cl), target inference (gcc-arm-noneabi) etc.",
+  "type": "string"
+}
+  }
+},
+"Index": {
+  "description": "Controls how clangd understands code outside the current file.",
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"Background": {
+  "description": "Whether files are built in the background to produce a project index. This is checked for translation units only, not headers they include.",
+  "type": "string",
+  "enum": [
+"Build",
+"Skip"
+  ],
+  "default": "Build"
+},
+"External": {
+  "description": "Used to define an external index source",
+  "oneOf": [
+{
+  "type": "string",
+  "pattern": "[nN][oO][nN][eE]"
+},
+{
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"File": {
+  "type": "string"
+}
+  },
+  "required": [
+"File"
+  ]
+},
+{
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"Server": {
+  "type": "string"
+},
+"MountPoint": {
+  "type": "string"
+}
+  },
+  "required": [
+"Server"
+  ]
+}
+  ]
+}
+  }
+},
+"Style": {
+  "description": "Describes the style of the codebase, beyond formatting",
+  "type": "object",
+  "additionalProperties": false,
+  "properties": {
+"FullyQualifiedNamespaces": {
+  "type": "array",
+  "items": {
+"type": "string"
+  }
+}
+  }
+},
+"Diagnostics": {
+  "type": "obj

[PATCH] D140462: [clangd] Add schema for `.clangd` config

[Edwin Kofler via Phabricator via cfe-commits]

hyperupcall abandoned this revision.
hyperupcall added a comment.

I think I abandon  
this revision for the reasons stated above - there is an improved path forward 
at https://reviews.llvm.org/D140745 with autogenerated files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140462

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

[PATCH] D141192: [Clang] Fix warnings on bad shifts.

[Dmitriy Chestnykh via Phabricator via cfe-commits]

chestnykh added a comment.

Ping, can someone review my changes?


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

https://reviews.llvm.org/D141192

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

[PATCH] D141280: [clang][wip] Build UsingType for elaborated type specifiers.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I'll bite, why do you say it's hacky?
Looks OK to me, by clang standards :-)




Comment at: clang/include/clang/Sema/DeclSpec.h:516
+  }
+  Decl *getRepAsOriginDecl() const {
+assert(isDeclRep((TST)TypeSpecType) && "DeclSpec does not store a decl");

elsewhere we call this the **found** decl, seems worth sticking with the 
terminology unless there's a distinction I'm missing.

(I'd also add a comment here, even though it doesn't seem to be local style, 
sigh)



Comment at: clang/include/clang/Sema/Sema.h:3307
 
   Decl *ActOnTag(Scope *S, unsigned TagSpec, TagUseKind TUK,
  SourceLocation KWLoc, CXXScopeSpec &SS, IdentifierInfo *Name,

hokein wrote:
> This is the key place where it returns the underlying decl.
> 
> Alternatively, we could return the UsingDecl as the result (there are only 4 
> places using it, might be possible to check/verify/adjust all caller place, I 
> feel a bit scary about it, as the name indicates, caller might assume the 
> return Decl is a `TagDecl`, e.g. the 
> [case](https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaTemplate.cpp#L10195))
Yeah, given the name I think returning the TagDecl makes sense.
(It looks like it should always return TagDecl, though it's hard to tell - if 
so then changing the static type to TagDecl* seems like it would help)

I think the extra out-param is fine, except given that there are few callsites, 
I'd prefer a mandatory `FoundUsingShadow*&` and explicitly ignoring it where 
appropriate - seems less cryptic and makes discarding sugar more obvious.



Comment at: clang/include/clang/Sema/Sema.h:3316
+ SkipBodyInfo *SkipBody = nullptr,
+ UsingShadowDecl** FoundUsingShadow = nullptr);
 

nit: needs clang-format for pointer-alignment here and elsewhere


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141280

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

[PATCH] D141342: [perf-training] Check extension in findFilesWithExtension

[Petr Hosek via Phabricator via cfe-commits]

phosek accepted this revision.
phosek 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/D141342/new/

https://reviews.llvm.org/D141342

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

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir added a comment.

It looks like this regressed the following example:

  % cat test.cc  # formatted with older clang-format
  a
  &cc() {
return 1;
  }
  % clang-format --version
  clang-format version 16.0.0 (https://github.com/llvm/llvm-project.git 
a28f0747c2f3728bd8a6f64f7c8ba80b4e0cda9f)
  % clang-format -style=google test.cc
  a &
  cc() {
return 1;
  }
  %

@owenpan, I'm planning to temporarily revert this until you have a chance to 
investigate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140956

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

[PATCH] D139774: [libclang] Add API to set temporary directory location

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

In D139774#4036496 , @aaron.ballman 
wrote:

> Given that we already have other setters to set global state, I'm less 
> concerned about adding another one. I hadn't realized we already introduced 
> the dangers here. But we should document the expectation that the call be 
> made before creating the index.

There is a difference: `clang_CXIndex_setGlobalOptions()` and 
`clang_CXIndex_setInvocationEmissionPathOption()` take a `CXIndex` argument and 
thus can only be called **after** creating an index. So requiring to call 
`clang_overrideTemporaryDirectory()` before creating an index, plus one more 
time with `nullptr` argument after disposing of the index, wouldn't be 
particularly consistent with other setters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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

[PATCH] D141349: Remove the ThreadLocal template from LLVM.

[Nikita Popov via Phabricator via cfe-commits]

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

LGTM

> As a precaution, use LLVM_THREAD_LOCAL which provides even greater
> backwards compatibility, allowing this to function even pre-C++11
> versions of GCC.

I don't think backwards compatibility is still relevant given our minimum 
version requirements. We can probably drop the `__thread` fallback. What 
LLVM_THREAD_LOCAL does though is fall back to a normal variable if 
`!LLVM_ENABLE_THREADS`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141349

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

[PATCH] D141362: [AST] include decls owned by FriendDecl in -ast-dump

[Sam McCall via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141362

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/test/AST/ast-dump-decl.cpp


Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -784,6 +784,7 @@
 // CHECK-NEXT:   FriendDecl
 // CHECK-NEXT: FunctionDecl{{.*}} foo
 // CHECK-NEXT:   FriendDecl{{.*}} 'class A':'A'
+// CHECK-NEXT: CXXRecordDecl{{.*}} class A
 // CHECK-NEXT:   FriendDecl{{.*}} 'T'
 
 namespace TestFileScopeAsmDecl {
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -644,8 +644,15 @@
   }
 
   void VisitFriendDecl(const FriendDecl *D) {
-if (!D->getFriendType())
+if (D->getFriendType()) {
+  // Traverse any CXXRecordDecl owned by this type, since
+  // it will not be in the parent context:
+  if (auto *ET = D->getFriendType()->getType()->getAs())
+if (auto *TD = ET->getOwnedTagDecl())
+  Visit(TD);
+} else {
   Visit(D->getFriendDecl());
+}
   }
 
   void VisitObjCMethodDecl(const ObjCMethodDecl *D) {


Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -784,6 +784,7 @@
 // CHECK-NEXT:   FriendDecl
 // CHECK-NEXT: FunctionDecl{{.*}} foo
 // CHECK-NEXT:   FriendDecl{{.*}} 'class A':'A'
+// CHECK-NEXT: CXXRecordDecl{{.*}} class A
 // CHECK-NEXT:   FriendDecl{{.*}} 'T'
 
 namespace TestFileScopeAsmDecl {
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -644,8 +644,15 @@
   }
 
   void VisitFriendDecl(const FriendDecl *D) {
-if (!D->getFriendType())
+if (D->getFriendType()) {
+  // Traverse any CXXRecordDecl owned by this type, since
+  // it will not be in the parent context:
+  if (auto *ET = D->getFriendType()->getType()->getAs())
+if (auto *TD = ET->getOwnedTagDecl())
+  Visit(TD);
+} else {
   Visit(D->getFriendDecl());
+}
   }
 
   void VisitObjCMethodDecl(const ObjCMethodDecl *D) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141362: [AST] include decls owned by FriendDecl in -ast-dump

[Haojian Wu via Phabricator via cfe-commits]

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

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141362

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

[PATCH] D139921: [include-cleaner] Ranking of providers based on hints

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 26 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:84
 
+/// Represents properties of a symbol provider.
+enum class Hint : uint8_t {

sammccall wrote:
> along with `SymbolLocation`, I think mixing our fundamental types and 
> analysis functions in this header makes the structure harder to understand 
> and think they both belong in `Types.h`
i still don't feel great about exposing these types as public API. would it 
help if I moved these into a `TypesInternal.h` ?



Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:85
+/// Represents properties of a symbol provider.
+enum class Hint : uint8_t {
+  None = 0x00,

sammccall wrote:
> hokein wrote:
> > IIUC, we're using a union bit for symbols and headers: some properties are 
> > for both (e.g. Complete); some properties are just for headers (e.g. 
> > Public, Canonical).
> > 
> > I found that this model is hard to follow (e.g. for each property, I need 
> > to keep thinking about whether this property is for just symbols/headers or 
> > both, the hint conversion step from Symbol to Header is not obvious).
> > 
> > What do you think about the following alternative ?
> > 
> > - define two hints (`SymbolHint`, `HeaderHint`)
> > - an explicit conversion from `SymbolHint` to `HeaderHint`
> > - `SymbolLocation` class has a `SymbolHint` member (for `Header` as well)
> > 
> > 
> > 
> > 
> I agree the model isn't obvious, but I think it is the right one and **should 
> be explained**.
> Breaking into a bunch of different types would be a mistake IMO. It would 
> make it look less like algebra and more like domain logic. The problem is the 
> algebra is important, e.g. it tells you how to merge hints to rank providers.
> 
> I think the model is:
>  - we have a "provides" DAG, headers provide locations which provide symbols 
> which satisfy AST nodes
>  - these hints are "capabilities" that can be provided by edges (or subpaths).
>  - if **any** path from Header => AST Node sees a capability, the header 
> provides that capability.
>  - this explains why most hints are expressed positively, and we take the 
> union. (if a header provides a complete definition and an incomplete one, we 
> want to call that "complete")
Yes, as discussed offline and Sam mentions in the comment idea is to have all 
hints expressed positively and take union of all the capabilities on the path 
from an AST node to a provider (also union in case there are multiple paths 
ending at the same provider). Each hint is to be provided by at most a single 
stage of the traversal (not sure why you think Complete is for both), I'll make 
this more concrete by renaming hint types to encode that in the name. I'll also 
add some docs explaining the model.



Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:90
+  Complete = 0x01,
+  /// Provides a non-private declaration for the symbol. Only absent in the
+  /// cases where file is explicitly marked as such, e.g. non self-contained or

sammccall wrote:
> hokein wrote:
> > I think this is no non-private declaration. The `Public` property is for 
> > headers, it indicates the symbol is provided by a non-private header.
> I agree with this one: because hints aggregate as union, it would be a 
> mistake to flag a Symbol as Public - that would mean any header that provided 
> it would end up with the `Public` hint, even if the header file is private!
> 
> I wonder if we should use a naming-convention for this, e.g. 
> `CompleteSymbol`, `PublicHeader` etc, indicating the layer at which the 
> signal is expected to be set.
i agree with renaming these to indicate the stage that produced them.



Comment at: clang-tools-extra/include-cleaner/lib/AnalysisInternal.h:93
+  /// IWYU private pragmas.
+  Public = 0x02,
+  /// Declaration is explicitly marked as canonical, e.g. with a IWYU private

hokein wrote:
> In most cases, this bit is set. If we use a flipped bit (`Private`), we don't 
> need to set it in many places.
as discussed, these need to be positively expressed (like others).



Comment at: clang-tools-extra/include-cleaner/lib/FindHeaders.cpp:89
+if (!PI)
+  return {{FE, Hint::Public}};
 while (FE) {

sammccall wrote:
> why never canonical?
> 
> In general, handling `!PI` as a special case up front seems like a bad idea: 
> the purpose of supporting it being nullable is to allow the code below to 
> used/tested without one. Seems we can get away with guarding PI-bits with `if 
> (PI)` below.
> (Avoiding the ternary initializer for `CurrentHints` would be a readability 
> side-benefit!)
in the absence of `PI`, we can't really do much in the loop below (as we would 
assume each file is self-contained by default) and would bail out in the first 
run. Hence

[PATCH] D139921: [include-cleaner] Ranking of providers based on hints

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 487721.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Address all comments but the ones on tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139921

Files:
  clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h
  clang-tools-extra/include-cleaner/lib/Analysis.cpp
  clang-tools-extra/include-cleaner/lib/AnalysisInternal.h
  clang-tools-extra/include-cleaner/lib/FindHeaders.cpp
  clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
  clang-tools-extra/include-cleaner/lib/LocateSymbol.cpp
  clang-tools-extra/include-cleaner/lib/Types.cpp
  clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
  clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp

Index: clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
===
--- clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/FindHeadersTest.cpp
@@ -7,7 +7,6 @@
 //===--===//
 
 #include "AnalysisInternal.h"
-#include "clang-include-cleaner/Analysis.h"
 #include "clang-include-cleaner/Record.h"
 #include "clang-include-cleaner/Types.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -15,15 +14,14 @@
 #include "clang/Basic/FileManager.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Testing/TestAST.h"
-#include "llvm/ADT/ArrayRef.h"
-#include "llvm/Support/raw_ostream.h"
-#include "llvm/Testing/Support/Annotations.h"
+#include "llvm/ADT/SmallVector.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
 
 namespace clang::include_cleaner {
 namespace {
+using testing::ElementsAre;
 using testing::UnorderedElementsAre;
 
 std::string guard(llvm::StringRef Code) {
@@ -53,11 +51,12 @@
   void buildAST() { AST = std::make_unique(Inputs); }
 
   llvm::SmallVector findHeaders(llvm::StringRef FileName) {
-return include_cleaner::findHeaders(
+auto Headers = include_cleaner::findHeaders(
 AST->sourceManager().translateFileLineCol(
 AST->fileManager().getFile(FileName).get(),
 /*Line=*/1, /*Col=*/1),
 AST->sourceManager(), &PI);
+return {Headers.begin(), Headers.end()};
   }
   const FileEntry *physicalHeader(llvm::StringRef FileName) {
 return AST->fileManager().getFile(FileName).get();
@@ -207,12 +206,166 @@
 CustomVisitor Visitor;
 Visitor.TraverseDecl(AST->context().getTranslationUnitDecl());
 
-llvm::SmallVector Headers = clang::include_cleaner::findHeaders(
+auto Headers = clang::include_cleaner::findHeaders(
 Visitor.Out->getLocation(), AST->sourceManager(),
 /*PragmaIncludes=*/nullptr);
 EXPECT_THAT(Headers, UnorderedElementsAre(physicalHeader("declare.h")));
   }
 }
 
+class HeadersForSymbolTest : public FindHeadersTest {
+protected:
+  llvm::SmallVector headersForFoo() {
+struct Visitor : public RecursiveASTVisitor {
+  const NamedDecl *Out = nullptr;
+  bool VisitNamedDecl(const NamedDecl *ND) {
+if (ND->getName() == "foo") {
+  EXPECT_TRUE(Out == nullptr || Out == ND->getCanonicalDecl())
+  << "Found multiple matches for foo.";
+  Out = cast(ND->getCanonicalDecl());
+}
+return true;
+  }
+};
+Visitor V;
+V.TraverseDecl(AST->context().getTranslationUnitDecl());
+if (!V.Out)
+  ADD_FAILURE() << "Couldn't find any decls named foo.";
+assert(V.Out);
+return headersForSymbol(*V.Out, AST->sourceManager(), &PI);
+  }
+};
+
+TEST_F(HeadersForSymbolTest, Deduplicates) {
+  Inputs.Code = R"cpp(
+#include "foo.h"
+  )cpp";
+  Inputs.ExtraFiles["foo.h"] = guard(R"cpp(
+// IWYU pragma: private, include "foo.h"
+void foo();
+void foo();
+  )cpp");
+  buildAST();
+  EXPECT_THAT(
+  headersForFoo(),
+  UnorderedElementsAre(physicalHeader("foo.h"),
+   // FIXME: de-duplicate across different kinds.
+   Header("\"foo.h\"")));
+}
+
+TEST_F(HeadersForSymbolTest, RankingPreservesDiscoveryOrder) {
+  Inputs.Code = R"cpp(
+#include "fox.h"
+#include "bar.h"
+  )cpp";
+  Inputs.ExtraFiles["fox.h"] = guard(R"cpp(
+void foo();
+  )cpp");
+  Inputs.ExtraFiles["bar.h"] = guard(R"cpp(
+void foo();
+  )cpp");
+  buildAST();
+  EXPECT_THAT(headersForFoo(),
+  ElementsAre(physicalHeader("fox.h"), physicalHeader("bar.h")));
+}
+
+TEST_F(HeadersForSymbolTest, Ranking) {
+  // Sorting is done over (canonical, public, complete) triplet.
+  Inputs.Code = R"cpp(
+#include "private.h"
+#include "public.h"
+#include "public_complete.h"
+  )cpp";
+  Inputs.ExtraFiles["public.h"] = guard(R"cpp(
+struct foo;
+  )cpp");
+  Inputs.ExtraFiles["private.h"] = guard(R"cpp(
+// IWYU pragma: private, in

[clang] 879bfe6 - Revert "[clang-format] Add an option for breaking after C++11 attributes"

[Krasimir Georgiev via cfe-commits]

Author: Krasimir Georgiev
Date: 2023-01-10T09:23:44Z
New Revision: 879bfe6a979295f834b76df66b19a203b93eed0f

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

LOG: Revert "[clang-format] Add an option for breaking after C++11 attributes"

This reverts commit a28f0747c2f3728bd8a6f64f7c8ba80b4e0cda9f.

It appears that this regresses some function definitions, added an
example as a comment over at https://reviews.llvm.org/D140956.

Added: 


Modified: 
clang/docs/ClangFormatStyleOptions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Format/Format.h
clang/lib/Format/ContinuationIndenter.cpp
clang/lib/Format/Format.cpp
clang/lib/Format/FormatToken.h
clang/lib/Format/TokenAnnotator.cpp
clang/lib/Format/TokenAnnotator.h
clang/unittests/Format/ConfigParseTest.cpp
clang/unittests/Format/FormatTest.cpp

Removed: 




diff  --git a/clang/docs/ClangFormatStyleOptions.rst 
b/clang/docs/ClangFormatStyleOptions.rst
index 4134d5a387d2d..23f5786723f6d 100644
--- a/clang/docs/ClangFormatStyleOptions.rst
+++ b/clang/docs/ClangFormatStyleOptions.rst
@@ -1762,41 +1762,6 @@ the configuration (without a prefix: ``Auto``).
}
 
 
-**BreakAfterAttributes** (``AttributeBreakingStyle``) 
:versionbadge:`clang-format 16`
-  Break after a group of C++11 attributes before a function
-  declaration/definition name.
-
-  Possible values:
-
-  * ``ABS_Always`` (in configuration: ``Always``)
-Always break after attributes.
-
-.. code-block:: c++
-
-  [[nodiscard]]
-  inline int f();
-  [[gnu::const]] [[nodiscard]]
-  int g();
-
-  * ``ABS_Leave`` (in configuration: ``Leave``)
-Leave the line breaking after attributes as is.
-
-.. code-block:: c++
-
-  [[nodiscard]] inline int f();
-  [[gnu::const]] [[nodiscard]]
-  int g();
-
-  * ``ABS_Never`` (in configuration: ``Never``)
-Never break after attributes.
-
-.. code-block:: c++
-
-  [[nodiscard]] inline int f();
-  [[gnu::const]] [[nodiscard]] int g();
-
-
-
 **BreakAfterJavaFieldAnnotations** (``Boolean``) :versionbadge:`clang-format 
3.8`
   Break after each annotation on a field in Java files.
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 8718deab35bf2..927c85249b729 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -889,8 +889,6 @@ clang-format
   To match the default behavior of clang-format 15, use the ``Keyword`` value.
 - Add ``IntegerLiteralSeparator`` option for fixing integer literal separators
   in C++, C#, Java, and JavaScript.
-- Add ``BreakAfterAttributes`` option for breaking after a group of C++11
-  attributes before a function declaration/definition name.
 - Add ``InsertNewlineAtEOF`` option for inserting a newline at EOF if missing.
 
 clang-extdef-mapping

diff  --git a/clang/include/clang/Format/Format.h 
b/clang/include/clang/Format/Format.h
index 6e5d3a163ec32..3a7a183d139f0 100644
--- a/clang/include/clang/Format/Format.h
+++ b/clang/include/clang/Format/Format.h
@@ -1221,36 +1221,6 @@ struct FormatStyle {
   /// \version 3.8
   BraceWrappingFlags BraceWrapping;
 
-  /// Different ways to break after attributes.
-  enum AttributeBreakingStyle : int8_t {
-/// Always break after attributes.
-/// \code
-///   [[nodiscard]]
-///   inline int f();
-///   [[gnu::const]] [[nodiscard]]
-///   int g();
-/// \endcode
-ABS_Always,
-/// Leave the line breaking after attributes as is.
-/// \code
-///   [[nodiscard]] inline int f();
-///   [[gnu::const]] [[nodiscard]]
-///   int g();
-/// \endcode
-ABS_Leave,
-/// Never break after attributes.
-/// \code
-///   [[nodiscard]] inline int f();
-///   [[gnu::const]] [[nodiscard]] int g();
-/// \endcode
-ABS_Never,
-  };
-
-  /// Break after a group of C++11 attributes before a function
-  /// declaration/definition name.
-  /// \version 16
-  AttributeBreakingStyle BreakAfterAttributes;
-
   /// If ``true``, clang-format will always break after a Json array `[`
   /// otherwise it will scan until the closing `]` to determine if it should 
add
   /// newlines between elements (prettier compatible).
@@ -4113,7 +4083,6 @@ struct FormatStyle {
BinPackArguments == R.BinPackArguments &&
BinPackParameters == R.BinPackParameters &&
BitFieldColonSpacing == R.BitFieldColonSpacing &&
-   BreakAfterAttributes == R.BreakAfterAttributes &&
BreakAfterJavaFieldAnnotations == R.BreakAfterJavaFieldAnnotations 
&&
BreakArrays == R.BreakArrays &&
BreakBeforeBinaryOperators == R.BreakBeforeBinaryOperators &&

diff  --git a/clang/lib/Format/ContinuationIndenter.cpp 
b/clang/lib/F

[clang] 96b52c1 - [Clang][RISCV] Expose vlenb to user

[via cfe-commits]

Author: eopXD
Date: 2023-01-10T02:23:44-08:00
New Revision: 96b52c1eec7b7952059a4cc4acd2f5e62e1ba1ca

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

LOG: [Clang][RISCV] Expose vlenb to user

This commit adds function `vlenb` into riscv_vector.h. `vlenb` is defined
through builtin function `__builtin_rvv_vlenb`, which is lowered to
`llvm.read_register`.

Reviewed By: kito-cheng, pcwang-thead

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

Added: 
clang/test/CodeGen/RISCV/rvv-intrinsics/vlenb.c

Modified: 
clang/include/clang/Basic/riscv_vector.td

Removed: 




diff  --git a/clang/include/clang/Basic/riscv_vector.td 
b/clang/include/clang/Basic/riscv_vector.td
index 6e089a7d87d6b..29ff8accf7f07 100644
--- a/clang/include/clang/Basic/riscv_vector.td
+++ b/clang/include/clang/Basic/riscv_vector.td
@@ -1558,6 +1558,32 @@ void vwrite_csr(enum RVV_CSR __csr, unsigned long 
__value) {
 }] in
 def vread_vwrite_csr: RVVHeader;
 
+let HeaderCode =
+[{
+#define vlenb() __builtin_rvv_vlenb()
+}] in
+def vlenb_macro: RVVHeader;
+
+let HasBuiltinAlias = false, HasVL = false, HasMasked = false,
+UnMaskedPolicyScheme = NonePolicy, MaskedPolicyScheme = NonePolicy,
+Log2LMUL = [0], IRName = "",
+ManualCodegen = [{
+{
+  LLVMContext &Context = CGM.getLLVMContext();
+  llvm::MDBuilder MDHelper(Context);
+
+  llvm::Metadata *Ops[] = {llvm::MDString::get(Context, "vlenb")};
+  llvm::MDNode *RegName = llvm::MDNode::get(Context, Ops);
+  llvm::Value *Metadata = llvm::MetadataAsValue::get(Context, RegName);
+  llvm::Function *F =
+CGM.getIntrinsic(llvm::Intrinsic::read_register, {SizeTy});
+  return Builder.CreateCall(F, Metadata);
+}
+}] in
+{
+  def vlenb : RVVBuiltin<"", "u", "i">;
+}
+
 // 6. Configuration-Setting Instructions
 // 6.1. vsetvli/vsetvl instructions
 

diff  --git a/clang/test/CodeGen/RISCV/rvv-intrinsics/vlenb.c 
b/clang/test/CodeGen/RISCV/rvv-intrinsics/vlenb.c
new file mode 100644
index 0..999e1accbed2f
--- /dev/null
+++ b/clang/test/CodeGen/RISCV/rvv-intrinsics/vlenb.c
@@ -0,0 +1,39 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --check-globals
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv32 -target-feature +v -disable-O0-optnone 
-emit-llvm -Qn %s -o - \
+// RUN: | opt -S -O2 | FileCheck --check-prefix=RV32 %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +v -disable-O0-optnone 
-emit-llvm -Qn %s -o - \
+// RUN: | opt -S -O2 | FileCheck --check-prefix=RV64 %s
+
+#include 
+
+// RV32-LABEL: @test_vlenb(
+// RV32-NEXT:  entry:
+// RV32-NEXT:[[TMP0:%.*]] = tail call i32 @llvm.read_register.i32(metadata 
[[META3:![0-9]+]])
+// RV32-NEXT:ret i32 [[TMP0]]
+//
+// RV64-LABEL: @test_vlenb(
+// RV64-NEXT:  entry:
+// RV64-NEXT:[[TMP0:%.*]] = tail call i64 @llvm.read_register.i64(metadata 
[[META3:![0-9]+]])
+// RV64-NEXT:ret i64 [[TMP0]]
+//
+unsigned long test_vlenb(void) {
+  return vlenb();
+}
+//.
+// RV32: attributes #0 = { mustprogress nofree noinline nosync nounwind 
willreturn memory(read) vscale_range(2,1024) "no-trapping-math"="true" 
"stack-protector-buffer-size"="8" 
"target-features"="+32bit,+d,+f,+v,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b"
 }
+// RV32: attributes #1 = { mustprogress nocallback nofree nosync nounwind 
willreturn memory(read) }
+//.
+// RV64: attributes #0 = { mustprogress nofree noinline nosync nounwind 
willreturn memory(read) vscale_range(2,1024) "no-trapping-math"="true" 
"stack-protector-buffer-size"="8" 
"target-features"="+64bit,+d,+f,+v,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b"
 }
+// RV64: attributes #1 = { mustprogress nocallback nofree nosync nounwind 
willreturn memory(read) }
+//.
+// RV32: !0 = !{i32 1, !"wchar_size", i32 4}
+// RV32: !1 = !{i32 1, !"target-abi", !"ilp32d"}
+// RV32: !2 = !{i32 1, !"SmallDataLimit", i32 0}
+// RV32: !3 = !{!"vlenb"}
+//.
+// RV64: !0 = !{i32 1, !"wchar_size", i32 4}
+// RV64: !1 = !{i32 1, !"target-abi", !"lp64d"}
+// RV64: !2 = !{i32 1, !"SmallDataLimit", i32 0}
+// RV64: !3 = !{!"vlenb"}
+//.



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

[PATCH] D141032: [Clang][RISCV] Expose vlenb to user

[Yueh-Ting (eop) Chen 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 rG96b52c1eec7b: [Clang][RISCV] Expose vlenb to user (authored 
by eopXD).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141032

Files:
  clang/include/clang/Basic/riscv_vector.td
  clang/test/CodeGen/RISCV/rvv-intrinsics/vlenb.c


Index: clang/test/CodeGen/RISCV/rvv-intrinsics/vlenb.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvv-intrinsics/vlenb.c
@@ -0,0 +1,39 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --check-globals
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv32 -target-feature +v -disable-O0-optnone 
-emit-llvm -Qn %s -o - \
+// RUN: | opt -S -O2 | FileCheck --check-prefix=RV32 %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +v -disable-O0-optnone 
-emit-llvm -Qn %s -o - \
+// RUN: | opt -S -O2 | FileCheck --check-prefix=RV64 %s
+
+#include 
+
+// RV32-LABEL: @test_vlenb(
+// RV32-NEXT:  entry:
+// RV32-NEXT:[[TMP0:%.*]] = tail call i32 @llvm.read_register.i32(metadata 
[[META3:![0-9]+]])
+// RV32-NEXT:ret i32 [[TMP0]]
+//
+// RV64-LABEL: @test_vlenb(
+// RV64-NEXT:  entry:
+// RV64-NEXT:[[TMP0:%.*]] = tail call i64 @llvm.read_register.i64(metadata 
[[META3:![0-9]+]])
+// RV64-NEXT:ret i64 [[TMP0]]
+//
+unsigned long test_vlenb(void) {
+  return vlenb();
+}
+//.
+// RV32: attributes #0 = { mustprogress nofree noinline nosync nounwind 
willreturn memory(read) vscale_range(2,1024) "no-trapping-math"="true" 
"stack-protector-buffer-size"="8" 
"target-features"="+32bit,+d,+f,+v,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b"
 }
+// RV32: attributes #1 = { mustprogress nocallback nofree nosync nounwind 
willreturn memory(read) }
+//.
+// RV64: attributes #0 = { mustprogress nofree noinline nosync nounwind 
willreturn memory(read) vscale_range(2,1024) "no-trapping-math"="true" 
"stack-protector-buffer-size"="8" 
"target-features"="+64bit,+d,+f,+v,+zve32f,+zve32x,+zve64d,+zve64f,+zve64x,+zvl128b,+zvl32b,+zvl64b"
 }
+// RV64: attributes #1 = { mustprogress nocallback nofree nosync nounwind 
willreturn memory(read) }
+//.
+// RV32: !0 = !{i32 1, !"wchar_size", i32 4}
+// RV32: !1 = !{i32 1, !"target-abi", !"ilp32d"}
+// RV32: !2 = !{i32 1, !"SmallDataLimit", i32 0}
+// RV32: !3 = !{!"vlenb"}
+//.
+// RV64: !0 = !{i32 1, !"wchar_size", i32 4}
+// RV64: !1 = !{i32 1, !"target-abi", !"lp64d"}
+// RV64: !2 = !{i32 1, !"SmallDataLimit", i32 0}
+// RV64: !3 = !{!"vlenb"}
+//.
Index: clang/include/clang/Basic/riscv_vector.td
===
--- clang/include/clang/Basic/riscv_vector.td
+++ clang/include/clang/Basic/riscv_vector.td
@@ -1558,6 +1558,32 @@
 }] in
 def vread_vwrite_csr: RVVHeader;
 
+let HeaderCode =
+[{
+#define vlenb() __builtin_rvv_vlenb()
+}] in
+def vlenb_macro: RVVHeader;
+
+let HasBuiltinAlias = false, HasVL = false, HasMasked = false,
+UnMaskedPolicyScheme = NonePolicy, MaskedPolicyScheme = NonePolicy,
+Log2LMUL = [0], IRName = "",
+ManualCodegen = [{
+{
+  LLVMContext &Context = CGM.getLLVMContext();
+  llvm::MDBuilder MDHelper(Context);
+
+  llvm::Metadata *Ops[] = {llvm::MDString::get(Context, "vlenb")};
+  llvm::MDNode *RegName = llvm::MDNode::get(Context, Ops);
+  llvm::Value *Metadata = llvm::MetadataAsValue::get(Context, RegName);
+  llvm::Function *F =
+CGM.getIntrinsic(llvm::Intrinsic::read_register, {SizeTy});
+  return Builder.CreateCall(F, Metadata);
+}
+}] in
+{
+  def vlenb : RVVBuiltin<"", "u", "i">;
+}
+
 // 6. Configuration-Setting Instructions
 // 6.1. vsetvli/vsetvl instructions
 


Index: clang/test/CodeGen/RISCV/rvv-intrinsics/vlenb.c
===
--- /dev/null
+++ clang/test/CodeGen/RISCV/rvv-intrinsics/vlenb.c
@@ -0,0 +1,39 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --check-globals
+// REQUIRES: riscv-registered-target
+// RUN: %clang_cc1 -triple riscv32 -target-feature +v -disable-O0-optnone -emit-llvm -Qn %s -o - \
+// RUN: | opt -S -O2 | FileCheck --check-prefix=RV32 %s
+// RUN: %clang_cc1 -triple riscv64 -target-feature +v -disable-O0-optnone -emit-llvm -Qn %s -o - \
+// RUN: | opt -S -O2 | FileCheck --check-prefix=RV64 %s
+
+#include 
+
+// RV32-LABEL: @test_vlenb(
+// RV32-NEXT:  entry:
+// RV32-NEXT:[[TMP0:%.*]] = tail call i32 @llvm.read_register.i32(metadata [[META3:![0-9]+]])
+// RV32-NEXT:ret i32 [[TMP0]]
+//
+// RV64-LABEL: @test_vlenb(
+// RV64-NEXT:  entry:
+// RV64-NEXT:[[TMP0:%.*]] = tail call i64 @llvm.read_register.i64(metadata [[META3:![0-9]+]])
+// RV64-NEXT:ret i64 [[TMP0]]
+//
+unsigned long test

[PATCH] D141350: Fix runtime problem for base class member data used in target region.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8490
+if (MD)
+  if (const CXXRecordDecl *RD = dyn_cast(MD->getParent()))
+HasBaseClass = RD->getNumBases() > 0;

`const auto *RD = dyn_cast(MD->getParent())`. Also, can we use 
`MD->getParent()->getAsCXXRecordDecl()`?



Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:8493
 // There should not be a mapper for a combined entry.
+if (MD && HasBaseClass) {
+  // OpenMP 5.2 148:21:

MD is true if HasBaseClass is true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141350

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

[PATCH] D139010: [clang][WebAssembly] Implement support for table types and builtins

[Paulo Matos via Phabricator via cfe-commits]

pmatos updated this revision to Diff 487747.
pmatos added a comment.

Fix test broken after recent change to opt call. Rebase again on main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139010

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/BuiltinsWebAssembly.def
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/TypeBitCodes.def
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Basic/Targets/WebAssembly.cpp
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGValue.h
  clang/lib/Sema/SemaCast.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExceptionSpec.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/WebAssembly/builtins-table.c
  clang/test/CodeGen/WebAssembly/table.c
  clang/test/Sema/builtins-wasm.c
  clang/test/Sema/wasm-refs-and-tables.c
  clang/test/SemaCXX/wasm-refs-and-tables.cpp
  clang/test/SemaCXX/wasm-refs.cpp
  llvm/include/llvm/CodeGen/WasmAddressSpaces.h
  llvm/include/llvm/IR/Type.h
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.cpp
  llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
  llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
  llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp

Index: llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyLowerRefTypesIntPtrConv.cpp
@@ -62,8 +62,9 @@
   for (inst_iterator I = inst_begin(F), E = inst_end(F); I != E; ++I) {
 PtrToIntInst *PTI = dyn_cast(&*I);
 IntToPtrInst *ITP = dyn_cast(&*I);
-if (!(PTI && WebAssembly::isRefType(PTI->getPointerOperand()->getType())) &&
-!(ITP && WebAssembly::isRefType(ITP->getDestTy(
+if (!(PTI &&
+  PTI->getPointerOperand()->getType()->isWebAssemblyReferenceType()) &&
+!(ITP && ITP->getDestTy()->isWebAssemblyReferenceType()))
   continue;
 
 UndefValue *U = UndefValue::get(I->getType());
Index: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
===
--- llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1202,7 +1202,7 @@
   // Lastly, if this is a call to a funcref we need to add an instruction
   // table.set to the chain and transform the call.
   if (CLI.CB &&
-  WebAssembly::isFuncrefType(CLI.CB->getCalledOperand()->getType())) {
+  CLI.CB->getCalledOperand()->getType()->isWebAssemblyFuncrefType()) {
 // In the absence of function references proposal where a funcref call is
 // lowered to call_ref, using reference types we generate a table.set to set
 // the funcref to a special table used solely for this purpose, followed by
Index: llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
===
--- llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
+++ llvm/lib/Target/WebAssembly/Utils/WebAssemblyTypeUtilities.h
@@ -45,43 +45,6 @@
   Multivalue = 0x,
 };
 
-enum WasmAddressSpace : unsigned {
-  // Default address space, for pointers to linear memory (stack, heap, data).
-  WASM_ADDRESS_SPACE_DEFAULT = 0,
-  // A non-integral address space for pointers to named objects outside of
-  // linear memory: WebAssembly globals or WebAssembly locals.  Loads and stores
-  // to these pointers are lowered to global.get / global.set or local.get /
-  // local.set, as appropriate.
-  WASM_ADDRESS_SPACE_VAR = 1,
-  // A non-integral address space for externref values
-  WASM_ADDRESS_SPACE_EXTERNREF = 10,
-  // A non-integral address space for funcref values
-  WASM_ADDRESS_SPACE_FUNCREF = 20,
-};
-
-inline bool isDefaultAddressSpace(unsigned AS) {
-  return AS == WASM_ADDRESS_SPACE_DEFAULT;
-}
-inline bool isWasmVarAddressSpace(unsigned AS) {
-  return AS == WASM_ADDRESS_SPACE_VAR;
-}
-inline bool isValidAddressSpace(unsigned AS) {
-  return isDefaultAddressSpace(AS) || isWasmVarAddressSpace(AS);
-}
-inline bool isFuncrefType(const Type *Ty) {
-  return isa(Ty) &&
- Ty->getPointerAddressSpace() ==
- WasmAddressSpace::WASM_ADDRESS_SPACE_FUNCREF;
-}
-inline bool isExternrefType(const Type *Ty) {
-  return isa(Ty) &&
- Ty->getPointerAddressSpace() =

[PATCH] D141050: [standalone-build] outsource, simplify and unify repetitive CMake code

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk updated this revision to Diff 487750.
kwk marked 2 inline comments as done.
kwk added a comment.

- Don't quote output variables
- Use 2 space indention in CMake files
- Make out_var optional for get_llvm_utility_binary_path
- Rename get_llvm_utility_binary_path -> require_llvm_utility_binary_path


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141050

Files:
  clang/CMakeLists.txt
  cmake/Modules/StandaloneBuildHelpers.cmake
  lld/CMakeLists.txt

Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -23,23 +23,16 @@
 # Must go below project(..)
 include(GNUInstallDirs)
 
-if(LLD_BUILT_STANDALONE)
-  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
-  set(CMAKE_CXX_STANDARD_REQUIRED YES)
-  set(CMAKE_CXX_EXTENSIONS NO)
-
-  set(CMAKE_INCLUDE_CURRENT_DIR ON)
-
-  find_package(LLVM REQUIRED HINTS "${LLVM_CMAKE_DIR}")
-  list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
-
-  # Turn into CACHE PATHs for overwritting
-  set(LLVM_INCLUDE_DIRS ${LLVM_INCLUDE_DIRS} CACHE PATH "Path to llvm/include and any other header dirs needed")
-  set(LLVM_BINARY_DIR "${LLVM_BINARY_DIR}" CACHE PATH "Path to LLVM build tree")
-  set(LLVM_MAIN_SRC_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../llvm" CACHE PATH "Path to LLVM source tree")
+# Make sure that our source directory is on the current cmake module path so that
+# we can include cmake files from this directory.
+list(INSERT CMAKE_MODULE_PATH 0
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
 
-  find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
-NO_DEFAULT_PATH)
+if(LLD_BUILT_STANDALONE)
+  include(StandaloneBuildHelpers)
+  require_llvm_utility_binary_path("llvm-tblgen" LLVM_TABLEGEN_EXE)
 
   # They are used as destination of target generators.
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
@@ -59,53 +52,17 @@
   if(LLVM_INCLUDE_TESTS)
 find_package(Python3 ${LLVM_MINIMUM_PYTHON_VERSION} REQUIRED
   COMPONENTS Interpreter)
-
-# Check prebuilt llvm/utils.
-if(EXISTS ${LLVM_TOOLS_BINARY_DIR}/FileCheck${CMAKE_EXECUTABLE_SUFFIX}
-AND EXISTS ${LLVM_TOOLS_BINARY_DIR}/not${CMAKE_EXECUTABLE_SUFFIX})
-  set(LLVM_UTILS_PROVIDED ON)
-endif()
-
-if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
-  # Note: path not really used, except for checking if lit was found
-  set(LLVM_LIT ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
-  if(NOT LLVM_UTILS_PROVIDED)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/FileCheck utils/FileCheck)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/not utils/not)
-set(LLVM_UTILS_PROVIDED ON)
-set(LLD_TEST_DEPS FileCheck not)
-  endif()
-  set(UNITTEST_DIR ${LLVM_THIRD_PARTY_DIR}/unittest)
-  if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h
-  AND NOT EXISTS ${LLVM_LIBRARY_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}gtest${CMAKE_STATIC_LIBRARY_SUFFIX}
-  AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
-add_subdirectory(${UNITTEST_DIR} third-party/unittest)
-  endif()
-else()
-  # Seek installed Lit.
-  find_program(LLVM_LIT
-   NAMES llvm-lit lit.py lit
-   PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
-   DOC "Path to lit.py")
-endif()
-
-if(LLVM_LIT)
-  # Define the default arguments to use with 'lit', and an option for the user
-  # to override.
-  set(LIT_ARGS_DEFAULT "-sv")
-  if (MSVC OR XCODE)
-set(LIT_ARGS_DEFAULT "${LIT_ARGS_DEFAULT} --no-progress-bar")
-  endif()
-  set(LLVM_LIT_ARGS "${LIT_ARGS_DEFAULT}" CACHE STRING "Default options for lit")
-
-  get_errc_messages(LLVM_LIT_ERRC_MESSAGES)
-
-  # On Win32 hosts, provide an option to specify the path to the GnuWin32 tools.
-  if(WIN32 AND NOT CYGWIN)
-set(LLVM_LIT_TOOLS_DIR "" CACHE PATH "Path to GnuWin32 tools")
-  endif()
-else()
-  set(LLVM_INCLUDE_TESTS OFF)
+find_external_lit()
+set_lit_defaults()
+require_llvm_utility_binary_path("FileCheck")
+require_llvm_utility_binary_path("count")
+require_llvm_utility_binary_path("not")
+
+set(UNITTEST_DIR ${LLVM_THIRD_PARTY_DIR}/unittest)
+if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h
+AND NOT EXISTS ${LLVM_LIBRARY_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}gtest${CMAKE_STATIC_LIBRARY_SUFFIX}
+AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
+  add_subdirectory(${UNITTEST_DIR} third-party/unittest)
 endif()
   endif()
 
@@ -153,12 +110,6 @@
 "`CMakeFiles'. Please delete them.")
 endif()
 
-# Add path for custom modules.
-list(INSERT CMAKE_MODULE_PATH 0
-  "${LLD_SOURCE_DIR}/cmake/modules"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
-  )
-
 include(AddLLD)
 
 option(LLD_USE_VTUNE
Index: cmake/Modules/Stan

[PATCH] D141050: [standalone-build] outsource, simplify and unify repetitive CMake code

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk added a comment.

Thank you @phosek for your review! I really appreciate it. I've addressed all 
of your comments. Could you give it another review?




Comment at: clang/CMakeLists.txt:35
+  include(StandaloneBuildHelpers)
+  get_llvm_utility_binary_path("llvm-tblgen" "LLVM_TABLEGEN_EXE")
 

phosek wrote:
> It's somewhat unusual to quote output variable names in our CMake files, I'd 
> prefer to follow existing conventions for consistency.
I obey to the consistency.



Comment at: cmake/Modules/StandaloneBuildHelpers.cmake:12-19
+if (NOT (CLANG_BUILT_STANDALONE
+ OR LLD_BUILT_STANDALONE
+ OR COMPILER_RT_STANDALONE_BUILD
+ OR OPENMP_STANDALONE_BUILD
+ OR MLIR_STANDALONE_BUILD
+ OR LLDB_BUILT_STANDALONE))
+message(FATAL_ERROR "Make sure you build in standalone mode")

phosek wrote:
> I'd omit this, I don't think it's necessary to restrict the usage of this 
> module.
I think restricting the usage makes it clear for what it is. And right now this 
CMake file does stuff just when you include it which not a good thing in terms 
of isolation. If you don't mind I'd like to keep this restriction and only 
remove it before this patch lands in a better shape. Okay?



Comment at: cmake/Modules/StandaloneBuildHelpers.cmake:25-30
+if(NOT MSVC_IDE)
+set(LLVM_ENABLE_ASSERTIONS ${ENABLE_ASSERTIONS}
+  CACHE BOOL "Enable assertions")
+# Assertions should follow llvm-config's.
+mark_as_advanced(LLVM_ENABLE_ASSERTIONS)
+endif()

phosek wrote:
> Do you know why is this needed or this just copy pasted from Clang? I'd 
> consider dropping this altogether (ideally in a separate change).
Honestly, I started to move the CMake code from clang over to this file piece 
by piece once I found that the exact or similar formatted code existed in other 
projects like lld. But you caught me red-handed :).

This is the patch that introduced this particular section: [[ 
https://github.com/llvm/llvm-project/commit/e6d79ec0ebac350355c76d8132e9f1cce62e0cac
 | e6d79ec0ebac350355c76d8132e9f1cce62e0cac ]]. It is from 2013.

I'm not so long involved in the llvm project that I can give a solid answer to 
this but hasn't llvm-config been deprecated for building LLVM itself? If that 
is the case, I'm happy to remove this section from the `clang/CMakeLists.txt` 
in the first place in a separate patch. 



Comment at: cmake/Modules/StandaloneBuildHelpers.cmake:45-46
+
+# Automatically add the current source and build directories to the include 
path.
+set(CMAKE_INCLUDE_CURRENT_DIR ON)
+

phosek wrote:
> This shouldn't be needed since you've already had to insert the module path 
> manually.
I don't see what the one has to do with the other immediately. Maybe I'm a 
fool. The `CMAKE_INCLUDE_CURRENT_DIR` is for including C/C++ files and not 
CMake files so it doesn't matter what I have in my module path, no?. Here's a 
little example project I created to proof to myself that my assumption is 
really true: https://github.com/kwk/cmake-example-CMAKE_INCLUDE_CURRENT_DIR . 
Just run `make` once you've clone it. Then comment out [[ 
https://github.com/kwk/cmake-example-CMAKE_INCLUDE_CURRENT_DIR/blob/main/src/CMakeLists.txt#L10
 | this line ]] to see the effect: `include(EnableCurrentIncludeDir)`.



Comment at: cmake/Modules/StandaloneBuildHelpers.cmake:61
+function(get_llvm_utility_binary_path utility out_var)
+set(_imploc IMPORTED_LOCATION_NOCONFIG)
+# Based on the build type that the installed LLVM was built with,

phosek wrote:
> We use 2 spaces for indentation in our CMake files.
I've fixed the indention to use 2 spaces.



Comment at: cmake/Modules/StandaloneBuildHelpers.cmake:82-109
+# The set_lit_defaults macro exists because the code existed in multiple
+# locations before.
+macro(set_lit_defaults)
+set(LIT_ARGS_DEFAULT "-sv")
+if (MSVC OR XCODE)
+  set(LIT_ARGS_DEFAULT "${LIT_ARGS_DEFAULT} --no-progress-bar")
+endif()

phosek wrote:
> Is there any reason to keep these two macros separate? Could we combine them?
Yes, to both of your questions :) . Yes, I find it easier to read the files 
that include this file and use the functions and macros when they are 
separated. And yes, we could combine them to `setup_external_lit` or something 
alike. But for the course of this patch I'd like to keep them separated and do 
the merge later if you don't mind.



Comment at: lld/CMakeLists.txt:57
+set_lit_defaults()
+get_llvm_utility_binary_path("FileCheck" "FileCheck_EXE")
+get_llvm_utility_binary_path("count" "count_EXE")

phosek wrote:
> Where are these variables being used?
They were not being used, yet. I forgot to make the second parameter of 
`get_llvm_utility_binary_path` an optional one. Now it is optional and you can 
omit the second parameter. I've

[PATCH] D140956: [clang-format] Add an option for breaking after C++11 attributes

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Before

  M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=identifier L=25 PPK=2 
FakeLParens= FakeRParens=0 II=0x1aa92d99900 Text='a'
  M=0 C=0 T=TemplateOpener S=0 F=0 B=0 BK=0 P=30 Name=less L=26 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='<'
  M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=360 Name=identifier L=37 PPK=2 
FakeLParens= FakeRParens=0 II=0x1aa92d99938 Text='bbb'
  M=0 C=0 T=TemplateCloser S=0 F=0 B=0 BK=0 P=270 Name=greater L=38 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='>'
  M=0 C=1 T=PointerOrReference S=1 F=0 B=0 BK=0 P=210 Name=amp L=40 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='&'
  M=0 C=1 T=FunctionDeclarationName S=0 F=0 B=0 BK=0 P=220 Name=identifier L=78 
PPK=2 FakeLParens= FakeRParens=0 II=0x1aa92d99988 
Text='cc'

After

  M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=identifier L=25 PPK=2 
FakeLParens= FakeRParens=0 II=0x206f1adc870 Text='a'
  M=0 C=0 T=TemplateOpener S=0 F=0 B=0 BK=0 P=30 Name=less L=26 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='<'
  M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=360 Name=identifier L=37 PPK=2 
FakeLParens= FakeRParens=0 II=0x206f1adc8a8 Text='bbb'
  M=0 C=0 T=TemplateCloser S=0 F=0 B=0 BK=0 P=270 Name=greater L=38 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='>'
  M=0 C=0 T=PointerOrReference S=1 F=0 B=0 BK=0 P=210 Name=amp L=40 PPK=2 
FakeLParens= FakeRParens=0 II=0x0 Text='&'
  M=0 C=1 T=FunctionDeclarationName S=0 F=0 B=0 BK=0 P=220 Name=identifier L=78 
PPK=2 FakeLParens= FakeRParens=0 II=0x206f1adc8f8 
Text='cc'

Looks like the "C" (Can Break?) goes from 1->0

"M=0 C=0 T=PointerOrReference"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140956

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

[PATCH] D141375: [SYCL][OpenMP] Fix compilation errors for unsupported __bf16 intrinsics

[Elizabeth Andrews via Phabricator via cfe-commits]

eandrews created this revision.
eandrews added reviewers: mikerice, jyu2, bader, jdoerfert, aaron.ballman.
Herald added subscribers: Naghasan, Anastasia, ebevhan, guansong, yaxunl.
Herald added a project: All.
eandrews requested review of this revision.
Herald added a subscriber: sstefan1.

This patch uses existing deferred diagnostics framework to emit error for 
unsupported type __bf16 in device code. Error is not emitted in host code.


https://reviews.llvm.org/D141375

Files:
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/SemaSYCL/bf16.cpp


Index: clang/test/SemaSYCL/bf16.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/bf16.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple spir64 -aux-triple x86_64-unknown-linux-gnu 
-fsycl-is-device -verify -fsyntax-only %s
+
+template 
+__attribute__((sycl_kernel)) void kernel(Func kernelFunc) {
+  kernelFunc(); // expected-note {{called by 'kernel}}
+}
+
+void host_ok(void) {
+  __bf16 A;
+}
+
+int main()
+{  host_ok();
+  __bf16 var; // expected-note {{'var' defined here}}
+  kernel([=]() {
+(void)var; // expected-error {{'var' requires 16 bit size '__bf16' type 
support, but target 'spir64' does not support it}}
+int B = sizeof(__bf16);
+  });
+
+  return 0;
+}
+
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -1518,9 +1518,10 @@
 break;
   case DeclSpec::TST_half:Result = Context.HalfTy; break;
   case DeclSpec::TST_BFloat16:
-if (!S.Context.getTargetInfo().hasBFloat16Type())
-  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported)
-<< "__bf16";
+if (!S.Context.getTargetInfo().hasBFloat16Type() &&
+!(S.getLangOpts().OpenMP && S.getLangOpts().OpenMPIsDevice) &&
+!S.getLangOpts().SYCLIsDevice)
+  S.Diag(DS.getTypeSpecTypeLoc(), diag::err_type_unsupported) << "__bf16";
 Result = Context.BFloat16Ty;
 break;
   case DeclSpec::TST_float:   Result = Context.FloatTy; break;
Index: clang/lib/Sema/Sema.cpp
===
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -1974,6 +1974,8 @@
 (Ty->isIbm128Type() && !Context.getTargetInfo().hasIbm128Type()) ||
 (Ty->isIntegerType() && Context.getTypeSize(Ty) == 128 &&
  !Context.getTargetInfo().hasInt128Type()) ||
+(Ty->isBFloat16Type() && !Context.getTargetInfo().hasBFloat16Type() &&
+ !LangOpts.CUDAIsDevice) ||
 LongDoubleMismatched) {
   PartialDiagnostic PD = PDiag(diag::err_target_unsupported_type);
   if (D)
Index: clang/lib/AST/ItaniumMangle.cpp
===
--- clang/lib/AST/ItaniumMangle.cpp
+++ clang/lib/AST/ItaniumMangle.cpp
@@ -3050,7 +3050,11 @@
 break;
   }
   case BuiltinType::BFloat16: {
-const TargetInfo *TI = &getASTContext().getTargetInfo();
+const TargetInfo *TI = ((getASTContext().getLangOpts().OpenMP &&
+ getASTContext().getLangOpts().OpenMPIsDevice) ||
+getASTContext().getLangOpts().SYCLIsDevice)
+   ? getASTContext().getAuxTargetInfo()
+   : &getASTContext().getTargetInfo();
 Out << TI->getBFloat16Mangling();
 break;
   }
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -2141,6 +2141,11 @@
   if (Target->hasBFloat16Type()) {
 Width = Target->getBFloat16Width();
 Align = Target->getBFloat16Align();
+  } else if ((getLangOpts().SYCLIsDevice ||
+  (getLangOpts().OpenMP && getLangOpts().OpenMPIsDevice)) &&
+ AuxTarget->hasBFloat16Type()) {
+Width = AuxTarget->getBFloat16Width();
+Align = AuxTarget->getBFloat16Align();
   }
   break;
 case BuiltinType::Float16:


Index: clang/test/SemaSYCL/bf16.cpp
===
--- /dev/null
+++ clang/test/SemaSYCL/bf16.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple spir64 -aux-triple x86_64-unknown-linux-gnu -fsycl-is-device -verify -fsyntax-only %s
+
+template 
+__attribute__((sycl_kernel)) void kernel(Func kernelFunc) {
+  kernelFunc(); // expected-note {{called by 'kernel}}
+}
+
+void host_ok(void) {
+  __bf16 A;
+}
+
+int main()
+{  host_ok();
+  __bf16 var; // expected-note {{'var' defined here}}
+  kernel([=]() {
+(void)var; // expected-error {{'var' requires 16 bit size '__bf16' type support, but target 'spir64' does not support it}}
+int B = sizeof(__bf16);
+  });
+
+  return 0;
+}
+
Index: clang/lib/Sema/SemaType.cpp
===

[clang] 98ae361 - [AST] include decls owned by FriendDecl in -ast-dump

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2023-01-10T13:59:58+01:00
New Revision: 98ae3616cd1536643a78beeddff119c028f71df6

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

LOG: [AST] include decls owned by FriendDecl in -ast-dump

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

Added: 


Modified: 
clang/include/clang/AST/ASTNodeTraverser.h
clang/test/AST/ast-dump-decl.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTNodeTraverser.h 
b/clang/include/clang/AST/ASTNodeTraverser.h
index 151a9c6b58524..3089658305bfc 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -644,8 +644,15 @@ class ASTNodeTraverser
   }
 
   void VisitFriendDecl(const FriendDecl *D) {
-if (!D->getFriendType())
+if (D->getFriendType()) {
+  // Traverse any CXXRecordDecl owned by this type, since
+  // it will not be in the parent context:
+  if (auto *ET = D->getFriendType()->getType()->getAs())
+if (auto *TD = ET->getOwnedTagDecl())
+  Visit(TD);
+} else {
   Visit(D->getFriendDecl());
+}
   }
 
   void VisitObjCMethodDecl(const ObjCMethodDecl *D) {

diff  --git a/clang/test/AST/ast-dump-decl.cpp 
b/clang/test/AST/ast-dump-decl.cpp
index 61e7d9ec60d09..e22ea60b26061 100644
--- a/clang/test/AST/ast-dump-decl.cpp
+++ b/clang/test/AST/ast-dump-decl.cpp
@@ -784,6 +784,7 @@ template class TestFriendDecl {
 // CHECK-NEXT:   FriendDecl
 // CHECK-NEXT: FunctionDecl{{.*}} foo
 // CHECK-NEXT:   FriendDecl{{.*}} 'class A':'A'
+// CHECK-NEXT: CXXRecordDecl{{.*}} class A
 // CHECK-NEXT:   FriendDecl{{.*}} 'T'
 
 namespace TestFileScopeAsmDecl {



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

[PATCH] D141362: [AST] include decls owned by FriendDecl in -ast-dump

[Sam McCall via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG98ae3616cd15: [AST] include decls owned by FriendDecl in 
-ast-dump (authored by sammccall).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141362

Files:
  clang/include/clang/AST/ASTNodeTraverser.h
  clang/test/AST/ast-dump-decl.cpp


Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -784,6 +784,7 @@
 // CHECK-NEXT:   FriendDecl
 // CHECK-NEXT: FunctionDecl{{.*}} foo
 // CHECK-NEXT:   FriendDecl{{.*}} 'class A':'A'
+// CHECK-NEXT: CXXRecordDecl{{.*}} class A
 // CHECK-NEXT:   FriendDecl{{.*}} 'T'
 
 namespace TestFileScopeAsmDecl {
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -644,8 +644,15 @@
   }
 
   void VisitFriendDecl(const FriendDecl *D) {
-if (!D->getFriendType())
+if (D->getFriendType()) {
+  // Traverse any CXXRecordDecl owned by this type, since
+  // it will not be in the parent context:
+  if (auto *ET = D->getFriendType()->getType()->getAs())
+if (auto *TD = ET->getOwnedTagDecl())
+  Visit(TD);
+} else {
   Visit(D->getFriendDecl());
+}
   }
 
   void VisitObjCMethodDecl(const ObjCMethodDecl *D) {


Index: clang/test/AST/ast-dump-decl.cpp
===
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -784,6 +784,7 @@
 // CHECK-NEXT:   FriendDecl
 // CHECK-NEXT: FunctionDecl{{.*}} foo
 // CHECK-NEXT:   FriendDecl{{.*}} 'class A':'A'
+// CHECK-NEXT: CXXRecordDecl{{.*}} class A
 // CHECK-NEXT:   FriendDecl{{.*}} 'T'
 
 namespace TestFileScopeAsmDecl {
Index: clang/include/clang/AST/ASTNodeTraverser.h
===
--- clang/include/clang/AST/ASTNodeTraverser.h
+++ clang/include/clang/AST/ASTNodeTraverser.h
@@ -644,8 +644,15 @@
   }
 
   void VisitFriendDecl(const FriendDecl *D) {
-if (!D->getFriendType())
+if (D->getFriendType()) {
+  // Traverse any CXXRecordDecl owned by this type, since
+  // it will not be in the parent context:
+  if (auto *ET = D->getFriendType()->getType()->getAs())
+if (auto *TD = ET->getOwnedTagDecl())
+  Visit(TD);
+} else {
   Visit(D->getFriendDecl());
+}
   }
 
   void VisitObjCMethodDecl(const ObjCMethodDecl *D) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

[Vincent LE GARREC via Phabricator via cfe-commits]

bansan added a comment.

About my template example: I wanted to say that the actual 
`bugprone-implicit-widening-of-multiplication-result` rule looks to not analyze 
template calculation problem. So I think it's better to use the desugared type 
(`size_t`).

It's not acceptable (IMHO) that the hard-coded `size_t` is resolved as `long`. 
According to https://en.cppreference.com/w/cpp/language/types `long` may be 32 
bits.

Unfortunately, I don't know anything about AST and which function you should 
use :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

> About my template example: I wanted to say that the actual 
> bugprone-implicit-widening-of-multiplication-result rule looks to not analyze 
> template calculation problem. So I think it's better to use the desugared 
> type (size_t).

Hmm, I don't think this two are related.

> It's not acceptable (IMHO) that the hard-coded size_t is resolved as long. 
> According to https://en.cppreference.com/w/cpp/language/types long may be 32 
> bits.

Yeah, I don't have a good feeling about it too. The point is that we love 
desugaring `size_type` to `size_t` (which I haven't found a way to achieve 
yet), and hate desugaring `int64_t` to `long`, but how can we classify them? 
They BOTH are the desugared type.

According to the conversation above, I'd use the qualified alias type name.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D141380: [clang-repl] XFAIL riscv targets in simple-exception test case

[Alex Bradbury via Phabricator via cfe-commits]

asb created this revision.
asb added reviewers: sunho, lhames.
Herald added subscribers: wingo, sunshaoce, pmatos, VincentWu, vkmr, evandro, 
luismarques, sameer.abuasal, s.egerton, Jim, benna, psnobl, PkmX, rogfer01, 
shiva0217, kito-cheng, simoncook, kristof.beyls, arichardson.
Herald added a project: All.
asb requested review of this revision.
Herald added subscribers: pcwang-thead, eopXD.
Herald added a project: clang.

I'm not familiar with this infrastructure so haven't dug into why it's failing, 
but it is failing for me on RISC-V. As Arm targets are also XFAILed it doesn't 
seem like an unreasonable "fix" to XFAIL for RISC-V too - but if it looks like 
an easy fix might be possible from someone who's been working in the area, I'd 
obviously welcome that as an alternative!

For reference, the output of this test is currently:

  clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> clang-repl> 
clang-repl> clang-repl> clang-repl> clang-repl> Running f()
  terminate called after throwing an instance of 'char const*'
  Aborted (core dumped)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141380

Files:
  clang/test/Interpreter/simple-exception.cpp


Index: clang/test/Interpreter/simple-exception.cpp
===
--- clang/test/Interpreter/simple-exception.cpp
+++ clang/test/Interpreter/simple-exception.cpp
@@ -1,7 +1,7 @@
 // clang-format off
 // UNSUPPORTED: system-aix
-// XFAIL for arm and arm64, or running on Windows.
-// XFAIL: target=arm{{.*}}, system-windows
+// XFAIL for arm, arm64, riscv, or running on Windows.
+// XFAIL: target={{(arm|riscv).*}}, system-windows
 // RUN: cat %s | clang-repl | FileCheck %s
 extern "C" int printf(const char *, ...);
 
@@ -11,4 +11,4 @@
 // CHECK: Running f()
 // CHECK-NEXT: Simple exception
 
-%quit
\ No newline at end of file
+%quit


Index: clang/test/Interpreter/simple-exception.cpp
===
--- clang/test/Interpreter/simple-exception.cpp
+++ clang/test/Interpreter/simple-exception.cpp
@@ -1,7 +1,7 @@
 // clang-format off
 // UNSUPPORTED: system-aix
-// XFAIL for arm and arm64, or running on Windows.
-// XFAIL: target=arm{{.*}}, system-windows
+// XFAIL for arm, arm64, riscv, or running on Windows.
+// XFAIL: target={{(arm|riscv).*}}, system-windows
 // RUN: cat %s | clang-repl | FileCheck %s
 extern "C" int printf(const char *, ...);
 
@@ -11,4 +11,4 @@
 // CHECK: Running f()
 // CHECK-NEXT: Simple exception
 
-%quit
\ No newline at end of file
+%quit
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140703: [clang][dataflow] Unify `TransferOptions` and `DataflowAnalysisContext::Options`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 487769.
ymandel added a comment.

use `std::optional`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140703

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
@@ -34,6 +35,8 @@
 using ::testing::NotNull;
 using ::testing::UnorderedElementsAre;
 
+using BuiltinOptions = DataflowAnalysisContext::Options;
+
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  DataflowAnalysisOptions Options,
@@ -44,15 +47,18 @@
   "-fsyntax-only", "-fno-delayed-template-parsing",
   "-std=" +
   std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  auto AI =
-  AnalysisInputs(Code, hasName(TargetFun),
-   [&Options](ASTContext &C, Environment &) {
- return NoopAnalysis(C, Options);
-   })
-  .withASTBuildArgs(ASTBuildArgs);
-  if (Options.BuiltinTransferOpts &&
-  Options.BuiltinTransferOpts->ContextSensitiveOpts)
-(void)std::move(AI).withContextSensitivity();
+  AnalysisInputs AI(
+  Code, hasName(TargetFun),
+  [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
+  Environment &Env) {
+return NoopAnalysis(
+C, DataflowAnalysisOptions{UseBuiltinModel
+   ? Env.getAnalysisOptions()
+   : llvm::Optional()});
+  });
+  AI.ASTBuildArgs = ASTBuildArgs;
+  if (Options.BuiltinOpts)
+AI.BuiltinOptions = *Options.BuiltinOpts;
   ASSERT_THAT_ERROR(
   checkDataflow(
   std::move(AI),
@@ -69,8 +75,8 @@
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
   runDataflow(Code, std::move(Match),
-  {ApplyBuiltinTransfer ? TransferOptions{}
-: llvm::Optional()},
+  {ApplyBuiltinTransfer ? BuiltinOptions{}
+: std::optional()},
   Std, TargetFun);
 }
 
@@ -4123,7 +4129,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {TransferOptions{/*.ContextSensitiveOpts=*/std::nullopt}});
+  {BuiltinOptions{/*.ContextSensitiveOpts=*/std::nullopt}});
 }
 
 // This test is a regression test, based on a real crash.
@@ -4152,7 +4158,7 @@
 
 EXPECT_THAT(Env.getValue(*Loc), IsNull());
   },
-  {TransferOptions{ContextSensitiveOptions{}}});
+  {BuiltinOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveDepthZero) {
@@ -4180,7 +4186,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {TransferOptions{ContextSensitiveOptions{/*.Depth=*/0}}});
+  {BuiltinOptions{ContextSensitiveOptions{/*.Depth=*/0}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTrue) {
@@ -4207,7 +4213,7 @@
 auto &FooVal = *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(FooVal));
   },
-  {TransferOptions{ContextSensitiveOptions{}}});
+  {BuiltinOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetFalse) {
@@ -4234,7 +4240,7 @@
 auto &FooVal = *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {TransferOptions{ContextSensitiveOptions{}}});
+  {BuiltinOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSens

[PATCH] D140703: [clang][dataflow] Unify `TransferOptions` and `DataflowAnalysisContext::Options`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:89
+ ? DataflowAnalysisContext::Options{}
+ : llvm::Optional()}) {}
 

xazax.hun wrote:
> If we already change this code, I think we can consider replacing this with 
> `std::optional` but feel free to ignore.
> If we already change this code, I think we can consider replacing this with 
> `std::optional` but feel free to ignore.

Thanks, I was actually wondering about that. I wasn't sure where llvm/clang was 
in terms of `std::optional` adoption. I'm happy to update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140703

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

[PATCH] D141324: [clang] extend external_source_symbol attribute with the USR clause

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I'm disturbed that the string-literal diagnostic you changed never shows up in 
the tests.  I suspect this attribute needs significantly better test coverage.




Comment at: clang/docs/LanguageExtensions.rst:4815
+
+An optional USR string literal can be added to the ``external_source_symbol``
+attribute. For example:

Can you explain in here what the "USR" is here/means here?  Or what said string 
is supposed to be?



Comment at: clang/docs/LanguageExtensions.rst:4823
+Query for this feature with
+``__has_feature(attribute_external_source_symbol_with_usr)``.

`__has_feature` isn't really the right solution here.  Though, we don't 
typically have a way to test for individual features of an attribute other than 
the value that `__has_attribute` returns.  In standard attributes, it returns a 
larger number when new features are added (see SD-10's Feature Test Macros).

I don't think I want us using `__has_feature` here though, either a different 
value for `__has_attribute` might be OK, or alternatively, perhaps just a 
simple feature-test-macro.



Comment at: clang/include/clang/Basic/AttrDocs.td:1754
+USR=\ *string-literal*
+  The exact USR of the foreign symbol. When not specified, Clang's
+  indexer will use the Clang USR for this symbol.

This DEFINITELY needs to better explain what a USR is, and why one would use 
it.  Many times this document is used by folks to discover attributes they 
didn't know existed, and this needs to be usable for someone who doesn't really 
get the context until reading this document.



Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:58
   "for optional message in 'availability' attribute|"
-  "for %select{language|source container}1 name in "
+  "for %select{language name|source container name|USR}1 in "
   "'external_source_symbol' attribute}0">;

I don't think adding 'name' to these makes them particularly more readable?  At 
least from what I can see reading it here.  


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

https://reviews.llvm.org/D141324

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

Re: [clang-tools-extra] ef9aa34 - Revert "Remove the ThreadLocal template from LLVM."

[Roman Lebedev via cfe-commits]

Reminder to please always mention the reason for the revert/recommit
in the commit message.

On Tue, Jan 10, 2023 at 7:22 AM Owen Anderson via cfe-commits
 wrote:
>
>
> Author: Owen Anderson
> Date: 2023-01-09T21:21:38-07:00
> New Revision: ef9aa34f0274cdbfa82c47f8ab99f02679fd0d13
>
> URL: 
> https://github.com/llvm/llvm-project/commit/ef9aa34f0274cdbfa82c47f8ab99f02679fd0d13
> DIFF: 
> https://github.com/llvm/llvm-project/commit/ef9aa34f0274cdbfa82c47f8ab99f02679fd0d13.diff
>
> LOG: Revert "Remove the ThreadLocal template from LLVM."
>
> This reverts commit 54d78b639b9c18b42abd4fac5c6e76105f06b3ef.
>
> Added:
> llvm/include/llvm/Support/ThreadLocal.h
> llvm/lib/Support/ThreadLocal.cpp
> llvm/lib/Support/Unix/ThreadLocal.inc
> llvm/lib/Support/Windows/ThreadLocal.inc
> llvm/unittests/Support/ThreadLocalTest.cpp
>
> Modified:
> clang-tools-extra/clangd/support/ThreadCrashReporter.cpp
> llvm/lib/Support/CMakeLists.txt
> llvm/lib/Support/CrashRecoveryContext.cpp
> llvm/unittests/Support/CMakeLists.txt
> mlir/include/mlir/Support/ThreadLocalCache.h
>
> Removed:
>
>
>
> 
> diff  --git a/clang-tools-extra/clangd/support/ThreadCrashReporter.cpp 
> b/clang-tools-extra/clangd/support/ThreadCrashReporter.cpp
> index 9551bbfda43c1..05afb3b25f28e 100644
> --- a/clang-tools-extra/clangd/support/ThreadCrashReporter.cpp
> +++ b/clang-tools-extra/clangd/support/ThreadCrashReporter.cpp
> @@ -7,6 +7,7 @@
>  
> //===--===//
>
>  #include "support/ThreadCrashReporter.h"
> +#include "llvm/Support/ThreadLocal.h"
>  #include 
>
>  namespace clang {
>
> diff  --git a/llvm/include/llvm/Support/ThreadLocal.h 
> b/llvm/include/llvm/Support/ThreadLocal.h
> new file mode 100644
> index 0..d6838c15fc345
> --- /dev/null
> +++ b/llvm/include/llvm/Support/ThreadLocal.h
> @@ -0,0 +1,62 @@
> +//===- llvm/Support/ThreadLocal.h - Thread Local Data *- C++ 
> -*-===//
> +//
> +// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
> Exceptions.
> +// See https://llvm.org/LICENSE.txt for license information.
> +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
> +//
> +//===--===//
> +//
> +// This file declares the llvm::sys::ThreadLocal class.
> +//
> +//===--===//
> +
> +#ifndef LLVM_SUPPORT_THREADLOCAL_H
> +#define LLVM_SUPPORT_THREADLOCAL_H
> +
> +#include "llvm/Support/DataTypes.h"
> +#include "llvm/Support/Threading.h"
> +#include 
> +
> +namespace llvm {
> +  namespace sys {
> +// ThreadLocalImpl - Common base class of all ThreadLocal instantiations.
> +// YOU SHOULD NEVER USE THIS DIRECTLY.
> +class ThreadLocalImpl {
> +  typedef uint64_t ThreadLocalDataTy;
> +  /// Platform-specific thread local data.
> +  ///
> +  /// This is embedded in the class and we avoid malloc'ing/free'ing it,
> +  /// to make this class more safe for use along with 
> CrashRecoveryContext.
> +  union {
> +char data[sizeof(ThreadLocalDataTy)];
> +ThreadLocalDataTy align_data;
> +  };
> +public:
> +  ThreadLocalImpl();
> +  virtual ~ThreadLocalImpl();
> +  void setInstance(const void* d);
> +  void *getInstance();
> +  void removeInstance();
> +};
> +
> +/// ThreadLocal - A class used to abstract thread-local storage.  It 
> holds,
> +/// for each thread, a pointer a single object of type T.
> +template
> +class ThreadLocal : public ThreadLocalImpl {
> +public:
> +  ThreadLocal() : ThreadLocalImpl() { }
> +
> +  /// get - Fetches a pointer to the object associated with the current
> +  /// thread.  If no object has yet been associated, it returns NULL;
> +  T* get() { return static_cast(getInstance()); }
> +
> +  // set - Associates a pointer to an object with the current thread.
> +  void set(T* d) { setInstance(d); }
> +
> +  // erase - Removes the pointer associated with the current thread.
> +  void erase() { removeInstance(); }
> +};
> +  }
> +}
> +
> +#endif
>
> diff  --git a/llvm/lib/Support/CMakeLists.txt 
> b/llvm/lib/Support/CMakeLists.txt
> index 9b5402fa54f0f..46469c6e9e624 100644
> --- a/llvm/lib/Support/CMakeLists.txt
> +++ b/llvm/lib/Support/CMakeLists.txt
> @@ -260,6 +260,7 @@ add_llvm_component_library(LLVMSupport
>Program.cpp
>RWMutex.cpp
>Signals.cpp
> +  ThreadLocal.cpp
>Threading.cpp
>Valgrind.cpp
>Watchdog.cpp
>
> diff  --git a/llvm/lib/Support/CrashRecoveryContext.cpp 
> b/llvm/lib/Support/CrashRecoveryContext.cpp
> index 9e792d1f51777..c7c384c9edc22 100644
> --- a/llvm/lib/Support/CrashRecoveryContext.cpp
> +++ b/llvm/lib/Support/CrashRecoveryContext.cpp
> @@ -11,6 +11,7 @@
>  #include "llvm/Support/ErrorHandling

[clang] 264976d - [clang][dataflow] Unify `TransferOptions` and `DataflowAnalysisContext::Options`.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2023-01-10T14:17:25Z
New Revision: 264976d98e785fa061ce6ac06db4a9bda2590506

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

LOG: [clang][dataflow] Unify `TransferOptions` and 
`DataflowAnalysisContext::Options`.

Merges `TransferOptions` into the newly-introduced
`DataflowAnalysisContext::Options` and removes explicit parameter for
`TransferOptions`, relying instead on the common options carried by the analysis
context. Given that there was no intent to allow different options between calls
to `transfer`, a common value for the options is preferable.

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Transfer.h
clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index 23bb6d66cc471..62ab9899206b8 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -82,9 +82,11 @@ class DataflowAnalysis : public TypeErasedDataflowAnalysis {
 
   /// Deprecated. Use the `DataflowAnalysisOptions` constructor instead.
   explicit DataflowAnalysis(ASTContext &Context, bool ApplyBuiltinTransfer)
-  : DataflowAnalysis(Context, {ApplyBuiltinTransfer
-   ? TransferOptions{}
-   : llvm::Optional()}) {}
+  : DataflowAnalysis(
+Context,
+{ApplyBuiltinTransfer
+ ? DataflowAnalysisContext::Options{}
+ : std::optional()}) {}
 
   explicit DataflowAnalysis(ASTContext &Context,
 DataflowAnalysisOptions Options)

diff  --git 
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index 98135508aabb4..cd4a48243785b 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -50,13 +50,22 @@ const Stmt &ignoreCFGOmittedNodes(const Stmt &S);
 /// Returns the set of all fields in the type.
 llvm::DenseSet getObjectFields(QualType Type);
 
+struct ContextSensitiveOptions {
+  /// The maximum depth to analyze. A value of zero is equivalent to disabling
+  /// context-sensitive analysis entirely.
+  unsigned Depth = 2;
+};
+
 /// Owns objects that encompass the state of a program and stores context that
 /// is used during dataflow analysis.
 class DataflowAnalysisContext {
 public:
-  // FIXME: merge with TransferOptions from Transfer.h.
   struct Options {
-bool EnableContextSensitiveAnalysis;
+/// Options for analyzing function bodies when present in the translation
+/// unit, or empty to disable context-sensitive analysis. Note that this is
+/// fundamentally limited: some constructs, such as recursion, are
+/// explicitly unsupported.
+llvm::Optional ContextSensitiveOpts;
   };
 
   /// Constructs a dataflow analysis context.
@@ -65,10 +74,10 @@ class DataflowAnalysisContext {
   ///
   ///  `S` must not be null.
   DataflowAnalysisContext(std::unique_ptr S,
-  Options Opts = {
-  /*EnableContextSensitiveAnalysis=*/false})
+  Options Opts = Options{
+  /*ContextSensitiveOpts=*/std::nullopt})
   : S(std::move(S)), TrueVal(createAtomicBoolValue()),
-FalseVal(createAtomicBoolValue()), Options(Opts) {
+FalseVal(createAtomicBoolValue()), Opts(Opts) {
 assert(this->S != nullptr);
   }
 
@@ -262,6 +271,8 @@ class DataflowAnalysisContext {
 
   void addFieldsReferencedInScope(llvm::DenseSet Fields);
 
+  const Options &getOptions() { return Opts; }
+
 private:
   friend class Environment;
 
@@ -345,7 +356,7 @@ class DataflowAnalysisContext {
   AtomicBoolValue &TrueVal;
   AtomicBoolValue &FalseVal;
 
-  Options Options;
+  Options Opts;
 
   // Indices that are used to avoid recreating the same composite boolean
   // values.

diff  --git a/clang/includ

[PATCH] D140703: [clang][dataflow] Unify `TransferOptions` and `DataflowAnalysisContext::Options`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG264976d98e78: [clang][dataflow] Unify `TransferOptions` and 
`DataflowAnalysisContext… (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140703

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -10,6 +10,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
@@ -34,6 +35,8 @@
 using ::testing::NotNull;
 using ::testing::UnorderedElementsAre;
 
+using BuiltinOptions = DataflowAnalysisContext::Options;
+
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  DataflowAnalysisOptions Options,
@@ -44,15 +47,18 @@
   "-fsyntax-only", "-fno-delayed-template-parsing",
   "-std=" +
   std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  auto AI =
-  AnalysisInputs(Code, hasName(TargetFun),
-   [&Options](ASTContext &C, Environment &) {
- return NoopAnalysis(C, Options);
-   })
-  .withASTBuildArgs(ASTBuildArgs);
-  if (Options.BuiltinTransferOpts &&
-  Options.BuiltinTransferOpts->ContextSensitiveOpts)
-(void)std::move(AI).withContextSensitivity();
+  AnalysisInputs AI(
+  Code, hasName(TargetFun),
+  [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
+  Environment &Env) {
+return NoopAnalysis(
+C, DataflowAnalysisOptions{UseBuiltinModel
+   ? Env.getAnalysisOptions()
+   : llvm::Optional()});
+  });
+  AI.ASTBuildArgs = ASTBuildArgs;
+  if (Options.BuiltinOpts)
+AI.BuiltinOptions = *Options.BuiltinOpts;
   ASSERT_THAT_ERROR(
   checkDataflow(
   std::move(AI),
@@ -69,8 +75,8 @@
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
   runDataflow(Code, std::move(Match),
-  {ApplyBuiltinTransfer ? TransferOptions{}
-: llvm::Optional()},
+  {ApplyBuiltinTransfer ? BuiltinOptions{}
+: std::optional()},
   Std, TargetFun);
 }
 
@@ -4123,7 +4129,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {TransferOptions{/*.ContextSensitiveOpts=*/std::nullopt}});
+  {BuiltinOptions{/*.ContextSensitiveOpts=*/std::nullopt}});
 }
 
 // This test is a regression test, based on a real crash.
@@ -4152,7 +4158,7 @@
 
 EXPECT_THAT(Env.getValue(*Loc), IsNull());
   },
-  {TransferOptions{ContextSensitiveOptions{}}});
+  {BuiltinOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveDepthZero) {
@@ -4180,7 +4186,7 @@
 EXPECT_FALSE(Env.flowConditionImplies(FooVal));
 EXPECT_FALSE(Env.flowConditionImplies(Env.makeNot(FooVal)));
   },
-  {TransferOptions{ContextSensitiveOptions{/*.Depth=*/0}}});
+  {BuiltinOptions{ContextSensitiveOptions{/*.Depth=*/0}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetTrue) {
@@ -4207,7 +4213,7 @@
 auto &FooVal = *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(FooVal));
   },
-  {TransferOptions{ContextSensitiveOptions{}}});
+  {BuiltinOptions{ContextSensitiveOptions{}}});
 }
 
 TEST(TransferTest, ContextSensitiveSetFalse) {
@@ -4234,7 +4240,7 @@
 auto &FooVal = *cast(Env.getValue(*FooDecl, SkipPast::None));
 EXPECT_TRUE(Env.flowConditionImplies(Env.makeNo

[PATCH] D137381: [clang][compiler-rt] Exception escape out of an non-unwinding function is an undefined behaviour

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D137381#4038716 , @MaskRay wrote:

> I am unfamiliar with Clang codegen so I am just commenting about what a user 
> may feel about this feature.
>
> `compiler-rt/test/ubsan/TestCases/Misc/exception-escape.cpp` cannot 
> demonstrate the value of the new UBSan check `-fsanitize=exception-escape`, 
> as the executable will fail without the option (Clang emits `call` instead of 
> `invoke` for a nounwind function call, and there is no LSDA information, 
> libgcc/libunwind will call terminate; it's trivial to get more diagnostic 
> with a SIBABRT signal handler).
> To demonstrate the value, `footgun` and its caller need to be in different 
> TUs and the caller does not know `footgun` is nounwind.
> In such a setup, I think people likely care more about 
> `-fno-exceptions`/`-fexceptions` exception propagation instead of 
> `__attribute__((pure))` and find `-fsanitize=exception-escape` not do what it 
> may advertise.
>
> The more common `-fno-exceptions`/`-fexceptions` exception propagation 
> dilemma can be detected today with `-fno-asynchronous-unwind-tables`.

Thank you for taking a look. I'm afraid this review comment is not based on 
reality, so i'm forced to ignore it.
The whole point of the clang change is to *NOT* lower such calls that we "know" 
will not unwind to a `call`, but into an `invoke`, with sanitization in the 
`landingpad`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137381

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

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

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487778.
ccotter added a comment.

rebase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140307

Files:
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -216,7 +216,7 @@
   :doc: `readability-redundant-string-cstr 
`
   check.
 
-- Improved :doc:`misc-redundant-expression 
`
+- Improved :doc:`modernize-loop-convert 
`
   to check for container functions ``begin``/``end`` etc on base classes of 
the container
   type, instead of only as direct members of the container type itself.
 


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -216,7 +216,7 @@
   :doc: `readability-redundant-string-cstr `
   check.
 
-- Improved :doc:`misc-redundant-expression `
+- Improved :doc:`modernize-loop-convert `
   to check for container functions ``begin``/``end`` etc on base classes of the container
   type, instead of only as direct members of the container type itself.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 089a544 - [clang][dataflow][NFC] Refine names and comments for field filtering.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2023-01-10T14:28:45Z
New Revision: 089a54469f63c2f3c4b9d79fb9694f21bef0d071

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

LOG: [clang][dataflow][NFC] Refine names and comments for field filtering.

Tweaks elements of the new API for filtering the set of modeled fields.

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Removed: 




diff  --git 
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index cd4a48243785b..72ddfbb780f17 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -269,8 +269,6 @@ class DataflowAnalysisContext {
   /// returns null.
   const ControlFlowContext *getControlFlowContext(const FunctionDecl *F);
 
-  void addFieldsReferencedInScope(llvm::DenseSet Fields);
-
   const Options &getOptions() { return Opts; }
 
 private:
@@ -287,8 +285,11 @@ class DataflowAnalysisContext {
 using DenseMapInfo::isEqual;
   };
 
-  /// Returns the subset of fields of `Type` that are referenced in the scope 
of
-  /// the analysis.
+  // Extends the set of modeled field declarations.
+  void addModeledFields(const llvm::DenseSet &Fields);
+
+  /// Returns the fields of `Type`, limited to the set of fields modeled by 
this
+  /// context.
   llvm::DenseSet getReferencedFields(QualType Type);
 
   /// Adds all constraints of the flow condition identified by `Token` and all
@@ -388,8 +389,8 @@ class DataflowAnalysisContext {
 
   llvm::DenseMap FunctionContexts;
 
-  // All fields referenced (statically) in the scope of the analysis.
-  llvm::DenseSet FieldsReferencedInScope;
+  // Fields modeled by environments covered by this context.
+  llvm::DenseSet ModeledFields;
 };
 
 } // namespace dataflow

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index 463230516185f..480606bdac8d8 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -25,15 +25,15 @@
 namespace clang {
 namespace dataflow {
 
-void DataflowAnalysisContext::addFieldsReferencedInScope(
-llvm::DenseSet Fields) {
-  llvm::set_union(FieldsReferencedInScope, Fields);
+void DataflowAnalysisContext::addModeledFields(
+const llvm::DenseSet &Fields) {
+  llvm::set_union(ModeledFields, Fields);
 }
 
 llvm::DenseSet
 DataflowAnalysisContext::getReferencedFields(QualType Type) {
   llvm::DenseSet Fields = getObjectFields(Type);
-  llvm::set_intersect(Fields, FieldsReferencedInScope);
+  llvm::set_intersect(Fields, ModeledFields);
   return Fields;
 }
 
@@ -46,7 +46,7 @@ StorageLocation 
&DataflowAnalysisContext::createStorageLocation(QualType Type) {
 // the allocation. Since we only collect fields used in the function where
 // the allocation occurs, we can't apply that filter when performing
 // context-sensitive analysis. But, this only applies to storage locations,
-// since fields access it not allowed to fail. In contrast, field *values*
+// since field access it not allowed to fail. In contrast, field *values*
 // don't need this allowance, since the API allows for uninitialized 
fields.
 auto Fields = Opts.ContextSensitiveOpts ? getObjectFields(Type)
 : getReferencedFields(Type);

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index c4c3d0be229cd..7d10a75b36e3e 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -253,7 +253,7 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
 initVars(Vars);
 // These have to be set before the lines that follow to ensure that create*
 // work correctly for structs.
-DACtx.addFieldsReferencedInScope(std::move(Fields));
+DACtx.addModeledFields(Fields);
 
 for (const auto *ParamDecl : FuncDecl->parameters()) {
   assert(ParamDecl != nullptr);
@@ -342,7 +342,7 @@ void Environment::pushCallInternal(const FunctionDecl 
*FuncDecl,
   }
   getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
   initVars(Vars);
-  DACtx->addFieldsReferencedInScope(std::move(Fields));
+  DACtx->addModeledFields(Fields);
 
   const auto *ParamIt = FuncDecl->param_begin();
 



___

[PATCH] D141319: [clang][dataflow][NFC] Refine names and comments for field filtering.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG089a54469f63: [clang][dataflow][NFC] Refine names and 
comments for field filtering. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141319

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


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -253,7 +253,7 @@
 initVars(Vars);
 // These have to be set before the lines that follow to ensure that create*
 // work correctly for structs.
-DACtx.addFieldsReferencedInScope(std::move(Fields));
+DACtx.addModeledFields(Fields);
 
 for (const auto *ParamDecl : FuncDecl->parameters()) {
   assert(ParamDecl != nullptr);
@@ -342,7 +342,7 @@
   }
   getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
   initVars(Vars);
-  DACtx->addFieldsReferencedInScope(std::move(Fields));
+  DACtx->addModeledFields(Fields);
 
   const auto *ParamIt = FuncDecl->param_begin();
 
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -25,15 +25,15 @@
 namespace clang {
 namespace dataflow {
 
-void DataflowAnalysisContext::addFieldsReferencedInScope(
-llvm::DenseSet Fields) {
-  llvm::set_union(FieldsReferencedInScope, Fields);
+void DataflowAnalysisContext::addModeledFields(
+const llvm::DenseSet &Fields) {
+  llvm::set_union(ModeledFields, Fields);
 }
 
 llvm::DenseSet
 DataflowAnalysisContext::getReferencedFields(QualType Type) {
   llvm::DenseSet Fields = getObjectFields(Type);
-  llvm::set_intersect(Fields, FieldsReferencedInScope);
+  llvm::set_intersect(Fields, ModeledFields);
   return Fields;
 }
 
@@ -46,7 +46,7 @@
 // the allocation. Since we only collect fields used in the function where
 // the allocation occurs, we can't apply that filter when performing
 // context-sensitive analysis. But, this only applies to storage locations,
-// since fields access it not allowed to fail. In contrast, field *values*
+// since field access it not allowed to fail. In contrast, field *values*
 // don't need this allowance, since the API allows for uninitialized 
fields.
 auto Fields = Opts.ContextSensitiveOpts ? getObjectFields(Type)
 : getReferencedFields(Type);
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -269,8 +269,6 @@
   /// returns null.
   const ControlFlowContext *getControlFlowContext(const FunctionDecl *F);
 
-  void addFieldsReferencedInScope(llvm::DenseSet Fields);
-
   const Options &getOptions() { return Opts; }
 
 private:
@@ -287,8 +285,11 @@
 using DenseMapInfo::isEqual;
   };
 
-  /// Returns the subset of fields of `Type` that are referenced in the scope 
of
-  /// the analysis.
+  // Extends the set of modeled field declarations.
+  void addModeledFields(const llvm::DenseSet &Fields);
+
+  /// Returns the fields of `Type`, limited to the set of fields modeled by 
this
+  /// context.
   llvm::DenseSet getReferencedFields(QualType Type);
 
   /// Adds all constraints of the flow condition identified by `Token` and all
@@ -388,8 +389,8 @@
 
   llvm::DenseMap FunctionContexts;
 
-  // All fields referenced (statically) in the scope of the analysis.
-  llvm::DenseSet FieldsReferencedInScope;
+  // Fields modeled by environments covered by this context.
+  llvm::DenseSet ModeledFields;
 };
 
 } // namespace dataflow


Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -253,7 +253,7 @@
 initVars(Vars);
 // These have to be set before the lines that follow to ensure that create*
 // work correctly for structs.
-DACtx.addFieldsReferencedInScope(std::move(Fields));
+DACtx.addModeledFields(Fields);
 
 for (const auto *ParamDecl : FuncDecl->parameters()) {
   assert(ParamDecl != nullptr);
@@ -342,7 +342,7 @@
   }
   getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
   initVars(Vars);
-  DACtx->addFieldsReferencedInScope(st

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[Felipe de Azevedo Piovezan via Phabricator via cfe-commits]

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

With codegen prior to this patch, truly indirect arguments -- i.e.
those that are not `byval` -- can have their debug information lost even
at O0. Because indirect arguments are passed by pointer, and this
pointer is likely placed in a register as per the function call ABI,
debug information is lost as soon as the register gets clobbered.

This patch solves the issue by storing the address of the parameter on
the stack, using a similar strategy employed when C++ references are
passed. In other words, this patch changes codegen from:

  define @foo(ptr %arg) {
 call void @llvm.dbg.declare(%arg, [...], metadata !DIExpression())

To:

  define @foo(ptr %arg) {
 %ptr_storage = alloca ptr
 store ptr %arg, ptr %ptr_storage
 call void @llvm.dbg.declare(%ptr_storage, [...], metadata 
!DIExpression(DW_OP_deref))

Some common cases where this may happen with C or C++ function calls:

1. "Big enough" trivial structures passed by value under the ARM ABI.
2. Structures that are non-trivial for the purposes of call (as per the Itanium 
ABI) when passed by value.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141381

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/debug-info.cpp

Index: clang/test/CodeGenCXX/debug-info.cpp
===
--- clang/test/CodeGenCXX/debug-info.cpp
+++ clang/test/CodeGenCXX/debug-info.cpp
@@ -4,7 +4,13 @@
 // CHECK: @_ZN6pr96081xE ={{.*}} global ptr null, align 8, !dbg [[X:![0-9]+]]
 
 // CHECK: define{{.*}} void @_ZN7pr147634funcENS_3fooE
-// CHECK: call void @llvm.dbg.declare({{.*}}, metadata ![[F:[0-9]+]], metadata !DIExpression())
+// CHECK-SAME: ptr noundef [[param:%.*]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   alloca ptr, align 8
+// CHECK-NEXT:   [[param_addr_storage:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:   store
+// CHECK-NEXT:   store ptr [[param]], ptr [[param_addr_storage]], align 8
+// CHECK-NEXT:   call void @llvm.dbg.declare(metadata ptr [[param_addr_storage]], metadata ![[F:[0-9]+]], metadata !DIExpression(DW_OP_deref))
 
 // !llvm.dbg.cu pulls in globals and their types first.
 // CHECK-NOT: !DIGlobalVariable(name: "c"
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -2475,6 +2475,13 @@
   Address AllocaPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
+  bool UseIndirectDebugAddress = false;
+  const bool ShouldEmitDebugInfo = [&] {
+// Emit debug info for param declarations in non-thunk functions.
+return getDebugInfo() && CGM.getCodeGenOpts().hasReducedDebugInfo() &&
+   !CurFuncIsThunk && !NoDebugInfo;
+  }();
+
   // If we already have a pointer to the argument, reuse the input pointer.
   if (Arg.isIndirect()) {
 // If we have a prettier pointer type at this point, bitcast to that.
@@ -2486,6 +2493,19 @@
 auto AllocaAS = CGM.getASTAllocaAddressSpace();
 auto *V = DeclPtr.getPointer();
 AllocaPtr = DeclPtr;
+
+// For truly ABI indirect arguments, that is, they are not `byval`, store
+// the address of the argument on the stack to preserve debug information.
+ABIArgInfo ArgInfo = CurFnInfo->arguments()[ArgNo - 1].info;
+if (ShouldEmitDebugInfo && ArgInfo.isIndirect())
+  UseIndirectDebugAddress = !ArgInfo.getIndirectByVal();
+if (UseIndirectDebugAddress) {
+  auto PtrTy = getContext().getPointerType(Ty);
+  AllocaPtr = CreateMemTemp(PtrTy, getContext().getTypeAlignInChars(PtrTy),
+D.getName() + ".indirect_addr");
+  EmitStoreOfScalar(V, AllocaPtr, /* Volatile */ false, PtrTy);
+}
+
 auto SrcLangAS = getLangOpts().OpenCL ? LangAS::opencl_private : AllocaAS;
 auto DestLangAS =
 getLangOpts().OpenCL ? LangAS::opencl_private : LangAS::Default;
@@ -2597,15 +2617,13 @@
 
   setAddrOfLocalVar(&D, DeclPtr);
 
-  // Emit debug info for param declarations in non-thunk functions.
-  if (CGDebugInfo *DI = getDebugInfo()) {
-if (CGM.getCodeGenOpts().hasReducedDebugInfo() && !CurFuncIsThunk &&
-!NoDebugInfo) {
-  llvm::DILocalVariable *DILocalVar = DI->EmitDeclareOfArgVariable(
-  &D, AllocaPtr.getPointer(), ArgNo, Builder);
-  if (const auto *Var = dyn_cast_or_null(&D))
-DI->getParamDbgMappings().insert({Var, DILocalVar});
-}
+  if (ShouldEmitDebugInfo) {
+CGDebugInfo *DI = getDebugInfo();
+assert(DI);
+llvm::DILocalVariable *DILocalVar = DI->EmitDeclareOfArgVariable(
+&D, AllocaPtr.getPointer(), ArgNo, Builder, UseIndirectDebugAddress);
+if (c

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

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487781.
ccotter added a comment.

rebase?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140307

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


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


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

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

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

Rebased! Sorry for the multiple diffs on this phab - I'm still getting the hang 
of `arc diff`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140307

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

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[Felipe de Azevedo Piovezan via Phabricator via cfe-commits]

fdeazeve updated this revision to Diff 487782.
fdeazeve added a comment.

Simplified condition spelling


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/debug-info.cpp

Index: clang/test/CodeGenCXX/debug-info.cpp
===
--- clang/test/CodeGenCXX/debug-info.cpp
+++ clang/test/CodeGenCXX/debug-info.cpp
@@ -4,7 +4,13 @@
 // CHECK: @_ZN6pr96081xE ={{.*}} global ptr null, align 8, !dbg [[X:![0-9]+]]
 
 // CHECK: define{{.*}} void @_ZN7pr147634funcENS_3fooE
-// CHECK: call void @llvm.dbg.declare({{.*}}, metadata ![[F:[0-9]+]], metadata !DIExpression())
+// CHECK-SAME: ptr noundef [[param:%.*]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   alloca ptr, align 8
+// CHECK-NEXT:   [[param_addr_storage:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:   store
+// CHECK-NEXT:   store ptr [[param]], ptr [[param_addr_storage]], align 8
+// CHECK-NEXT:   call void @llvm.dbg.declare(metadata ptr [[param_addr_storage]], metadata ![[F:[0-9]+]], metadata !DIExpression(DW_OP_deref))
 
 // !llvm.dbg.cu pulls in globals and their types first.
 // CHECK-NOT: !DIGlobalVariable(name: "c"
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -2475,6 +2475,12 @@
   Address AllocaPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
+  bool UseIndirectDebugAddress = false;
+  // Emit debug info for param declarations in non-thunk functions.
+  const bool ShouldEmitDebugInfo = getDebugInfo() &&
+   CGM.getCodeGenOpts().hasReducedDebugInfo() &&
+   !CurFuncIsThunk && !NoDebugInfo;
+
   // If we already have a pointer to the argument, reuse the input pointer.
   if (Arg.isIndirect()) {
 // If we have a prettier pointer type at this point, bitcast to that.
@@ -2486,6 +2492,19 @@
 auto AllocaAS = CGM.getASTAllocaAddressSpace();
 auto *V = DeclPtr.getPointer();
 AllocaPtr = DeclPtr;
+
+// For truly ABI indirect arguments, that is, they are not `byval`, store
+// the address of the argument on the stack to preserve debug information.
+ABIArgInfo ArgInfo = CurFnInfo->arguments()[ArgNo - 1].info;
+if (ShouldEmitDebugInfo && ArgInfo.isIndirect())
+  UseIndirectDebugAddress = !ArgInfo.getIndirectByVal();
+if (UseIndirectDebugAddress) {
+  auto PtrTy = getContext().getPointerType(Ty);
+  AllocaPtr = CreateMemTemp(PtrTy, getContext().getTypeAlignInChars(PtrTy),
+D.getName() + ".indirect_addr");
+  EmitStoreOfScalar(V, AllocaPtr, /* Volatile */ false, PtrTy);
+}
+
 auto SrcLangAS = getLangOpts().OpenCL ? LangAS::opencl_private : AllocaAS;
 auto DestLangAS =
 getLangOpts().OpenCL ? LangAS::opencl_private : LangAS::Default;
@@ -2597,15 +2616,13 @@
 
   setAddrOfLocalVar(&D, DeclPtr);
 
-  // Emit debug info for param declarations in non-thunk functions.
-  if (CGDebugInfo *DI = getDebugInfo()) {
-if (CGM.getCodeGenOpts().hasReducedDebugInfo() && !CurFuncIsThunk &&
-!NoDebugInfo) {
-  llvm::DILocalVariable *DILocalVar = DI->EmitDeclareOfArgVariable(
-  &D, AllocaPtr.getPointer(), ArgNo, Builder);
-  if (const auto *Var = dyn_cast_or_null(&D))
-DI->getParamDbgMappings().insert({Var, DILocalVar});
-}
+  if (ShouldEmitDebugInfo) {
+CGDebugInfo *DI = getDebugInfo();
+assert(DI);
+llvm::DILocalVariable *DILocalVar = DI->EmitDeclareOfArgVariable(
+&D, AllocaPtr.getPointer(), ArgNo, Builder, UseIndirectDebugAddress);
+if (const auto *Var = dyn_cast_or_null(&D))
+  DI->getParamDbgMappings().insert({Var, DILocalVar});
   }
 
   if (D.hasAttr())
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -488,10 +488,10 @@
 
   /// Emit call to \c llvm.dbg.declare for an argument variable
   /// declaration.
-  llvm::DILocalVariable *EmitDeclareOfArgVariable(const VarDecl *Decl,
-  llvm::Value *AI,
-  unsigned ArgNo,
-  CGBuilderTy &Builder);
+  llvm::DILocalVariable *
+  EmitDeclareOfArgVariable(const VarDecl *Decl, llvm::Value *AI, unsigned ArgNo,
+   CGBuilderTy &Builder,
+   const bool UsePointerValue = false);
 
   /// Emit call to \c llvm.dbg.declare for the block-literal argument
   /// to a block invocation function.
Index: clang/lib/CodeGen/CGDebugInfo.cpp
=

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[Felipe de Azevedo Piovezan via Phabricator via cfe-commits]

fdeazeve updated this revision to Diff 487784.
fdeazeve added a comment.

Improved comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGDebugInfo.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/test/CodeGenCXX/debug-info.cpp

Index: clang/test/CodeGenCXX/debug-info.cpp
===
--- clang/test/CodeGenCXX/debug-info.cpp
+++ clang/test/CodeGenCXX/debug-info.cpp
@@ -4,7 +4,13 @@
 // CHECK: @_ZN6pr96081xE ={{.*}} global ptr null, align 8, !dbg [[X:![0-9]+]]
 
 // CHECK: define{{.*}} void @_ZN7pr147634funcENS_3fooE
-// CHECK: call void @llvm.dbg.declare({{.*}}, metadata ![[F:[0-9]+]], metadata !DIExpression())
+// CHECK-SAME: ptr noundef [[param:%.*]])
+// CHECK-NEXT: entry:
+// CHECK-NEXT:   alloca ptr, align 8
+// CHECK-NEXT:   [[param_addr_storage:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:   store
+// CHECK-NEXT:   store ptr [[param]], ptr [[param_addr_storage]], align 8
+// CHECK-NEXT:   call void @llvm.dbg.declare(metadata ptr [[param_addr_storage]], metadata ![[F:[0-9]+]], metadata !DIExpression(DW_OP_deref))
 
 // !llvm.dbg.cu pulls in globals and their types first.
 // CHECK-NOT: !DIGlobalVariable(name: "c"
Index: clang/lib/CodeGen/CGDecl.cpp
===
--- clang/lib/CodeGen/CGDecl.cpp
+++ clang/lib/CodeGen/CGDecl.cpp
@@ -2475,6 +2475,12 @@
   Address AllocaPtr = Address::invalid();
   bool DoStore = false;
   bool IsScalar = hasScalarEvaluationKind(Ty);
+  bool UseIndirectDebugAddress = false;
+  // Emit debug info for param declarations in non-thunk functions.
+  const bool ShouldEmitDebugInfo = getDebugInfo() &&
+   CGM.getCodeGenOpts().hasReducedDebugInfo() &&
+   !CurFuncIsThunk && !NoDebugInfo;
+
   // If we already have a pointer to the argument, reuse the input pointer.
   if (Arg.isIndirect()) {
 // If we have a prettier pointer type at this point, bitcast to that.
@@ -2486,6 +2492,19 @@
 auto AllocaAS = CGM.getASTAllocaAddressSpace();
 auto *V = DeclPtr.getPointer();
 AllocaPtr = DeclPtr;
+
+// For truly ABI indirect arguments -- those that are not `byval` -- store
+// the address of the argument on the stack to preserve debug information.
+ABIArgInfo ArgInfo = CurFnInfo->arguments()[ArgNo - 1].info;
+if (ShouldEmitDebugInfo && ArgInfo.isIndirect())
+  UseIndirectDebugAddress = !ArgInfo.getIndirectByVal();
+if (UseIndirectDebugAddress) {
+  auto PtrTy = getContext().getPointerType(Ty);
+  AllocaPtr = CreateMemTemp(PtrTy, getContext().getTypeAlignInChars(PtrTy),
+D.getName() + ".indirect_addr");
+  EmitStoreOfScalar(V, AllocaPtr, /* Volatile */ false, PtrTy);
+}
+
 auto SrcLangAS = getLangOpts().OpenCL ? LangAS::opencl_private : AllocaAS;
 auto DestLangAS =
 getLangOpts().OpenCL ? LangAS::opencl_private : LangAS::Default;
@@ -2597,15 +2616,13 @@
 
   setAddrOfLocalVar(&D, DeclPtr);
 
-  // Emit debug info for param declarations in non-thunk functions.
-  if (CGDebugInfo *DI = getDebugInfo()) {
-if (CGM.getCodeGenOpts().hasReducedDebugInfo() && !CurFuncIsThunk &&
-!NoDebugInfo) {
-  llvm::DILocalVariable *DILocalVar = DI->EmitDeclareOfArgVariable(
-  &D, AllocaPtr.getPointer(), ArgNo, Builder);
-  if (const auto *Var = dyn_cast_or_null(&D))
-DI->getParamDbgMappings().insert({Var, DILocalVar});
-}
+  if (ShouldEmitDebugInfo) {
+CGDebugInfo *DI = getDebugInfo();
+assert(DI);
+llvm::DILocalVariable *DILocalVar = DI->EmitDeclareOfArgVariable(
+&D, AllocaPtr.getPointer(), ArgNo, Builder, UseIndirectDebugAddress);
+if (const auto *Var = dyn_cast_or_null(&D))
+  DI->getParamDbgMappings().insert({Var, DILocalVar});
   }
 
   if (D.hasAttr())
Index: clang/lib/CodeGen/CGDebugInfo.h
===
--- clang/lib/CodeGen/CGDebugInfo.h
+++ clang/lib/CodeGen/CGDebugInfo.h
@@ -488,10 +488,10 @@
 
   /// Emit call to \c llvm.dbg.declare for an argument variable
   /// declaration.
-  llvm::DILocalVariable *EmitDeclareOfArgVariable(const VarDecl *Decl,
-  llvm::Value *AI,
-  unsigned ArgNo,
-  CGBuilderTy &Builder);
+  llvm::DILocalVariable *
+  EmitDeclareOfArgVariable(const VarDecl *Decl, llvm::Value *AI, unsigned ArgNo,
+   CGBuilderTy &Builder,
+   const bool UsePointerValue = false);
 
   /// Emit call to \c llvm.dbg.declare for the block-literal argument
   /// to a block invocation function.
Index: clang/lib/CodeGen/CGDebugInfo.cpp

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[Felipe de Azevedo Piovezan via Phabricator via cfe-commits]

fdeazeve added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.h:494
+   CGBuilderTy &Builder,
+   const bool UsePointerValue = false);
 

FWIW I used a `const` bool here to match the style already present in this class



Comment at: clang/lib/CodeGen/CGDecl.cpp:2482
+   CGM.getCodeGenOpts().hasReducedDebugInfo() 
&&
+   !CurFuncIsThunk && !NoDebugInfo;
+

this is hoisted from below


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

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

[PATCH] D140794: [ASTMatcher] Add coroutineBodyStmt matcher

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487789.
ccotter added a comment.

- Fix ParserTest.Errors


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140794

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/lib/ASTMatchers/Dynamic/Registry.cpp
  clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
  clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp

Index: clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
===
--- clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
+++ clang/unittests/ASTMatchers/Dynamic/ParserTest.cpp
@@ -366,7 +366,8 @@
 "to build matcher: mapAnyOf.",
 ParseWithError("mapAnyOf(\"foo\")"));
   EXPECT_EQ("Input value has unresolved overloaded type: "
-"Matcher",
+"Matcher",
 ParseMatcherWithError("hasBody(stmt())"));
   EXPECT_EQ(
   "1:1: Error parsing argument 1 for matcher decl.\n"
Index: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -678,6 +678,48 @@
   EXPECT_TRUE(matchesConditionally(CoYieldCode, 
coyieldExpr(isExpansionInMainFile()), 
true, {"-std=c++20", "-I/"}, M));
+
+  StringRef NonCoroCode = R"cpp(
+#include 
+void non_coro_function() {
+}
+)cpp";
+
+  EXPECT_TRUE(matchesConditionally(CoReturnCode, coroutineBodyStmt(), true,
+   {"-std=c++20", "-I/"}, M));
+  EXPECT_TRUE(matchesConditionally(CoAwaitCode, coroutineBodyStmt(), true,
+   {"-std=c++20", "-I/"}, M));
+  EXPECT_TRUE(matchesConditionally(CoYieldCode, coroutineBodyStmt(), true,
+   {"-std=c++20", "-I/"}, M));
+
+  EXPECT_FALSE(matchesConditionally(NonCoroCode, coroutineBodyStmt(), true,
+{"-std=c++20", "-I/"}, M));
+
+  StringRef CoroWithDeclCode = R"cpp(
+#include 
+void coro() {
+  int thevar;
+  co_return 1;
+}
+)cpp";
+  EXPECT_TRUE(matchesConditionally(
+  CoroWithDeclCode,
+  coroutineBodyStmt(hasBody(compoundStmt(
+  has(declStmt(containsDeclaration(0, varDecl(hasName("thevar",
+  true, {"-std=c++20", "-I/"}, M));
+
+  StringRef CoroWithTryCatchDeclCode = R"cpp(
+#include 
+void coro() try {
+  int thevar;
+  co_return 1;
+} catch (...) {}
+)cpp";
+  EXPECT_TRUE(matchesConditionally(
+  CoroWithTryCatchDeclCode,
+  coroutineBodyStmt(hasBody(cxxTryStmt(has(compoundStmt(has(
+  declStmt(containsDeclaration(0, varDecl(hasName("thevar")),
+  true, {"-std=c++20", "-I/"}, M));
 }
 
 TEST(Matcher, isClassMessage) {
Index: clang/lib/ASTMatchers/Dynamic/Registry.cpp
===
--- clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -175,6 +175,7 @@
   REGISTER_MATCHER(containsDeclaration);
   REGISTER_MATCHER(continueStmt);
   REGISTER_MATCHER(coreturnStmt);
+  REGISTER_MATCHER(coroutineBodyStmt);
   REGISTER_MATCHER(coyieldExpr);
   REGISTER_MATCHER(cudaKernelCallExpr);
   REGISTER_MATCHER(cxxBaseSpecifier);
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -909,6 +909,8 @@
 const internal::VariadicDynCastAllOfMatcher caseStmt;
 const internal::VariadicDynCastAllOfMatcher defaultStmt;
 const internal::VariadicDynCastAllOfMatcher compoundStmt;
+const internal::VariadicDynCastAllOfMatcher
+coroutineBodyStmt;
 const internal::VariadicDynCastAllOfMatcher cxxCatchStmt;
 const internal::VariadicDynCastAllOfMatcher cxxTryStmt;
 const internal::VariadicDynCastAllOfMatcher cxxThrowExpr;
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2449,6 +2449,17 @@
 extern const internal::VariadicDynCastAllOfMatcher
 coyieldExpr;
 
+/// Matches coroutine body statements.
+///
+/// coroutineBodyStmt() matches the coroutine below
+/// \code
+///   generator gen() {
+/// co_return;
+///   }
+/// \endcode
+extern const internal::VariadicDynCastAllOfMatcher
+coroutineBodyStmt;
+
 /// Matches nullptr literal.
 extern const internal::VariadicDynCastAllOfMatcher
 cxxNullPtrLiteralExpr;
@@ -5460,9 +5471,9 @@
 }
 
 /// Matches a 'for', 'while', 'do while' statement or a function
-/// definition that has a given body. Note that in case of 

[clang] 95be33f - Update dump_ast_matchers.py to Python 3

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-01-10T09:53:06-05:00
New Revision: 95be33fb99a605f7261b5eb78ab25954e2e5ce26

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

LOG: Update dump_ast_matchers.py to Python 3

Also regenerates the documentation and fixed a validation diagnostic
about use of 'is' vs '=='.

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/docs/tools/dump_ast_matchers.py

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index f9cb9f2e942b..e49eb8430f47 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -3940,6 +3940,28 @@ Narrowing Matchers
 
 
 
+MatcherDecl>isInAnonymousNamespace
+Matches 
declarations in an anonymous namespace.
+
+Given
+  class vector {};
+  namespace foo {
+class vector {};
+namespace {
+  class vector {}; // #1
+}
+  }
+  namespace {
+class vector {}; // #2
+namespace foo {
+  class vector{}; // #3
+}
+  }
+cxxRecordDecl(hasName("vector"), isInAnonymousNamespace()) will match
+#1, #2 and #3.
+
+
+
 MatcherDecl>isInStdNamespace
 Matches 
declarations in the namespace `std`, but not in nested namespaces.
 
@@ -3962,25 +3984,6 @@ Narrowing Matchers
 cxxRecordDecl(hasName("vector"), isInStdNamespace()) will match only #1.
 
 
-MatcherDecl>isInAnonymousNamespace
-Matches 
declarations in an anonymous namespace.
-
-Given
-  class vector {};
-  namespace foo {
-class vector {};
-namespace {
-  class vector {}; // #1
-}
-  }
-  namespace {
-class vector {}; // #2
-namespace foo {
-  class vector{}; // #3
-}
-  }
-cxxRecordDecl(hasName("vector"), isInAnonymousNamespace()) will match #1, #2 
and #3.
-
 
 MatcherDecl>isInstantiated
 Matches declarations 
that are template instantiations or are inside
@@ -8550,7 +8553,7 @@ AST Traversal Matchers
 
 
 
-MatcherLambdaCapture>capturesVarMatcherVarDecl>
 InnerMatcher
+MatcherLambdaCapture>capturesVarMatcherValueDecl>
 InnerMatcher
 Matches a 
`LambdaCapture` that refers to the specified `VarDecl`. The
 `VarDecl` can be a separate variable that is captured by value or
 reference, or a synthesized variable if the capture has an initializer.

diff  --git a/clang/docs/tools/dump_ast_matchers.py 
b/clang/docs/tools/dump_ast_matchers.py
index 2ac0af1f38bc..d3f39ee45675 100755
--- a/clang/docs/tools/dump_ast_matchers.py
+++ b/clang/docs/tools/dump_ast_matchers.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 # A tool to parse ASTMatchers.h and update the documentation in
 # ../LibASTMatchersReference.html automatically. Run from the
 # directory in which this file is located to update the docs.
@@ -12,7 +12,7 @@
 
 CLASS_INDEX_PAGE_URL = 'https://clang.llvm.org/doxygen/classes.html'
 try:
-  CLASS_INDEX_PAGE = urlopen(CLASS_INDEX_PAGE_URL).read()
+  CLASS_INDEX_PAGE = urlopen(CLASS_INDEX_PAGE_URL).read().decode('utf-8')
 except Exception as e:
   raise Exception('Unable to get %s: %s' % (CLASS_INDEX_PAGE_URL, e))
 
@@ -422,7 +422,7 @@ def act_on_decl(declaration, comment, allowed_types):
   m = re.match(r'(?:^|.*\s+)internal::(?:Bindable)?Matcher<([^>]+)>$', 
result)
   if m:
 result_types = [m.group(1)]
-if template_name and len(result_types) is 1 and result_types[0] == 
template_name:
+if template_name and len(result_types) == 1 and result_types[0] == 
template_name:
   result_types = ['*']
   else:
 result_types = extract_result_types(comment)
@@ -502,6 +502,6 @@ def sort_table(matcher_type, matcher_map):
 reference = re.sub(r'',
traversal_matcher_table, reference, flags=re.S)
 
-with open('../LibASTMatchersReference.html', 'wb') as output:
+with open('../LibASTMatchersReference.html', 'w', newline='\n') as output:
   output.write(reference)
 



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

[clang] f30257e - Correct documentation for the hasBody() AST matcher; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-01-10T09:53:19-05:00
New Revision: f30257ea9f34b36597067554a45a266dc914b5a7

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

LOG: Correct documentation for the hasBody() AST matcher; NFC

Note, when regenerating the documentation for this change, no changes
were made to the HTML file. There is an existing bug with the AST
matcher python script that fails to handle hasBody() specifically. So
this commit is to correct the internal documentation but another change
will be needed to fix the public documentation.

Added: 


Modified: 
clang/include/clang/ASTMatchers/ASTMatchers.h

Removed: 




diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 5d3d458b6409..2046c59de287 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -5468,7 +5468,7 @@ AST_MATCHER_P(ArraySubscriptExpr, hasBase,
 /// \code
 ///   for (;;) {}
 /// \endcode
-/// hasBody(compoundStmt())
+/// forStmt(hasBody(compoundStmt()))
 ///   matches 'for (;;) {}'
 /// with compoundStmt()
 ///   matching '{}'
@@ -5478,7 +5478,7 @@ AST_MATCHER_P(ArraySubscriptExpr, hasBase,
 ///   void f();
 ///   void f() {}
 /// \endcode
-/// hasBody(functionDecl())
+/// functionDecl(hasBody(compoundStmt()))
 ///   matches 'void f() {}'
 /// with compoundStmt()
 ///   matching '{}'



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

[PATCH] D141384: [clang][dataflow] Fix 2 bugs in `MemberExpr` interpretation.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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

There were two (small) bugs causing crashes in the analysis.  This patch fixes 
both of them.

1. An enum value was accessed as a class member. Now, the engine gracefully

ignores such member expressions.

2. Field access in `MemberExpr` of struct/class-typed global variables. Analysis

didn't interpret fields of global vars, because the vars were initialized before
the fields were added to the "allowlist". Now, the allowlist is set _before_
init of globals.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141384

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1066,6 +1066,28 @@
   });
 }
 
+TEST(TransferTest, StructMemberEnum) {
+  std::string Code = R"(
+struct A {
+  int Bar;
+  enum E { ONE, TWO };
+};
+
+void target(A Foo) {
+  A::E Baz = Foo.ONE;
+  // [[p]]
+}
+  )";
+  // Minimal expectations -- we're just testing that it doesn't crash, since
+  // enums aren't interpreted.
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+EXPECT_THAT(Results.keys(), UnorderedElementsAre("p"));
+  });
+}
+
 TEST(TransferTest, DerivedBaseMemberClass) {
   std::string Code = R"(
 class A {
Index: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -118,7 +118,7 @@
 Context);
   const auto *Fun = selectFirst("target", Results);
   const auto *Var = selectFirst("global", Results);
-  ASSERT_TRUE(Fun != nullptr);
+  ASSERT_THAT(Fun, NotNull());
   ASSERT_THAT(Var, NotNull());
 
   // Verify the global variable is populated when we analyze `Target`.
@@ -126,6 +126,53 @@
   EXPECT_THAT(Env.getValue(*Var, SkipPast::None), NotNull());
 }
 
+TEST_F(EnvironmentTest, InitGlobalVarsFieldFun) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+ struct S { int Bar; };
+ S Global = {0};
+ int Target () { return Global.Bar; }
+  )cc";
+
+  auto Unit =
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto Results =
+  match(decl(anyOf(varDecl(hasName("Global")).bind("global"),
+   functionDecl(hasName("Target")).bind("target"))),
+Context);
+  const auto *Fun = selectFirst("target", Results);
+  const auto *GlobalDecl = selectFirst("global", Results);
+  ASSERT_THAT(Fun, NotNull());
+  ASSERT_THAT(GlobalDecl, NotNull());
+
+  ASSERT_TRUE(GlobalDecl->getType()->isStructureType());
+  auto GlobalFields = GlobalDecl->getType()->getAsRecordDecl()->fields();
+
+  FieldDecl *BarDecl = nullptr;
+  for (FieldDecl *Field : GlobalFields) {
+if (Field->getNameAsString() == "Bar") {
+  BarDecl = Field;
+  break;
+}
+FAIL() << "Unexpected field: " << Field->getNameAsString();
+  }
+  ASSERT_THAT(BarDecl, NotNull());
+
+  // Verify the global variable is populated when we analyze `Target`.
+  Environment Env(DAContext, *Fun);
+  const auto *GlobalLoc = cast(
+  Env.getStorageLocation(*GlobalDecl, SkipPast::None));
+  const auto *GlobalVal = cast(Env.getValue(*GlobalLoc));
+  const auto *BarVal = GlobalVal->getChild(*BarDecl);
+  ASSERT_THAT(BarVal, NotNull());
+  EXPECT_TRUE(isa(BarVal));
+}
+
 TEST_F(EnvironmentTest, InitGlobalVarsConstructor) {
   using namespace ast_matchers;
 
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -461,6 +461,10 @@
 if (Member->isFunctionOrFunctionTemplate())
   return;
 
+// FIXME: if/when we add support for modeling enums, use that support here.
+if (isa(Member))
+  return;
+
 if (auto *D = dyn_cast(Member)) {
   if (D->hasGlobalStorage()) {
 auto *VarDeclLoc = Env.getStorageLocation(*D, SkipPast::None);
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
=

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487794.
ccotter added a comment.

- [clang-tidy] Support begin/end free functions in modernize-loop-convert
- Add release note
- Update docs
- Fix tests
- Remove unused variable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -445,6 +445,34 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & I : Dpp)
   // CHECK-FIXES-NEXT: printf("%d\n", I->X);
+
+  for (S::iterator It = begin(Ss), E = end(Ss); It != E; ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
+
+  for (S::iterator It = begin(*Ps), E = end(*Ps); It != E; ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : *Ps)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
+
+  for (S::iterator It = begin(*Ps); It != end(*Ps); ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : *Ps)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
+
+  for (S::const_iterator It = cbegin(Ss), E = cend(Ss); It != E; ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto It : Ss)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
 }
 
 // Tests to verify the proper use of auto where the init variable type and the
@@ -653,6 +681,36 @@
   // CHECK-FIXES: for (int I : V)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
   // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = size(V); E != I; ++I) {
+printf("Fibonacci number is %d\n", V[I]);
+Sum += V[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : V)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = size(V); E != I; ++I) {
+V[I] = 0;
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int & I : V)
+  // CHECK-FIXES-NEXT: I = 0;
+
+  // Although 'length' might be a valid free function, only std::size() is standardized
+  for (int I = 0, E = length(V); E != I; ++I) {
+printf("Fibonacci number is %d\n", V[I]);
+Sum += V[I] + 2;
+  }
+
+  dependent Vals;
+  for (int I = 0, E = size(Vals); E != I; ++I) {
+Sum += Vals[I].X;
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & Val : Vals)
+  // CHECK-FIXES-NEXT: Sum += Val.X;
 }
 
 // Ensure that 'const auto &' is used with containers of non-trivial types.
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
@@ -39,6 +39,14 @@
   iterator end();
 };
 
+S::const_iterator begin(const S& s);
+S::const_iterator end(const S& s);
+S::const_iterator cbegin(const S& s);
+S::const_iterator cend(const S& s);
+S::iterator begin(S& s);
+S::iterator end(S& s);
+unsigned size(const S& s);
+
 struct T {
   typedef int value_type;
   struct iterator {
@@ -126,6 +134,11 @@
   void constFoo() const;
 };
 
+template
+unsigned size(const dependent&);
+template
+unsigned length(const dependent&);
+
 template
 class doublyDependent{
  public:
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
@@ -91,10 +91,18 @@
   for (vector::iterator it = v.begin(); it != v.end(); ++it)
 cout << *it;
 
+  // reas

[PATCH] D140794: [ASTMatcher] Add coroutineBodyStmt matcher

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a subscriber: sammccall.
aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5503
+  FunctionDecl,
+  CoroutineBodyStmt),
   internal::Matcher, InnerMatcher) {

ccotter wrote:
> aaron.ballman wrote:
> > I'm not certain it makes sense to me to add `CoroutineBodyStmt` to 
> > `hasBody` -- in this case, it doesn't *have* a body, it *is* the body.
> With respect to `hasBody()`, my intent was to treat the CoroutineBodyStmt 
> node as analogous to a FunctionDecl or WhileStmt. WhileStmts have information 
> like the loop condition expression, as CoroutineBodyStmts contain the 
> synthesized expressions such as the initial suspend Stmt. Both WhileStmts and 
> CoroutineBodyStmts then have the `getBody()` methods, usually a CompoundStmt 
> for WhileStmts and either a CompoundStmt or TryStmt for CoroutineBodyStmts.
> 
> Ultimately, I wanted to be able to match the CoroutineBodyStmt's 
> `function-body` (using the grammar) from the standard, e.g., 
> `coroutineBodyStmt(hasBody(compoundStmt().bind(...)))`. If there is a 
> different approach you'd recommend that's in line with the AST matcher design 
> strategy, I can use that instead.
What concerns me about the approach you have is that it doesn't model the AST. 
A coroutine body statement is a special kind of function body, so it does not 
itself *have* a body. So this is a bit like adding `CompoundStmt` to the 
`hasBody` matcher in that regard.

To me, the correct way to surface this would be to write the matcher: 
`functionDecl(hasBody(coroutineBodyStmt()))` to get to the coroutine body, and 
if you want to bind to the compound statement, I'd imagine 
`functionDecl(hasBody(coroutineBodyStmt(has(compoundStmt().bind(...)` would 
be the correct approach. My thinking there is that we'd use the `has()` 
traversal matcher to go from the coroutine body to any of the extra information 
it tracks (the compound statement, the promise, allocate/deallocate, etc).

Pinging @klimek and @sammccall for additional opinions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140794

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

[PATCH] D140925: [CMake] Use Clang to infer the target triple

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai accepted this revision.
smeenai added a comment.

@tamas' suggestion would be a good change to make IMO, but I think it's outside 
the scope of this patch, and the patch as-is improves the status quo, so LGTM.

Is there any way to share the normalization logic between the two locations, or 
does compiler-rt's CMake logic still need to be standalone?




Comment at: compiler-rt/lib/builtins/CMakeLists.txt:39
+  if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+set(CXX_TARGET_TRIPLE ${CMAKE_CXX_COMPILER} 
--target=${LLVM_RUNTIME_TRIPLE} -print-target-triple)
+execute_process(COMMAND ${CXX_TARGET_TRIPLE}

Maybe something slightly more wordy like `NORMALIZE_TARGET_TRIPLE_COMMAND` 
would be a clearer name?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140925

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

[PATCH] D135405: fix handling of braced-init temporaries for modernize-use-emplace

[Peter Wolf via Phabricator via cfe-commits]

BigPeet added a comment.

Ping.

(Sorry, it's my first time contributing to LLVM and I simply don't know what 
happens next. Do I need to do anything? Or is it just waiting to get merged at 
some point?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135405

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

[PATCH] D140663: CUDA/HIP: Use kernel name to map to symbol

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: clang/test/CodeGenCUDA/incomplete-func-ptr-type.cu:2
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -emit-llvm -x hip %s -o - \
+// RUN: | FileCheck %s
+

need to check `_Z19__device_stub__kern7TempValIjE` generates the correct call 
of hipLaunchKernel using the correct handle.

also need to check hipRegisterFunction uses the correct function name and 
handle.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140663

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

[PATCH] D140315: [AMDGCN] Update search path for device libraries

[Yaxun Liu via Phabricator via cfe-commits]

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

LGTM. Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140315

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

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added a comment.

@njames93 Do you agree with the overall idea of documenting config file options 
like this? I need this in place for the other patch that cleans the duplication 
for `HeaderFileExtensions`, `ImplementationFileExtensions`, `IncludeStyle`, etc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

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

[clang] 44a080e - Fix the documentation for the hasBody AST matcher

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-01-10T10:39:30-05:00
New Revision: 44a080eee52abdf3c88aff7df399ec23f445484d

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

LOG: Fix the documentation for the hasBody AST matcher

The problem was whitespace between the comment and the code for the
matcher. Rather than fix the script, I went the easier route and
removed the offending newline. If this problem comes up again though,
we should consider making the script less fragile.

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index e49eb8430f47..108fc443b0e2 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -6747,7 +6747,26 @@ AST Traversal Matchers
 
 
 MatcherCXXForRangeStmt>hasBodyMatcherStmt> 
InnerMatcher
-
+Matches a 'for', 'while', 
'do' statement or a function definition that has
+a given body. Note that in case of functions this matcher only matches the
+definition itself and not the other declarations of the same function.
+
+Given
+  for (;;) {}
+forStmt(hasBody(compoundStmt()))
+  matches 'for (;;) {}'
+with compoundStmt()
+  matching '{}'
+
+Given
+  void f();
+  void f() {}
+functionDecl(hasBody(compoundStmt()))
+  matches 'void f() {}'
+with compoundStmt()
+  matching '{}'
+  but does not match 'void f();'
+
 
 
 MatcherCXXForRangeStmt>hasInitStatementMatcherStmt> 
InnerMatcher
@@ -7823,7 +7842,26 @@ AST Traversal Matchers
 
 
 MatcherDoStmt>hasBodyMatcherStmt> 
InnerMatcher
-
+Matches a 'for', 'while', 
'do' statement or a function definition that has
+a given body. Note that in case of functions this matcher only matches the
+definition itself and not the other declarations of the same function.
+
+Given
+  for (;;) {}
+forStmt(hasBody(compoundStmt()))
+  matches 'for (;;) {}'
+with compoundStmt()
+  matching '{}'
+
+Given
+  void f();
+  void f() {}
+functionDecl(hasBody(compoundStmt()))
+  matches 'void f() {}'
+with compoundStmt()
+  matching '{}'
+  but does not match 'void f();'
+
 
 
 MatcherDoStmt>hasConditionMatcherExpr> 
InnerMatcher
@@ -8140,7 +8178,26 @@ AST Traversal Matchers
 
 
 MatcherForStmt>hasBodyMatcherStmt> 
InnerMatcher
-
+Matches a 'for', 'while', 
'do' statement or a function definition that has
+a given body. Note that in case of functions this matcher only matches the
+definition itself and not the other declarations of the same function.
+
+Given
+  for (;;) {}
+forStmt(hasBody(compoundStmt()))
+  matches 'for (;;) {}'
+with compoundStmt()
+  matching '{}'
+
+Given
+  void f();
+  void f() {}
+functionDecl(hasBody(compoundStmt()))
+  matches 'void f() {}'
+with compoundStmt()
+  matching '{}'
+  but does not match 'void f();'
+
 
 
 MatcherForStmt>hasConditionMatcherExpr> 
InnerMatcher
@@ -8315,7 +8372,26 @@ AST Traversal Matchers
 
 
 MatcherFunctionDecl>hasBodyMatcherStmt> 
InnerMatcher
-
+Matches a 'for', 'while', 
'do' statement or a function definition that has
+a given body. Note that in case of functions this matcher only matches the
+definition itself and not the other declarations of the same function.
+
+Given
+  for (;;) {}
+forStmt(hasBody(compoundStmt()))
+  matches 'for (;;) {}'
+with compoundStmt()
+  matching '{}'
+
+Given
+  void f();
+  void f() {}
+functionDecl(hasBody(compoundStmt()))
+  matches 'void f() {}'
+with compoundStmt()
+  matching '{}'
+  but does not match 'void f();'
+
 
 
 MatcherFunctionDecl>hasExplicitSpecifierMatcherExpr> 
InnerMatcher
@@ -9960,7 +10036,26 @@ AST Traversal Matchers
 
 
 MatcherWhileStmt>hasBodyMatcher

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:219
 
+- Improved :doc:`modernize-loop-convert 
` to
+  refactor container based for loops that initialize iterators with free 
function calls

Please keep alphabetical order (for check names) in this section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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

[clang] 03b83cd - Remove a stale FIXME comment; NFC

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2023-01-10T10:44:40-05:00
New Revision: 03b83cd703d403cdfd6f0082423f99aae08d3606

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

LOG: Remove a stale FIXME comment; NFC

Also regenerates the AST matcher documentation. This matcher is tested
in TEST(HasImplicitDestinationType, MatchesSimpleCase) and
TEST(HasImplicitDestinationType, DoesNotMatchIncorrectly) in
ASTMatchersTraversalTest.cpp.

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index 108fc443b0e2..a67b384d537b 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -8541,8 +8541,6 @@ AST Traversal Matchers
 MatcherImplicitCastExpr>hasImplicitDestinationTypeMatcherQualType>
 InnerMatcher
 Matches 
implicit casts whose destination type matches a given
 matcher.
-
-FIXME: Unit test this matcher
 
 
 

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index a9002895f829..f1b858c511c0 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -5822,8 +5822,6 @@ AST_MATCHER_P(ExplicitCastExpr, hasDestinationType,
 
 /// Matches implicit casts whose destination type matches a given
 /// matcher.
-///
-/// FIXME: Unit test this matcher
 AST_MATCHER_P(ImplicitCastExpr, hasImplicitDestinationType,
   internal::Matcher, InnerMatcher) {
   return InnerMatcher.matches(Node.getType(), Finder, Builder);



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

[PATCH] D135384: [AIX] Fix mcount name and call arguments

[Chris Bowler via Phabricator via cfe-commits]

cebowleratibm closed this revision.
cebowleratibm added a comment.

I had a typo in the commit so this didn't auto close.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135384

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

[clang] 3ce03c4 - [clang][dataflow] Fix 2 bugs in `MemberExpr` interpretation.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2023-01-10T15:48:00Z
New Revision: 3ce03c42dbb46531968c527d80c0243c2db7bc0e

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

LOG: [clang][dataflow] Fix 2 bugs in `MemberExpr` interpretation.

There were two (small) bugs causing crashes in the analysis.  This patch fixes 
both of them.

1. An enum value was accessed as a class member. Now, the engine gracefully
ignores such member expressions.

2. Field access in `MemberExpr` of struct/class-typed global variables. Analysis
didn't interpret fields of global vars, because the vars were initialized before
the fields were added to the "allowlist". Now, the allowlist is set _before_
init of globals.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 7d10a75b36e3e..37d200509e8d2 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -250,11 +250,12 @@ Environment::Environment(DataflowAnalysisContext &DACtx,
 }
 getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
 
-initVars(Vars);
-// These have to be set before the lines that follow to ensure that create*
-// work correctly for structs.
+// These have to be added before the lines that follow to ensure that
+// `create*` work correctly for structs.
 DACtx.addModeledFields(Fields);
 
+initVars(Vars);
+
 for (const auto *ParamDecl : FuncDecl->parameters()) {
   assert(ParamDecl != nullptr);
   auto &ParamLoc = createStorageLocation(*ParamDecl);
@@ -341,9 +342,13 @@ void Environment::pushCallInternal(const FunctionDecl 
*FuncDecl,
 }
   }
   getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
-  initVars(Vars);
+
+  // These have to be added before the lines that follow to ensure that
+  // `create*` work correctly for structs.
   DACtx->addModeledFields(Fields);
 
+  initVars(Vars);
+
   const auto *ParamIt = FuncDecl->param_begin();
 
   // FIXME: Parameters don't always map to arguments 1:1; examples include

diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index c2bf18754be51..259b82d647981 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -461,6 +461,10 @@ class TransferVisitor : public 
ConstStmtVisitor {
 if (Member->isFunctionOrFunctionTemplate())
   return;
 
+// FIXME: if/when we add support for modeling enums, use that support here.
+if (isa(Member))
+  return;
+
 if (auto *D = dyn_cast(Member)) {
   if (D->hasGlobalStorage()) {
 auto *VarDeclLoc = Env.getStorageLocation(*D, SkipPast::None);

diff  --git 
a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index e66ed70be4feb..dfbd4ff274154 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -118,7 +118,7 @@ TEST_F(EnvironmentTest, InitGlobalVarsFun) {
 Context);
   const auto *Fun = selectFirst("target", Results);
   const auto *Var = selectFirst("global", Results);
-  ASSERT_TRUE(Fun != nullptr);
+  ASSERT_THAT(Fun, NotNull());
   ASSERT_THAT(Var, NotNull());
 
   // Verify the global variable is populated when we analyze `Target`.
@@ -126,6 +126,53 @@ TEST_F(EnvironmentTest, InitGlobalVarsFun) {
   EXPECT_THAT(Env.getValue(*Var, SkipPast::None), NotNull());
 }
 
+TEST_F(EnvironmentTest, InitGlobalVarsFieldFun) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+ struct S { int Bar; };
+ S Global = {0};
+ int Target () { return Global.Bar; }
+  )cc";
+
+  auto Unit =
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto Results =
+  match(decl(anyOf(varDecl(hasName("Global")).bind("global"),
+   functionDecl(hasName("Target")).bind("target"))),
+Context);
+  const auto *Fun = selectFirst("target", Results);
+  const auto *GlobalDecl = selectFirst("global", Results);
+  ASSERT_THAT(Fun, NotNull());
+  ASSERT_THAT(GlobalDecl, NotNull());
+
+  ASSERT_TRUE(GlobalDecl->get

[PATCH] D141384: [clang][dataflow] Fix 2 bugs in `MemberExpr` interpretation.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3ce03c42dbb4: [clang][dataflow] Fix 2 bugs in `MemberExpr` 
interpretation. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141384

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1066,6 +1066,28 @@
   });
 }
 
+TEST(TransferTest, StructMemberEnum) {
+  std::string Code = R"(
+struct A {
+  int Bar;
+  enum E { ONE, TWO };
+};
+
+void target(A Foo) {
+  A::E Baz = Foo.ONE;
+  // [[p]]
+}
+  )";
+  // Minimal expectations -- we're just testing that it doesn't crash, since
+  // enums aren't interpreted.
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+EXPECT_THAT(Results.keys(), UnorderedElementsAre("p"));
+  });
+}
+
 TEST(TransferTest, DerivedBaseMemberClass) {
   std::string Code = R"(
 class A {
Index: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -118,7 +118,7 @@
 Context);
   const auto *Fun = selectFirst("target", Results);
   const auto *Var = selectFirst("global", Results);
-  ASSERT_TRUE(Fun != nullptr);
+  ASSERT_THAT(Fun, NotNull());
   ASSERT_THAT(Var, NotNull());
 
   // Verify the global variable is populated when we analyze `Target`.
@@ -126,6 +126,53 @@
   EXPECT_THAT(Env.getValue(*Var, SkipPast::None), NotNull());
 }
 
+TEST_F(EnvironmentTest, InitGlobalVarsFieldFun) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+ struct S { int Bar; };
+ S Global = {0};
+ int Target () { return Global.Bar; }
+  )cc";
+
+  auto Unit =
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto Results =
+  match(decl(anyOf(varDecl(hasName("Global")).bind("global"),
+   functionDecl(hasName("Target")).bind("target"))),
+Context);
+  const auto *Fun = selectFirst("target", Results);
+  const auto *GlobalDecl = selectFirst("global", Results);
+  ASSERT_THAT(Fun, NotNull());
+  ASSERT_THAT(GlobalDecl, NotNull());
+
+  ASSERT_TRUE(GlobalDecl->getType()->isStructureType());
+  auto GlobalFields = GlobalDecl->getType()->getAsRecordDecl()->fields();
+
+  FieldDecl *BarDecl = nullptr;
+  for (FieldDecl *Field : GlobalFields) {
+if (Field->getNameAsString() == "Bar") {
+  BarDecl = Field;
+  break;
+}
+FAIL() << "Unexpected field: " << Field->getNameAsString();
+  }
+  ASSERT_THAT(BarDecl, NotNull());
+
+  // Verify the global variable is populated when we analyze `Target`.
+  Environment Env(DAContext, *Fun);
+  const auto *GlobalLoc = cast(
+  Env.getStorageLocation(*GlobalDecl, SkipPast::None));
+  const auto *GlobalVal = cast(Env.getValue(*GlobalLoc));
+  const auto *BarVal = GlobalVal->getChild(*BarDecl);
+  ASSERT_THAT(BarVal, NotNull());
+  EXPECT_TRUE(isa(BarVal));
+}
+
 TEST_F(EnvironmentTest, InitGlobalVarsConstructor) {
   using namespace ast_matchers;
 
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -461,6 +461,10 @@
 if (Member->isFunctionOrFunctionTemplate())
   return;
 
+// FIXME: if/when we add support for modeling enums, use that support here.
+if (isa(Member))
+  return;
+
 if (auto *D = dyn_cast(Member)) {
   if (D->hasGlobalStorage()) {
 auto *VarDeclLoc = Env.getStorageLocation(*D, SkipPast::None);
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -250,11 +250,12 @@
 }
 getFieldsAndGlobalVars(*FuncDecl->getBody(), Fields, Vars);
 
-initVars(Vars);
-// These have to be set before the lines that follow to ensure that create*
-// work correctly for structs.
+// These have to be added before the lines that follow to ensure that
+// `create*`

[PATCH] D140794: [ASTMatcher] Add coroutineBodyStmt matcher

[Chris Cotter via Phabricator via cfe-commits]

ccotter added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5503
+  FunctionDecl,
+  CoroutineBodyStmt),
   internal::Matcher, InnerMatcher) {

aaron.ballman wrote:
> ccotter wrote:
> > aaron.ballman wrote:
> > > I'm not certain it makes sense to me to add `CoroutineBodyStmt` to 
> > > `hasBody` -- in this case, it doesn't *have* a body, it *is* the body.
> > With respect to `hasBody()`, my intent was to treat the CoroutineBodyStmt 
> > node as analogous to a FunctionDecl or WhileStmt. WhileStmts have 
> > information like the loop condition expression, as CoroutineBodyStmts 
> > contain the synthesized expressions such as the initial suspend Stmt. Both 
> > WhileStmts and CoroutineBodyStmts then have the `getBody()` methods, 
> > usually a CompoundStmt for WhileStmts and either a CompoundStmt or TryStmt 
> > for CoroutineBodyStmts.
> > 
> > Ultimately, I wanted to be able to match the CoroutineBodyStmt's 
> > `function-body` (using the grammar) from the standard, e.g., 
> > `coroutineBodyStmt(hasBody(compoundStmt().bind(...)))`. If there is a 
> > different approach you'd recommend that's in line with the AST matcher 
> > design strategy, I can use that instead.
> What concerns me about the approach you have is that it doesn't model the 
> AST. A coroutine body statement is a special kind of function body, so it 
> does not itself *have* a body. So this is a bit like adding `CompoundStmt` to 
> the `hasBody` matcher in that regard.
> 
> To me, the correct way to surface this would be to write the matcher: 
> `functionDecl(hasBody(coroutineBodyStmt()))` to get to the coroutine body, 
> and if you want to bind to the compound statement, I'd imagine 
> `functionDecl(hasBody(coroutineBodyStmt(has(compoundStmt().bind(...)` 
> would be the correct approach. My thinking there is that we'd use the `has()` 
> traversal matcher to go from the coroutine body to any of the extra 
> information it tracks (the compound statement, the promise, 
> allocate/deallocate, etc).
> 
> Pinging @klimek and @sammccall for additional opinions.
I think I see. With my proposal, the match would be 
`functionDecl(hasBody(coroutineBodyStmt(hasBody(stmt().bind(...)`. For 
completeness, your suggestion would need 
`functionDecl(hasBody(coroutineBodyStmt(has(stmt(anyOf(cxxTryStmt(), 
compoundStmt()).bind(...))`.

I am a bit hung up though on two things, and clarifications on both would help 
solidify my understanding of the design philosophy of the matchers. First, 
since `CoroutineBodyStmt` has a `getBody()` method, I assumed it would make it 
eligible for the `hasBody` matcher, although perhaps I am making an incorrect 
assumption/generalization here?

Second, the `has()` approach seems less accurate since we would be relying on 
the fact that the other children nodes of `CoroutineBodyStmt` (initial or final 
suspend point, etc) would never be a `CompoundStmt` or `CXXTryStmt`, else we 
might get unintentional matches. Or, one might miss the CXXTryStmt corner case.

Follow up question - should a need arise (say, authoring many clang-tidy checks 
that need extensive coroutine matcher support on the initial suspend point 
etc), would we see the matchers supporting things like 
`coroutineBodyStmt(hasInitialSuspendPoint(...), hasFinalSuspendPoint(..))`, or 
rely on a combination of `has` approach / non-ASTMatcher logic (or locally 
defined ASTMatchers within the clang-tidy library)?

It looks like this phab can be broken down into two changes - first, the 
addition of a `coroutineBodyStmt` matcher, and second, a `hasBody` traversal 
matcher for `coroutineBodyStmt`. Happy to separate these out depending on the 
direction of the discussion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140794

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

[PATCH] D141389: [DFSAN] Add support for strnlen, strncat, strsep, sscanf and _tolower

[Tomasz Kuchta via Phabricator via cfe-commits]

tkuchta created this revision.
tkuchta added reviewers: vitalybuka, glider.
tkuchta added a project: Sanitizers.
Herald added a subscriber: Enna1.
Herald added a project: All.
tkuchta requested review of this revision.

This patch adds support for the following libc functions in DFSAN: strnlen, 
strncat, strsep and sscanf. It further enables _tolower via the options file.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D141389

Files:
  compiler-rt/lib/dfsan/dfsan_custom.cpp
  compiler-rt/lib/dfsan/done_abilist.txt
  compiler-rt/test/dfsan/custom.cpp

Index: compiler-rt/test/dfsan/custom.cpp
===
--- compiler-rt/test/dfsan/custom.cpp
+++ compiler-rt/test/dfsan/custom.cpp
@@ -364,6 +364,47 @@
   ASSERT_LABEL(dst[11], j_label);
 }
 
+void test_strncat() {
+  char src[] = "world";
+  int volatile x = 0; // buffer to ensure src and dst do not share origins
+  (void)x;
+  char dst[] = "hello \0";
+  int volatile y = 0; // buffer to ensure dst and p do not share origins
+  (void)y;
+  char *p = dst;
+  dfsan_set_label(k_label, &p, sizeof(p));
+  dfsan_set_label(i_label, src, sizeof(src));
+  dfsan_set_label(j_label, dst, sizeof(dst));
+  dfsan_origin dst_o = dfsan_get_origin((long)dst[0]);
+  (void)dst_o;
+  char *ret = strncat(p, src, strlen(src));
+  ASSERT_LABEL(ret, k_label);
+  ASSERT_EQ_ORIGIN(ret, p);
+  assert(ret == dst);
+  assert(strcmp(src, dst + 6) == 0);
+  // Origins are assigned for every 4 contiguous 4-aligned bytes. After
+  // appending src to dst, origins of src can overwrite origins of dst if their
+  // application adddresses are within [start_aligned_down, end_aligned_up).
+  // Other origins are not changed.
+  char *start_aligned_down = (char *)(((size_t)(dst + 6)) & ~3UL);
+  char *end_aligned_up = (char *)(((size_t)(dst + 11 + 4)) & ~3UL);
+  for (int i = 0; i < 12; ++i) {
+if (dst + i < start_aligned_down || dst + i >= end_aligned_up) {
+  ASSERT_INIT_ORIGIN(&dst[i], dst_o);
+} else {
+  ASSERT_INIT_ORIGIN_EQ_ORIGIN(&dst[i], src[0]);
+}
+  }
+  for (int i = 0; i < 6; ++i) {
+ASSERT_LABEL(dst[i], j_label);
+  }
+  for (int i = 6; i < strlen(dst); ++i) {
+ASSERT_LABEL(dst[i], i_label);
+assert(dfsan_get_label(dst[i]) == dfsan_get_label(src[i - 6]));
+  }
+  ASSERT_LABEL(dst[11], j_label);
+}
+
 void test_strlen() {
   char str1[] = "str1";
   dfsan_set_label(i_label, &str1[3], 1);
@@ -378,6 +419,20 @@
 #endif
 }
 
+void test_strnlen() {
+  char str1[] = "str1";
+  dfsan_set_label(i_label, &str1[3], 1);
+
+  int rv = strnlen(str1, 4);
+  assert(rv == 4);
+#ifdef STRICT_DATA_DEPENDENCIES
+  ASSERT_ZERO_LABEL(rv);
+#else
+  ASSERT_LABEL(rv, i_label);
+  ASSERT_EQ_ORIGIN(rv, str1[3]);
+#endif
+}
+
 void test_strdup() {
   char str1[] = "str1";
   dfsan_set_label(i_label, &str1[3], 1);
@@ -1627,6 +1682,77 @@
 #endif
 }
 
+void test_strsep() {
+  char *s = strdup("Hello world/");
+  char *delim = strdup(" /");
+
+  char *p_s = s;
+  char *base = s;
+  char *p_delim = delim;
+
+  dfsan_set_label(n_label, p_delim, sizeof(p_delim));
+
+  char *rv = strsep(&p_s, p_delim);
+  assert(rv == &base[0]);
+#ifdef STRICT_DATA_DEPENDENCIES
+  ASSERT_ZERO_LABEL(rv);
+#else
+  ASSERT_LABEL(rv, n_label);
+  ASSERT_INIT_ORIGIN_EQ_ORIGIN(&rv, *p_delim);
+#endif
+
+  dfsan_set_label(m_label, p_s, sizeof(p_s));
+  rv = strsep(&p_s, p_delim);
+
+  assert(rv == &base[6]);
+#ifdef STRICT_DATA_DEPENDENCIES
+  ASSERT_LABEL(rv, m_label);
+  ASSERT_INIT_ORIGIN_EQ_ORIGIN(&rv, base[6]);
+#else
+  ASSERT_LABEL(rv, dfsan_union(m_label, n_label));
+  ASSERT_INIT_ORIGIN_EQ_ORIGIN(&rv, base[6]);
+#endif
+
+  free(s);
+  s = strdup("Hello world/");
+  base = s;
+  free(delim);
+  delim = strdup(" /");
+  p_delim = delim;
+  dfsan_set_label(i_label, &s[7], 1);
+  dfsan_set_label(j_label, &delim[0], 1);
+
+  rv = strsep(&s, delim);
+  assert(rv == &base[0]);
+#ifdef STRICT_DATA_DEPENDENCIES
+  ASSERT_ZERO_LABEL(rv);
+#else
+  ASSERT_LABEL(rv, j_label);
+  ASSERT_INIT_ORIGIN_EQ_ORIGIN(&rv, delim[1]);
+#endif
+
+  char *ps = s;
+  dfsan_set_label(dfsan_union(j_label, dfsan_read_label(ps, strlen(ps))), ps,
+  strlen(ps));
+  rv = strsep(&ps, " /");
+  assert(rv == &base[6]);
+#ifdef STRICT_DATA_DEPENDENCIES
+  ASSERT_LABEL(rv, i_j_label);
+#else
+  ASSERT_LABEL(rv, i_j_label);
+  ASSERT_INIT_ORIGIN_EQ_ORIGIN(&rv, base[6]);
+#endif
+  rv = strsep(&ps, " /");
+  assert(strlen(rv) == 0);
+#ifdef STRICT_DATA_DEPENDENCIES
+  ASSERT_ZERO_LABEL(ps);
+#else
+  ASSERT_ZERO_LABEL(rv);
+  ASSERT_INIT_ORIGIN_EQ_ORIGIN(&rv, 0);
+
+#endif
+}
+
 void test_memchr() {
   char str1[] = "str1";
   dfsan_set_label(i_label, &str1[3], 1);
@@ -1968,6 +2094,138 @@
   ASSERT_LABEL(r, 0);
 }
 
+template 
+void test_sscanf_chunk(T expected, const char *format, const char *input) {
+  char padded_input[512];
+  strcpy(padded_input, "foo ");
+  strcat(padded_input, input);
+  strcat(padded_input, " bar");
+
+  char padded_format[51

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487807.
ccotter added a comment.

- alpha


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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

Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -445,6 +445,34 @@
   // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
   // CHECK-FIXES: for (auto & I : Dpp)
   // CHECK-FIXES-NEXT: printf("%d\n", I->X);
+
+  for (S::iterator It = begin(Ss), E = end(Ss); It != E; ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : Ss)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
+
+  for (S::iterator It = begin(*Ps), E = end(*Ps); It != E; ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : *Ps)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
+
+  for (S::iterator It = begin(*Ps); It != end(*Ps); ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & It : *Ps)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
+
+  for (S::const_iterator It = cbegin(Ss), E = cend(Ss); It != E; ++It) {
+printf("s has value %d\n", (*It).X);
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto It : Ss)
+  // CHECK-FIXES-NEXT: printf("s has value %d\n", It.X);
 }
 
 // Tests to verify the proper use of auto where the init variable type and the
@@ -653,6 +681,36 @@
   // CHECK-FIXES: for (int I : V)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
   // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = size(V); E != I; ++I) {
+printf("Fibonacci number is %d\n", V[I]);
+Sum += V[I] + 2;
+  }
+  // CHECK-MESSAGES: :[[@LINE-4]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int I : V)
+  // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
+  // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = size(V); E != I; ++I) {
+V[I] = 0;
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (int & I : V)
+  // CHECK-FIXES-NEXT: I = 0;
+
+  // Although 'length' might be a valid free function, only std::size() is standardized
+  for (int I = 0, E = length(V); E != I; ++I) {
+printf("Fibonacci number is %d\n", V[I]);
+Sum += V[I] + 2;
+  }
+
+  dependent Vals;
+  for (int I = 0, E = size(Vals); E != I; ++I) {
+Sum += Vals[I].X;
+  }
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: use range-based for loop instead
+  // CHECK-FIXES: for (auto & Val : Vals)
+  // CHECK-FIXES-NEXT: Sum += Val.X;
 }
 
 // Ensure that 'const auto &' is used with containers of non-trivial types.
Index: clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
@@ -39,6 +39,14 @@
   iterator end();
 };
 
+S::const_iterator begin(const S& s);
+S::const_iterator end(const S& s);
+S::const_iterator cbegin(const S& s);
+S::const_iterator cend(const S& s);
+S::iterator begin(S& s);
+S::iterator end(S& s);
+unsigned size(const S& s);
+
 struct T {
   typedef int value_type;
   struct iterator {
@@ -126,6 +134,11 @@
   void constFoo() const;
 };
 
+template
+unsigned size(const dependent&);
+template
+unsigned length(const dependent&);
+
 template
 class doublyDependent{
  public:
Index: clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
+++ clang-tools-extra/docs/clang-tidy/checks/modernize/loop-convert.rst
@@ -91,10 +91,18 @@
   for (vector::iterator it = v.begin(); it != v.end(); ++it)
 cout << *it;
 
+  // reasonable conversion
+  for (vector::iterator it = begin(v); it != end(v); ++it)
+cout << *it;
+
   // reasonable conversion
   for (in

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added a comment.

> FWIW a lot of build systems support setting CXXFLAGS/CFLAGS before invoking 
> the build system/build generator (cmake, for instance) and respects those - 
> so might be relatively easy to add a new warning flag to the build (& CXX/CC 
> to set the compiler to point to your local build with the patch/changes)

Thanks for your advice! I'll give it a try. It might be a nice opportunity for 
me to learn some compiler development methods.

> So, based on best guess at least from grepping - you don't find any new 
> instances of the warning in these code bases?

No, I did not find any.


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

https://reviews.llvm.org/D140860

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

[PATCH] D141391: [NFC] [clang-tools-extra] Alphabetize clang-tidy release notes

[Chris Cotter via Phabricator via cfe-commits]

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

Alphabetize order of clang-tidy release notes, and fix `:doc:` link.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141391

Files:
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -167,6 +167,12 @@
   ` check for 
exceptions
   thrown by code emitted from macros in system headers.
 
+- Improved :doc:`misc-redundant-expression 
`
+  check.
+
+  The check now skips concept definitions since redundant expressions still 
make sense
+  inside them.
+
 - Improved :doc:`modernize-use-emplace 
`
   check.
 
@@ -206,14 +212,8 @@
   ` to not
   warn about `const` value parameters of declarations inside macros.
 
-- Improved :doc:`misc-redundant-expression 
`
-  check.
-
-  The check now skips concept definitions since redundant expressions still 
make sense
-  inside them.
-
 - Support removing ``c_str`` calls from ``std::string_view`` constructor calls 
in
-  :doc: `readability-redundant-string-cstr 
`
+  :doc:`readability-redundant-string-cstr 
`
   check.
 
 Removed checks


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -167,6 +167,12 @@
   ` check for exceptions
   thrown by code emitted from macros in system headers.
 
+- Improved :doc:`misc-redundant-expression `
+  check.
+
+  The check now skips concept definitions since redundant expressions still make sense
+  inside them.
+
 - Improved :doc:`modernize-use-emplace `
   check.
 
@@ -206,14 +212,8 @@
   ` to not
   warn about `const` value parameters of declarations inside macros.
 
-- Improved :doc:`misc-redundant-expression `
-  check.
-
-  The check now skips concept definitions since redundant expressions still make sense
-  inside them.
-
 - Support removing ``c_str`` calls from ``std::string_view`` constructor calls in
-  :doc: `readability-redundant-string-cstr `
+  :doc:`readability-redundant-string-cstr `
   check.
 
 Removed checks
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140760: [clang-tidy] Support begin/end free functions in modernize-loop-convert

[Chris Cotter via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:219
 
+- Improved :doc:`modernize-loop-convert 
` to
+  refactor container based for loops that initialize iterators with free 
function calls

Eugene.Zelenko wrote:
> Please keep alphabetical order (for check names) in this section.
Done, and I fixed what looks like an existing out of order check in the release 
notes: https://reviews.llvm.org/D141391


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140760

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

[PATCH] D139774: [libclang] Add API to set temporary directory location

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D139774#4039018 , @vedgy wrote:

> In D139774#4036496 , @aaron.ballman 
> wrote:
>
>> Given that we already have other setters to set global state, I'm less 
>> concerned about adding another one. I hadn't realized we already introduced 
>> the dangers here. But we should document the expectation that the call be 
>> made before creating the index.
>
> There is a difference: `clang_CXIndex_setGlobalOptions()` and 
> `clang_CXIndex_setInvocationEmissionPathOption()` take a `CXIndex` argument 
> and thus can only be called **after** creating an index. So requiring to call 
> `clang_overrideTemporaryDirectory()` before creating an index, plus one more 
> time with `nullptr` argument after disposing of the index, wouldn't be 
> particularly consistent with other setters.

Oh that is a good point! Apologies for not noticing that earlier -- that makes 
me wonder if a better approach here is to add a `std::string` to the `CIndexer` 
class to represent the temp path to use. I started investigating that idea and 
then I realized I have no idea what is calling `system_temp_directory()` in the 
first place. It doesn't seem to be anything from the C indexing API but it's 
possible this is coming from one of the other library components. Can you help 
me track down the call stack where we start creating the temporary files so I 
can better understand? My hope is that we can thread the information through 
rather than using a global setter, if possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774

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

[clang-tools-extra] e432952 - [clang-tidy] Match derived types in in modernize-loop-convert

[Carlos Galvez via cfe-commits]

Author: Chris Cotter
Date: 2023-01-10T16:08:25Z
New Revision: e43295209bb86e93008363c66c1277c7d8bb148c

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

LOG: [clang-tidy] Match derived types in in modernize-loop-convert

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

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

Reviewed By: carlosgalvezp

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/docs/ReleaseNotes.rst

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

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
index 84df6445ef77e..0056acbe5fc8a 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
@@ -252,17 +252,18 @@ StatementMatcher makePseudoArrayLoopMatcher() {
   // functions called begin() and end() taking the container as an argument
   // are also allowed.
   TypeMatcher RecordWithBeginEnd = qualType(anyOf(
-  qualType(
-  isConstQualified(),
-  hasUnqualifiedDesugaredType(recordType(hasDeclaration(cxxRecordDecl(
-  hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
-  hasMethod(cxxMethodDecl(hasName("end"),
-  isConst()   // hasDeclaration
- ))), // qualType
+  qualType(isConstQualified(),
+   hasUnqualifiedDesugaredType(recordType(hasDeclaration(
+   cxxRecordDecl(isSameOrDerivedFrom(cxxRecordDecl(
+   hasMethod(cxxMethodDecl(hasName("begin"), isConst())),
+   hasMethod(cxxMethodDecl(hasName("end"),
+   isConst())) // 
hasDeclaration
+  ))), // qualType
   qualType(unless(isConstQualified()),
hasUnqualifiedDesugaredType(recordType(hasDeclaration(
-   cxxRecordDecl(hasMethod(hasName("begin")),
- hasMethod(hasName("end"))) // qualType
+   cxxRecordDecl(isSameOrDerivedFrom(cxxRecordDecl(
+   hasMethod(hasName("begin")),
+   hasMethod(hasName("end") // qualType
   ));
 
   StatementMatcher SizeCallMatcher = cxxMemberCallExpr(

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst 
b/clang-tools-extra/docs/ReleaseNotes.rst
index 0f060f7f5fc7b..70d26dbe42d67 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -216,6 +216,10 @@ Changes in existing checks
   :doc: `readability-redundant-string-cstr 
`
   check.
 
+- Improved :doc:`modernize-loop-convert 
`
+  to check for container functions ``begin``/``end`` etc on base classes of 
the container
+  type, instead of only as direct members of the container type itself.
+
 Removed checks
 ^^
 

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
index abd64904ee702..bc025ceab1f7a 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/Inputs/loop-convert/structures.h
@@ -126,6 +126,10 @@ class dependent {
   void constFoo() const;
 };
 
+template
+class dependent_derived : public dependent {
+};
+
 template
 class doublyDependent{
  public:

diff  --git 
a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
index b42ce4a19a6b7..96b3956358787 100644
--- 
a/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ 
b/clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -575,6 +575,7 @@ const dependent Constv;
 const dependent *Pconstv;
 
 transparent> Cv;
+dependent_derived VD;
 
 void f() {
   int Sum = 0;
@@ -653,6 +654,15 @@ void f() {
   // CHECK-FIXES: for (int I : V)
   // CHECK-FIXES-NEXT: printf("Fibonacci number is %d\n", I);
   // CHECK-FIXES-NEXT: Sum += I + 2;
+
+  for (int I = 0, E = VD.size(); E != I; ++I) {
+printf("Fibonacci number is %d\n", VD[I]);
+Sum += VD[I] + 2;
+  }
+  //

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

[Carlos Galvez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe43295209bb8: [clang-tidy] Match derived types in in 
modernize-loop-convert (authored by ccotter, committed by carlosgalvezp).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140307

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


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


Index: clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize/loop-convert-basic.cpp
@@ -575,6 +575,7 @@
 const dependent *Pconstv;
 
 transparent> Cv;
+dependent_derived VD;
 
 void f() {
   int Sum = 0;
@@ -653,6 +654,15 @@
   // CHECK-FIXES: for (int I : 

[PATCH] D141391: [NFC] [clang-tools-extra] Alphabetize clang-tidy release notes

[Eugene Zelenko via Phabricator via cfe-commits]

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

Thank you for fix! Such issues may be result of rebase artifacts and slip into 
code base.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141391

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

[PATCH] D141391: [NFC] [clang-tools-extra] Alphabetize clang-tidy release notes

[Chris Cotter via Phabricator via cfe-commits]

ccotter added a comment.

I can't land these myself, so if you are able to, would you mind doing so? 
Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141391

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

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

[Evan Smal via Phabricator via cfe-commits]

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



In D141283#4038703 , @tbaeder wrote:

> When grepping `clang/test/` for "for char array is too long", I get a bunch 
> of other hits for this diagnostic, are the those tests not failing for you 
> (e.g. `clang/test/Sema/array-init.c:149`)?

Yeah, the tests don't fail for me. I can update  them, but I'm  unsure how to 
tell if they're working properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141283

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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Thanks for reviewing, @erichkeane! And a gentle ping to @aaron.ballman


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

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

[PATCH] D139608: [Clang][NFC] Add default `getBFloat16Mangling` impl

[Ties Stuij via Phabricator via cfe-commits]

stuij added a comment.

I went back and forth on this for a bit. There's a review outstanding to change 
the mangling name for X86: https://reviews.llvm.org/D136919 to align with the 
C++23 std::bfloat16_t issue from above. I settled on preferring to be 
conservative, and not risk at some point accidentally opting an arch into a 
name that doesn't conform with their ABI.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139608

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

[PATCH] D141391: [NFC] [clang-tools-extra] Alphabetize clang-tidy release notes

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487813.
ccotter added a comment.

- merge from upstream


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141391

Files:
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -167,6 +167,16 @@
   ` check for 
exceptions
   thrown by code emitted from macros in system headers.
 
+- Improved :doc:`misc-redundant-expression 
`
+  check.
+
+  The check now skips concept definitions since redundant expressions still 
make sense
+  inside them.
+
+- Improved :doc:`modernize-loop-convert 
`
+  to check for container functions ``begin``/``end`` etc on base classes of 
the container
+  type, instead of only as direct members of the container type itself.
+
 - Improved :doc:`modernize-use-emplace 
`
   check.
 
@@ -206,20 +216,10 @@
   ` to not
   warn about `const` value parameters of declarations inside macros.
 
-- Improved :doc:`misc-redundant-expression 
`
-  check.
-
-  The check now skips concept definitions since redundant expressions still 
make sense
-  inside them.
-
 - Support removing ``c_str`` calls from ``std::string_view`` constructor calls 
in
-  :doc: `readability-redundant-string-cstr 
`
+  :doc:`readability-redundant-string-cstr 
`
   check.
 
-- Improved :doc:`modernize-loop-convert 
`
-  to check for container functions ``begin``/``end`` etc on base classes of 
the container
-  type, instead of only as direct members of the container type itself.
-
 Removed checks
 ^^
 


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -167,6 +167,16 @@
   ` check for exceptions
   thrown by code emitted from macros in system headers.
 
+- Improved :doc:`misc-redundant-expression `
+  check.
+
+  The check now skips concept definitions since redundant expressions still make sense
+  inside them.
+
+- Improved :doc:`modernize-loop-convert `
+  to check for container functions ``begin``/``end`` etc on base classes of the container
+  type, instead of only as direct members of the container type itself.
+
 - Improved :doc:`modernize-use-emplace `
   check.
 
@@ -206,20 +216,10 @@
   ` to not
   warn about `const` value parameters of declarations inside macros.
 
-- Improved :doc:`misc-redundant-expression `
-  check.
-
-  The check now skips concept definitions since redundant expressions still make sense
-  inside them.
-
 - Support removing ``c_str`` calls from ``std::string_view`` constructor calls in
-  :doc: `readability-redundant-string-cstr `
+  :doc:`readability-redundant-string-cstr `
   check.
 
-- Improved :doc:`modernize-loop-convert `
-  to check for container functions ``begin``/``end`` etc on base classes of the container
-  type, instead of only as direct members of the container type itself.
-
 Removed checks
 ^^
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141391: [NFC] [clang-tools-extra] Alphabetize clang-tidy release notes

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a subscriber: carlosgalvezp.
Eugene.Zelenko added a comment.

In D141391#4040002 , @ccotter wrote:

> I can't land these myself, so if you are able to, would you mind doing so? 
> Thanks!

@carlosgalvezp: Could you please help?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141391

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

[PATCH] D140794: [ASTMatcher] Add coroutineBodyStmt matcher

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:5503
+  FunctionDecl,
+  CoroutineBodyStmt),
   internal::Matcher, InnerMatcher) {

ccotter wrote:
> aaron.ballman wrote:
> > ccotter wrote:
> > > aaron.ballman wrote:
> > > > I'm not certain it makes sense to me to add `CoroutineBodyStmt` to 
> > > > `hasBody` -- in this case, it doesn't *have* a body, it *is* the body.
> > > With respect to `hasBody()`, my intent was to treat the CoroutineBodyStmt 
> > > node as analogous to a FunctionDecl or WhileStmt. WhileStmts have 
> > > information like the loop condition expression, as CoroutineBodyStmts 
> > > contain the synthesized expressions such as the initial suspend Stmt. 
> > > Both WhileStmts and CoroutineBodyStmts then have the `getBody()` methods, 
> > > usually a CompoundStmt for WhileStmts and either a CompoundStmt or 
> > > TryStmt for CoroutineBodyStmts.
> > > 
> > > Ultimately, I wanted to be able to match the CoroutineBodyStmt's 
> > > `function-body` (using the grammar) from the standard, e.g., 
> > > `coroutineBodyStmt(hasBody(compoundStmt().bind(...)))`. If there is a 
> > > different approach you'd recommend that's in line with the AST matcher 
> > > design strategy, I can use that instead.
> > What concerns me about the approach you have is that it doesn't model the 
> > AST. A coroutine body statement is a special kind of function body, so it 
> > does not itself *have* a body. So this is a bit like adding `CompoundStmt` 
> > to the `hasBody` matcher in that regard.
> > 
> > To me, the correct way to surface this would be to write the matcher: 
> > `functionDecl(hasBody(coroutineBodyStmt()))` to get to the coroutine body, 
> > and if you want to bind to the compound statement, I'd imagine 
> > `functionDecl(hasBody(coroutineBodyStmt(has(compoundStmt().bind(...)` 
> > would be the correct approach. My thinking there is that we'd use the 
> > `has()` traversal matcher to go from the coroutine body to any of the extra 
> > information it tracks (the compound statement, the promise, 
> > allocate/deallocate, etc).
> > 
> > Pinging @klimek and @sammccall for additional opinions.
> I think I see. With my proposal, the match would be 
> `functionDecl(hasBody(coroutineBodyStmt(hasBody(stmt().bind(...)`. For 
> completeness, your suggestion would need 
> `functionDecl(hasBody(coroutineBodyStmt(has(stmt(anyOf(cxxTryStmt(), 
> compoundStmt()).bind(...))`.
> 
> I am a bit hung up though on two things, and clarifications on both would 
> help solidify my understanding of the design philosophy of the matchers. 
> First, since `CoroutineBodyStmt` has a `getBody()` method, I assumed it would 
> make it eligible for the `hasBody` matcher, although perhaps I am making an 
> incorrect assumption/generalization here?
> 
> Second, the `has()` approach seems less accurate since we would be relying on 
> the fact that the other children nodes of `CoroutineBodyStmt` (initial or 
> final suspend point, etc) would never be a `CompoundStmt` or `CXXTryStmt`, 
> else we might get unintentional matches. Or, one might miss the CXXTryStmt 
> corner case.
> 
> Follow up question - should a need arise (say, authoring many clang-tidy 
> checks that need extensive coroutine matcher support on the initial suspend 
> point etc), would we see the matchers supporting things like 
> `coroutineBodyStmt(hasInitialSuspendPoint(...), hasFinalSuspendPoint(..))`, 
> or rely on a combination of `has` approach / non-ASTMatcher logic (or locally 
> defined ASTMatchers within the clang-tidy library)?
> 
> It looks like this phab can be broken down into two changes - first, the 
> addition of a `coroutineBodyStmt` matcher, and second, a `hasBody` traversal 
> matcher for `coroutineBodyStmt`. Happy to separate these out depending on the 
> direction of the discussion.
> I am a bit hung up though on two things, and clarifications on both would 
> help solidify my understanding of the design philosophy of the matchers. 
> First, since CoroutineBodyStmt has a getBody() method, I assumed it would 
> make it eligible for the hasBody matcher, although perhaps I am making an 
> incorrect assumption/generalization here?

As I understand the evolution here, we started out with `hasBody()` to match on 
function declaration AST nodes with a body. It then evolved to include 
do/while/for (and eventually range-based for) loops, which is a bit of an odd 
decision (why not anything with a substatement, like `if` or `switch`?). Now 
we're looking to add support for the coroutine body node, but that has a half 
dozen+ different things we can traverse to, one of which is the compound 
statement for the coroutine body. All of these cases are kind of different from 
one another, so the design is a bit confusing (at least to me).

> Second,

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D141283#4038703 , @tbaeder wrote:

> When grepping `clang/test/` for "for char array is too long", I get a bunch 
> of other hits for this diagnostic, are the those tests not failing for you 
> (e.g. `clang/test/Sema/array-init.c:149`)?

The change is specific to C++, so it should not apply to `.c` file, right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141283

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

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added a comment.

In D141283#4038703 , @tbaeder wrote:

> When grepping `clang/test/` for "for char array is too long", I get a bunch 
> of other hits for this diagnostic, are the those tests not failing for you 
> (e.g. `clang/test/Sema/array-init.c:149`)?

"expected-error" matches a substring, that's why they are not failing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141283

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

[PATCH] D139774: [libclang] Add API to set temporary directory location

[Igor Kushnir via Phabricator via cfe-commits]

vedgy added a comment.

In D139774#4039975 , @aaron.ballman 
wrote:

> Oh that is a good point! Apologies for not noticing that earlier -- that 
> makes me wonder if a better approach here is to add a `std::string` to the 
> `CIndexer` class to represent the temp path to use.

I have suggested the possibility in this review:

> A copy of user-provided temporary directory path buffer can be stored in 
> class `CIndexer`. But this API in //llvm/Support/Path.h// would stay fragile.

So the question is whether the LLVM API or `CIndexer` should store the buffer.

The only possible caller of `system_temp_directory()` used by libclang is 
`llvm::sys::fs::createUniquePath()`. This helper function is called by 
`createUniqueEntity()`, which in turn is called by several other helper 
functions, all in //llvm/lib/Support/Path.cpp//. Here is a backtrace from 
KDevelop built against LLVM at this review's branch:

  #0 llvm::sys::path::system_temp_directory(bool, llvm::SmallVectorImpl&) 
(ErasedOnReboot=true, Result=...) at 
/home/Mint14_home/igor/Documents/C/LinuxProjects/external/llvm-project/llvm/lib/Support/Unix/Path.inc:1451
  #1 0x7fb372a55d60 in llvm::sys::fs::createUniquePath(llvm::Twine const&, 
llvm::SmallVectorImpl&, bool) (Model=, ResultPath=..., 
MakeAbsolute=) at 
/home/Mint14_home/igor/Documents/C/LinuxProjects/external/llvm-project/llvm/lib/Support/Path.cpp:811
  #2 0x7fb372a55eb7 in createUniqueEntity(llvm::Twine const&, int&, 
llvm::SmallVectorImpl&, bool, FSEntity, llvm::sys::fs::OpenFlags, 
unsigned int) (Model=..., ResultFD=@0x7fb36effc2d0: 0, ResultPath=..., 
MakeAbsolute=, Type=FS_File, Flags=llvm::sys::fs::OF_None, 
Mode=384) at 
/home/Mint14_home/igor/Documents/C/LinuxProjects/external/llvm-project/llvm/lib/Support/Path.cpp:181
  #3 0x7fb372a5623b in llvm::sys::fs::createTemporaryFile 
(Flags=llvm::sys::fs::OF_None, Type=FS_File, ResultPath=..., 
ResultFD=@0x7fb36effc2d0: 0, Model=...) at 
/home/Mint14_home/igor/Documents/C/LinuxProjects/external/llvm-project/llvm/include/llvm/ADT/Twine.h:233
  #4 llvm::sys::fs::createTemporaryFile(llvm::Twine const&, llvm::StringRef, 
int&, llvm::SmallVectorImpl&, FSEntity, llvm::sys::fs::OpenFlags) 
(Prefix=, Suffix=..., ResultFD=@0x7fb36effc2d0: 0, 
ResultPath=..., Type=Type@entry=FS_File, Flags=llvm::sys::fs::OF_None) at 
/home/Mint14_home/igor/Documents/C/LinuxProjects/external/llvm-project/llvm/lib/Support/Path.cpp:865
  #5 0x7fb372a56451 in llvm::sys::fs::createTemporaryFile(llvm::Twine 
const&, llvm::StringRef, int&, llvm::SmallVectorImpl&, 
llvm::sys::fs::OpenFlags) (Prefix=, Suffix=..., 
ResultFD=, ResultPath=, Flags=) at 
/home/Mint14_home/igor/Documents/C/LinuxProjects/external/llvm-project/llvm/lib/Support/Path.cpp:873
  #6 0x7fb3717cc1b7 in (anonymous namespace)::TempPCHFile::create () at 
/home/Mint14_home/igor/Documents/C/LinuxProjects/external/llvm-project/llvm/include/llvm/ADT/Twine.h:233
  #7 clang::PrecompiledPreamble::Build(clang::CompilerInvocation const&, 
llvm::MemoryBuffer const*, clang::PreambleBounds, clang::DiagnosticsEngine&, 
llvm::IntrusiveRefCntPtr, 
std::shared_ptr, bool, 
clang::PreambleCallbacks&) (Invocation=..., MainFileBuffer=0x7fb35000a360, 
Bounds=Bounds@entry=..., Diagnostics=..., VFS=..., 
PCHContainerOps=std::shared_ptr (use count 5, 
weak count 0) = {...}, StoreInMemory=false, Callbacks=...) at 
/home/igor/Documents/C/LinuxProjects/external/llvm-project/clang/lib/Frontend/PrecompiledPreamble.cpp:421
  #8 0x7fb3717234a8 in 
clang::ASTUnit::getMainBufferWithPrecompiledPreamble(std::shared_ptr,
 clang::CompilerInvocation&, llvm::IntrusiveRefCntPtr, 
bool, unsigned int) (this=0x7fb3505c44d0, 
PCHContainerOps=std::shared_ptr (use count 5, 
weak count 0) = {...}, PreambleInvocationIn=..., VFS=..., 
AllowRebuild=, MaxLines=0) at 
/usr/include/c++/12.2.0/bits/unique_ptr.h:191
  #9 0x7fb371729bd6 in 
clang::ASTUnit::LoadFromCompilerInvocation(std::shared_ptr,
 unsigned int, llvm::IntrusiveRefCntPtr) 
(this=0x7fb3505c44d0, 
PCHContainerOps=std::shared_ptr (use count 5, 
weak count 0) = {...}, PrecompilePreambleAfterNParses=, VFS=...) 
at 
/home/igor/Documents/C/LinuxProjects/external/llvm-project/clang/lib/Frontend/ASTUnit.cpp:1685
  #10 0x7fb37172e359 in clang::ASTUnit::LoadFromCommandLine(char const**, 
char const**, std::shared_ptr, 
llvm::IntrusiveRefCntPtr, llvm::StringRef, bool, 
clang::CaptureDiagsKind, 
llvm::ArrayRef, std::allocator >, llvm::MemoryBuffer*> >, bool, 
unsigned int, clang::TranslationUnitKind, bool, bool, bool, 
clang::SkipFunctionBodiesScope, bool, bool, bool, bool, 
llvm::Optional, std::unique_ptr >*, 
llvm::IntrusiveRefCntPtr) (ArgBegin=, 
ArgEnd=, 
PCHContainerOps=std::shared_ptr (empty) = {...}, 
Diags=..., ResourceFilesPath=..., OnlyLocalDecls=false, 
CaptureDiagnostics=clang::CaptureDiagsKind::All, RemappedFiles=..., 
RemappedFilesKeepOriginalName=true, PrecompilePreambleAfterNParses=1, 
TUKind=clang::TU_Complete, CacheCodeCom

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

This causes codegen to be different depending on whether debug-info is 
generated. Please don't do that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

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

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

I should expand on my previous comment.  Extending the lifetime of these 
variables by saving the pointers into local variables, that's fine; just make 
sure it's not conditional on -g.  The only difference between -g and not -g 
should be metadata and dbg.* instructions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

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

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[Felipe de Azevedo Piovezan via Phabricator via cfe-commits]

fdeazeve added a comment.

In D141381#4040071 , @probinson wrote:

> just make sure it's not conditional on -g.

This makes sense, I'll update the patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

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

[PATCH] D141050: [standalone-build] outsource, simplify and unify repetitive CMake code

[Konrad Wilhelm Kleine via Phabricator via cfe-commits]

kwk updated this revision to Diff 487826.
kwk added a comment.

- Make require_llvm_utility_binary_path usable from a build-tree setup


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141050

Files:
  clang/CMakeLists.txt
  cmake/Modules/StandaloneBuildHelpers.cmake
  lld/CMakeLists.txt

Index: lld/CMakeLists.txt
===
--- lld/CMakeLists.txt
+++ lld/CMakeLists.txt
@@ -23,23 +23,16 @@
 # Must go below project(..)
 include(GNUInstallDirs)
 
-if(LLD_BUILT_STANDALONE)
-  set(CMAKE_CXX_STANDARD 17 CACHE STRING "C++ standard to conform to")
-  set(CMAKE_CXX_STANDARD_REQUIRED YES)
-  set(CMAKE_CXX_EXTENSIONS NO)
-
-  set(CMAKE_INCLUDE_CURRENT_DIR ON)
-
-  find_package(LLVM REQUIRED HINTS "${LLVM_CMAKE_DIR}")
-  list(APPEND CMAKE_MODULE_PATH "${LLVM_DIR}")
-
-  # Turn into CACHE PATHs for overwritting
-  set(LLVM_INCLUDE_DIRS ${LLVM_INCLUDE_DIRS} CACHE PATH "Path to llvm/include and any other header dirs needed")
-  set(LLVM_BINARY_DIR "${LLVM_BINARY_DIR}" CACHE PATH "Path to LLVM build tree")
-  set(LLVM_MAIN_SRC_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../llvm" CACHE PATH "Path to LLVM source tree")
+# Make sure that our source directory is on the current cmake module path so that
+# we can include cmake files from this directory.
+list(INSERT CMAKE_MODULE_PATH 0
+  "${CMAKE_CURRENT_SOURCE_DIR}/cmake/modules"
+  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
+  )
 
-  find_program(LLVM_TABLEGEN_EXE "llvm-tblgen" ${LLVM_TOOLS_BINARY_DIR}
-NO_DEFAULT_PATH)
+if(LLD_BUILT_STANDALONE)
+  include(StandaloneBuildHelpers)
+  require_llvm_utility_binary_path("llvm-tblgen" LLVM_TABLEGEN_EXE)
 
   # They are used as destination of target generators.
   set(LLVM_RUNTIME_OUTPUT_INTDIR ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin)
@@ -59,53 +52,17 @@
   if(LLVM_INCLUDE_TESTS)
 find_package(Python3 ${LLVM_MINIMUM_PYTHON_VERSION} REQUIRED
   COMPONENTS Interpreter)
-
-# Check prebuilt llvm/utils.
-if(EXISTS ${LLVM_TOOLS_BINARY_DIR}/FileCheck${CMAKE_EXECUTABLE_SUFFIX}
-AND EXISTS ${LLVM_TOOLS_BINARY_DIR}/not${CMAKE_EXECUTABLE_SUFFIX})
-  set(LLVM_UTILS_PROVIDED ON)
-endif()
-
-if(EXISTS ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
-  # Note: path not really used, except for checking if lit was found
-  set(LLVM_LIT ${LLVM_MAIN_SRC_DIR}/utils/lit/lit.py)
-  if(NOT LLVM_UTILS_PROVIDED)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/FileCheck utils/FileCheck)
-add_subdirectory(${LLVM_MAIN_SRC_DIR}/utils/not utils/not)
-set(LLVM_UTILS_PROVIDED ON)
-set(LLD_TEST_DEPS FileCheck not)
-  endif()
-  set(UNITTEST_DIR ${LLVM_THIRD_PARTY_DIR}/unittest)
-  if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h
-  AND NOT EXISTS ${LLVM_LIBRARY_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}gtest${CMAKE_STATIC_LIBRARY_SUFFIX}
-  AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
-add_subdirectory(${UNITTEST_DIR} third-party/unittest)
-  endif()
-else()
-  # Seek installed Lit.
-  find_program(LLVM_LIT
-   NAMES llvm-lit lit.py lit
-   PATHS "${LLVM_MAIN_SRC_DIR}/utils/lit"
-   DOC "Path to lit.py")
-endif()
-
-if(LLVM_LIT)
-  # Define the default arguments to use with 'lit', and an option for the user
-  # to override.
-  set(LIT_ARGS_DEFAULT "-sv")
-  if (MSVC OR XCODE)
-set(LIT_ARGS_DEFAULT "${LIT_ARGS_DEFAULT} --no-progress-bar")
-  endif()
-  set(LLVM_LIT_ARGS "${LIT_ARGS_DEFAULT}" CACHE STRING "Default options for lit")
-
-  get_errc_messages(LLVM_LIT_ERRC_MESSAGES)
-
-  # On Win32 hosts, provide an option to specify the path to the GnuWin32 tools.
-  if(WIN32 AND NOT CYGWIN)
-set(LLVM_LIT_TOOLS_DIR "" CACHE PATH "Path to GnuWin32 tools")
-  endif()
-else()
-  set(LLVM_INCLUDE_TESTS OFF)
+find_external_lit()
+set_lit_defaults()
+require_llvm_utility_binary_path("FileCheck")
+require_llvm_utility_binary_path("count")
+require_llvm_utility_binary_path("not")
+
+set(UNITTEST_DIR ${LLVM_THIRD_PARTY_DIR}/unittest)
+if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h
+AND NOT EXISTS ${LLVM_LIBRARY_DIR}/${CMAKE_STATIC_LIBRARY_PREFIX}gtest${CMAKE_STATIC_LIBRARY_SUFFIX}
+AND EXISTS ${UNITTEST_DIR}/CMakeLists.txt)
+  add_subdirectory(${UNITTEST_DIR} third-party/unittest)
 endif()
   endif()
 
@@ -153,12 +110,6 @@
 "`CMakeFiles'. Please delete them.")
 endif()
 
-# Add path for custom modules.
-list(INSERT CMAKE_MODULE_PATH 0
-  "${LLD_SOURCE_DIR}/cmake/modules"
-  "${LLVM_COMMON_CMAKE_UTILS}/Modules"
-  )
-
 include(AddLLD)
 
 option(LLD_USE_VTUNE
Index: cmake/Modules/StandaloneBuildHelpers.cmake
===
--- /dev/null
+++ cmake/Modules/StandaloneBuildHelpers.cmake
@@ -0,0 +1,124

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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:3863
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {

erichkeane wrote:
> Hmm... it is strange to me that the variables 'endloc' is the end of the 
> identifier and not the end of the variable declaration.  I unfortunately 
> don't have a good feeling as to the 'correct' behavior for that (Aaron is 
> typically the one who understands source locations better than me!), so 
> hopefully he can come along and see.  
I don't think we should have to do this dance here, this is something that 
`getEndLoc()` should be able to tell us. The way that source locations work is 
that AST nodes can override `getSourceRange()` to produce the correct range 
information for the node, and then they expose different accessors for any 
other source location information. In this case, I think 
`VarTemplateSpecializationDecl` isn't overloading `getSourceRange()` and so 
we're getting the default behavior from `VarDecl`.



Comment at: clang/test/FixIt/fixit-const-var-init.cpp:24
+
+template <> constexpr float d; // expected-error {{must be 
initialized by a constant expression}}
+// CHECK: fix-it:"{{.*}}":{24:42-24:42}:" = 0.0"

I'd like to see some additional test coverage for more complicated declarators:
```
void (* const func)(int, int);
const int var [[clang::annotate("")]];
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

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

[PATCH] D141144: [clang-tidy][doc] Improve clang-tidy documentation

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:34
 
+static StringRef TrimFirstChar(StringRef x) { return x.substr(1); }
+

This seems a bit of a needless change,  if you want to remove the newline, just 
in date the raw string literal and remove the empty line



Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:53
+- key:   check-a.SomeOption
+  value: value-a
+- key:   check-b.OtherOption

Can this be changed to match the previous format defined below 



Comment at: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp:82
 CheckOptions:
-  some-check.SomeOption: 'some value'
+  - key:some-check.SomeOption
+value: 'some value'

This change can be reverted.



Comment at: clang-tools-extra/docs/clang-tidy/index.rst:261
+CheckOptions:
+  - key:   check-a.SomeOption
+value: value-a

Again as before 



Comment at: clang-tools-extra/docs/clang-tidy/index.rst:292
+- key:some-check.SomeOption
+  value: 'some value'
   ...

Ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141144

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

[PATCH] D135495: [clang-tidy] handle pointers in `ExceptionAnalyzer`

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:60
+// Checks if T1 is convertible to T2.
+static bool isMultiLevelConvertiblePointer(QualType P1, QualType P2,
+   unsigned CurrentLevel = 0,

Did you take a look at `ASTContext::hasCvrSimilarType`? I wonder if it is 
already doing what you want. 


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

https://reviews.llvm.org/D135495

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

[PATCH] D139986: [clang][TypePrinter] Teach isSubstitutedDefaultArgument about integral types

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

@dblaikie

We added the libc++ tests to the Clang pre-commit CI after discussing with 
@erichkeane since he told me breaking libc++ was a recurring pain point, and 
having a way to detect that would be greatly appreciated by the Clang folks. 
The goal was really only to help Clang developers catch more issues earlier. I 
believe this confusion is only the result of miscommunication within the Clang 
community -- it seems that not all Clang developers know equally well what 
tools are available to them and what each bit of infrastructure should be used 
for. @erichkeane @aaron.ballman Would it make sense for you folks to post 
something on Discourse to explain what the expectations are w.r.t. Clang 
pre-commit CI?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139986

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

[PATCH] D135405: fix handling of braced-init temporaries for modernize-use-emplace

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D135405#4039820 , @BigPeet wrote:

> Ping.
>
> (Sorry, it's my first time contributing to LLVM and I simply don't know what 
> happens next. Do I need to do anything? Or is it just waiting to get merged 
> at some point?)

My apologies, what is your GitHub username and email so I can attribute you as 
the author of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135405

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

[PATCH] D139986: [clang][TypePrinter] Teach isSubstitutedDefaultArgument about integral types

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D139986#4040185 , @ldionne wrote:

> @dblaikie
>
> We added the libc++ tests to the Clang pre-commit CI after discussing with 
> @erichkeane since he told me breaking libc++ was a recurring pain point, and 
> having a way to detect that would be greatly appreciated by the Clang folks. 
> The goal was really only to help Clang developers catch more issues earlier. 
> I believe this confusion is only the result of miscommunication within the 
> Clang community -- it seems that not all Clang developers know equally well 
> what tools are available to them and what each bit of infrastructure should 
> be used for. @erichkeane @aaron.ballman Would it make sense for you folks to 
> post something on Discourse to explain what the expectations are w.r.t. Clang 
> pre-commit CI?

That is DEFINITELY true, the Clang pre-commit CI is historically really 
unreliable, so seeing a failed CI is something that even the most experienced 
of us are ignoring quite a bit thanks to its unreliability.  An announcement of 
some sort that we CAN trust the libc++ one is perhaps a good idea.  Perhaps 
I'll work with Aaron to come up with a message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139986

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

[PATCH] D140809: [clang][Interp] Implement logical and/or operators

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:387
+  // Logical AND.
+  // Visit LHS. Only visit RHS is LHS was TRUE.
+  LabelTy LabelFalse = this->getLabel();




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140809

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

[PATCH] D141403: [AArch64] Add command line support for v9.4-A's Instrumentation Extension

[Lucas Prates via Phabricator via cfe-commits]

pratlucas created this revision.
Herald added a subscriber: kristof.beyls.
Herald added a project: All.
pratlucas requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.

This introduces command line support (`+ite`) for the v9.4-A's
Instrumentation Extension (FEAT_ITE).

Patch by Son Tuan Vu.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141403

Files:
  clang/test/Driver/aarch64-ite.c
  llvm/include/llvm/TargetParser/AArch64TargetParser.def
  llvm/include/llvm/TargetParser/AArch64TargetParser.h
  llvm/unittests/TargetParser/TargetParserTest.cpp


Index: llvm/unittests/TargetParser/TargetParserTest.cpp
===
--- llvm/unittests/TargetParser/TargetParserTest.cpp
+++ llvm/unittests/TargetParser/TargetParserTest.cpp
@@ -1610,6 +1610,7 @@
   AArch64::AEK_B16B16,  AArch64::AEK_SMEF16F16, AArch64::AEK_CSSC,
   AArch64::AEK_RCPC3,   AArch64::AEK_THE,   AArch64::AEK_D128,
   AArch64::AEK_LSE128,  AArch64::AEK_SPECRES2,  AArch64::AEK_RASv2,
+  AArch64::AEK_ITE,
   };
 
   std::vector Features;
@@ -1681,6 +1682,7 @@
   EXPECT_TRUE(llvm::is_contained(Features, "+d128"));
   EXPECT_TRUE(llvm::is_contained(Features, "+lse128"));
   EXPECT_TRUE(llvm::is_contained(Features, "+specres2"));
+  EXPECT_TRUE(llvm::is_contained(Features, "+ite"));
 
   // Assuming we listed every extension above, this should produce the same
   // result. (note that AEK_NONE doesn't have a name so it won't be in the
Index: llvm/include/llvm/TargetParser/AArch64TargetParser.h
===
--- llvm/include/llvm/TargetParser/AArch64TargetParser.h
+++ llvm/include/llvm/TargetParser/AArch64TargetParser.h
@@ -146,6 +146,7 @@
   AEK_LSE128 =  1ULL << 52, // FEAT_LSE128
   AEK_SPECRES2 =1ULL << 53, // FEAT_SPECRES2
   AEK_RASv2 =   1ULL << 54, // FEAT_RASv2
+  AEK_ITE = 1ULL << 55, // FEAT_ITE
 };
 // clang-format on
 
Index: llvm/include/llvm/TargetParser/AArch64TargetParser.def
===
--- llvm/include/llvm/TargetParser/AArch64TargetParser.def
+++ llvm/include/llvm/TargetParser/AArch64TargetParser.def
@@ -217,6 +217,7 @@
 AARCH64_ARCH_EXT_NAME("d128", AArch64::AEK_D128, "+d128", "-d128", MAX, "", 0)
 AARCH64_ARCH_EXT_NAME("lse128", AArch64::AEK_LSE128, "+lse128", "-lse128", MAX,
   "", 0)
+AARCH64_ARCH_EXT_NAME("ite", AArch64::AEK_ITE, "+ite", "-ite", MAX, "", 0)
 AARCH64_ARCH_EXT_NAME("sha1", AArch64::AEK_NONE, {}, {}, SHA1,
   "+fp-armv8,+neon", 120)
 AARCH64_ARCH_EXT_NAME("pmull", AArch64::AEK_NONE, {}, {}, PMULL,
Index: clang/test/Driver/aarch64-ite.c
===
--- /dev/null
+++ clang/test/Driver/aarch64-ite.c
@@ -0,0 +1,17 @@
+// Test that target feature ite is implemented and available correctly
+
+// FEAT_ITE is optional (off by default) for v8.9a/9.4a and older, and can be 
enabled using +ite
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a   %s 
2>&1 | FileCheck %s --check-prefix=NOT_ENABLED
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+ite   %s 
2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+noite %s 
2>&1 | FileCheck %s --check-prefix=DISABLED
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a   %s 
2>&1 | FileCheck %s --check-prefix=NOT_ENABLED
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+ite   %s 
2>&1 | FileCheck %s --check-prefix=ENABLED
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+noite %s 
2>&1 | FileCheck %s --check-prefix=DISABLED
+
+// FEAT_ITE is invalid before v8
+// RUN: %clang -### -target arm-none-none-eabi -march=armv7-a+ite %s 2>&1 
| FileCheck %s --check-prefix=INVALID
+
+// INVALID: error: unsupported argument 'armv7-a+ite' to option '-march='
+// ENABLED: "-target-feature" "+ite"
+// NOT_ENABLED-NOT: "-target-feature" "+ite"
+// DISABLED: "-target-feature" "-ite"


Index: llvm/unittests/TargetParser/TargetParserTest.cpp
===
--- llvm/unittests/TargetParser/TargetParserTest.cpp
+++ llvm/unittests/TargetParser/TargetParserTest.cpp
@@ -1610,6 +1610,7 @@
   AArch64::AEK_B16B16,  AArch64::AEK_SMEF16F16, AArch64::AEK_CSSC,
   AArch64::AEK_RCPC3,   AArch64::AEK_THE,   AArch64::AEK_D128,
   AArch64::AEK_LSE128,  AArch64::AEK_SPECRES2,  AArch64::AEK_RASv2,
+  AArch64::AEK_ITE,
   };
 
   std::vector Features;
@@ -1681,6 +1682,7 @@
   EXPECT_TRUE(llvm::is_contained(Features, "+d128"));
   EXPECT_TRUE(llvm::is_contained(Features, "+lse128"));
   EXPECT_TRUE(llvm::is_contained(Features, "+specres2"));
+  EXPECT_TRUE(llvm::is_contained(Features, "+ite"));
 
   // Assuming 

[PATCH] D140639: clang: Fix handling of __builtin_elementwise_copysign

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.
Herald added a subscriber: StephenFan.

ping, I think this should get in before the branch date to fix the current 
broken behavior before this is in a release


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

https://reviews.llvm.org/D140639

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

[PATCH] D141404: [AArch64][Clang] Adjust default features for v8.9-A/v9.4-A in clang driver

[Lucas Prates via Phabricator via cfe-commits]

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

Update the clang driver to include the following features as default for
the v8.9-A/v9.4-A architecture versions:

- FEAT_SPECRES2
- FEAT_CSSC
- FEAT_RASv2

Patch by Sam Elliott.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141404

Files:
  clang/lib/Driver/ToolChains/Arch/AArch64.cpp
  clang/test/Driver/aarch64-cssc.c


Index: clang/test/Driver/aarch64-cssc.c
===
--- clang/test/Driver/aarch64-cssc.c
+++ clang/test/Driver/aarch64-cssc.c
@@ -1,12 +1,13 @@
 // Test that target feature cssc is implemented and available correctly
+// FEAT_CSSC is a required part of v8.9a/v9.4a and optional from v8.7a/v9.3a 
onwards.
 // RUN: %clang -### -target aarch64-none-none-eabi %s 
2>&1 | FileCheck %s --check-prefix=ABSENT_CSSC
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+cssc   %s 
2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.9-a%s 
2>&1 | FileCheck %s --check-prefix=ABSENT_CSSC
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+cssc   %s 
2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.7-a+cssc   %s 
2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.9-a%s 
2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.9-a+cssc   %s 
2>&1 | FileCheck %s
 // RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.9-a+nocssc %s 
2>&1 | FileCheck %s --check-prefix=NO_CSSC
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+cssc   %s 
2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.4-a%s 
2>&1 | FileCheck %s --check-prefix=ABSENT_CSSC
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+cssc   %s 
2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.2-a+cssc   %s 
2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.4-a%s 
2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.4-a+cssc   %s 
2>&1 | FileCheck %s
 // RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.4-a+nocssc %s 
2>&1 | FileCheck %s --check-prefix=NO_CSSC
 
 // CHECK: "-target-feature" "+cssc"
Index: clang/lib/Driver/ToolChains/Arch/AArch64.cpp
===
--- clang/lib/Driver/ToolChains/Arch/AArch64.cpp
+++ clang/lib/Driver/ToolChains/Arch/AArch64.cpp
@@ -498,6 +498,11 @@
 Features.insert(std::next(Features.begin() + ArchFeatPos),
 {"+hbc", "+mops"});
 
+  // Default features for Armv8.9-a/Armv9.4-a or later.
+  if (V8Version >= 9 || V9Version >= 4)
+Features.insert(std::next(Features.begin() + ArchFeatPos),
+{"+specres2", "+cssc", "+rasv2"});
+
   if (Arg *A = Args.getLastArg(options::OPT_mno_unaligned_access,
options::OPT_munaligned_access)) {
 if (A->getOption().matches(options::OPT_mno_unaligned_access))


Index: clang/test/Driver/aarch64-cssc.c
===
--- clang/test/Driver/aarch64-cssc.c
+++ clang/test/Driver/aarch64-cssc.c
@@ -1,12 +1,13 @@
 // Test that target feature cssc is implemented and available correctly
+// FEAT_CSSC is a required part of v8.9a/v9.4a and optional from v8.7a/v9.3a onwards.
 // RUN: %clang -### -target aarch64-none-none-eabi %s 2>&1 | FileCheck %s --check-prefix=ABSENT_CSSC
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+cssc   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.9-a%s 2>&1 | FileCheck %s --check-prefix=ABSENT_CSSC
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.8-a+cssc   %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.7-a+cssc   %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.9-a%s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.9-a+cssc   %s 2>&1 | FileCheck %s
 // RUN: %clang -### -target aarch64-none-none-eabi -march=armv8.9-a+nocssc %s 2>&1 | FileCheck %s --check-prefix=NO_CSSC
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+cssc   %s 2>&1 | FileCheck %s
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.4-a%s 2>&1 | FileCheck %s --check-prefix=ABSENT_CSSC
-// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.3-a+cssc   %s 2>&1 | FileCheck %s
+// RUN: %clang -### -target aarch64-none-none-eabi -march=armv9.2-a+cssc  

[PATCH] D135495: [clang-tidy] handle pointers in `ExceptionAnalyzer`

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:60
+// Checks if T1 is convertible to T2.
+static bool isMultiLevelConvertiblePointer(QualType P1, QualType P2,
+   unsigned CurrentLevel = 0,

xazax.hun wrote:
> Did you take a look at `ASTContext::hasCvrSimilarType`? I wonder if it is 
> already doing what you want. 
Oh, maybe it is not, but it might also make sense to take a look at 
`ASTContext::typesAreCompatible`.


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

https://reviews.llvm.org/D135495

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

[PATCH] D135495: [clang-tidy] handle pointers in `ExceptionAnalyzer`

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/ExceptionAnalyzer.cpp:95
+
+  if (P1->isArrayType() && P2->isArrayType()) {
+// At every level that array type is involved in, at least

If none of the functions I recommended works for you, I still strongly suggest 
reusing utilities from ASTContext, like `UnwrapSimilarTypes` and 
`UnwrapSimilarArrayTypes`.


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

https://reviews.llvm.org/D135495

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

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

Ah, so the intent is that this causes these indirect args to be handled the 
same way as other arguments at -O0 - placed in an alloca, eg:

  struct t1 {
t1(const t1&);
  };
  void f1(int, int);
  void f2(t1 v, int x) {
f1(3, 4);
  }

  0x0033: DW_TAG_formal_parameter
DW_AT_location(indexed (0x0) loclist = 0x0010: 
   [0x, 0x0010): DW_OP_breg5 
RDI+0
   [0x0010, 0x0020): 
DW_OP_entry_value(DW_OP_reg5 RDI))
DW_AT_name("v")
  ...
  
  0x003c: DW_TAG_formal_parameter
DW_AT_location(DW_OP_fbreg -4)
DW_AT_name("x")
  ...

Though I guess it doesn't change the codegen so that the alloca is used to load 
the value like with other values. Maybe it'd be worth changing the codegen to 
be more similar in that way? But maybe not. Don't know.

It seems plausible - though the impact on -O0 will be a question - if this 
hurts code size/perf too much (there's /some/ line there, but I don't know what 
it is), it couldn't go in -O0 and I'm not sure where it'd go. Logically `-Og` 
would make sense, but that's `-O1` which runs mem2reg anyway, so would lose the 
alloca, which doesn't help...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

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

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Nathan James via Phabricator via cfe-commits]

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

Can you run this through clang format to make sure the pre merge is happy and 
address those nits




Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:81
+  Lambda->getCaptureDefaultLoc(),
+  "lambdas that capture 'this' should not specify a capture default");
+

Would be nice to show if `this` is implicitly captured.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst:6
+
+Warns when lambda specify a capture default and capture ``this``. The check 
also
+offers fix-its.

Not necessary to specify fix behaviour as that's provided in the check list



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst:19
+  struct AClass {
+int DataMember;
+void misleadingLogic() {

Please fix these examples as this isn't actually captured because the names 
don't match.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:10
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture 'this' 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-default-when-capturing-this]
+// CHECK-FIXES: [this]() { };
+

Please can you include the whole expected line in the check fix markers, same 
goes for all tests below 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

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

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

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:3863
 
+  SourceLocation EndLoc = VD->getEndLoc();
+  if (const auto *VTSD = dyn_cast(VD)) {

aaron.ballman wrote:
> erichkeane wrote:
> > Hmm... it is strange to me that the variables 'endloc' is the end of the 
> > identifier and not the end of the variable declaration.  I unfortunately 
> > don't have a good feeling as to the 'correct' behavior for that (Aaron is 
> > typically the one who understands source locations better than me!), so 
> > hopefully he can come along and see.  
> I don't think we should have to do this dance here, this is something that 
> `getEndLoc()` should be able to tell us. The way that source locations work 
> is that AST nodes can override `getSourceRange()` to produce the correct 
> range information for the node, and then they expose different accessors for 
> any other source location information. In this case, I think 
> `VarTemplateSpecializationDecl` isn't overloading `getSourceRange()` and so 
> we're getting the default behavior from `VarDecl`.
Sorry! I'm confused here. Do you mean we should use `getEndLoc()` directly as 
the location where the fix-hint insert? But isn't the original code using 
`getEndLoc()`, it turns out to add the fix-hint after the end of the identifier 
instead of the right angle of `VarTemplateSpecializationDecl`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139705

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

[PATCH] D141381: [codegen] Store address of indirect arguments on the stack

[Felipe de Azevedo Piovezan via Phabricator via cfe-commits]

fdeazeve added a comment.

In D141381#4040314 , @dblaikie wrote:

> Ah, so the intent is that this causes these indirect args to be handled the 
> same way as other arguments at -O0 - placed in an alloca, eg:

Yup, that's correct!

> Though I guess it doesn't change the codegen so that the alloca is used to 
> load the value like with other values. Maybe it'd be worth changing the 
> codegen to be more similar in that way?

Note that the situation here is slightly different: loading this new alloca 
would _not_ produce the value, it would produce the address of the value 
(because the argument is indirect). In that way, I'm not sure we would gain 
anything by loading from the alloca. Does that make sense?

> though the impact on -O0 will be a question

Indeed. I'm not exactly sure what the right tradeoffs here are, but will think 
about this some more.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141381

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