[clang] 5f667a5 - [clang][dataflow] Fix -Wunused-const-variable in WatchedLiteralsSolver.cpp (NFC)

2023-09-05 Thread Jie Fu via cfe-commits 

Author: Jie Fu
Date: 2023-09-05T15:03:07+08:00
New Revision: 5f667a57b5329f449d27f514d041d457b1ea9da6

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

LOG: [clang][dataflow] Fix -Wunused-const-variable in WatchedLiteralsSolver.cpp 
(NFC)

/data/llvm-project/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp:62:26:
 error: unused variable 'NullLit' [-Werror,-Wunused-const-variable]
static constexpr Literal NullLit = 0;

Added: 


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

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp 
b/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
index 05ae36ddbc898e..ab3a8104e31717 100644
--- a/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
+++ b/clang/lib/Analysis/FlowSensitive/WatchedLiteralsSolver.cpp
@@ -59,7 +59,7 @@ using Literal = uint32_t;
 
 /// A null literal is used as a placeholder in various data structures and
 /// algorithms.
-static constexpr Literal NullLit = 0;
+[[maybe_unused]] static constexpr Literal NullLit = 0;
 
 /// Returns the positive literal `V`.
 static constexpr Literal posLit(Variable V) { return 2 * V; }



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

[PATCH] D159414: Haiku: Fixes for header / library paths

2023-09-05 Thread Trung Nguyen via Phabricator via cfe-commits 
trungnt2910 added a comment.

I'm not the Haiku toolchain expert here 😅, all I can do is comparing this with 
what the GCC toolchain for Haiku does.




Comment at: clang/lib/Driver/ToolChains/Haiku.cpp:24-25
 
+  getFilePaths().push_back(concat(getDriver().SysRoot, "/boot/system/lib"));
+  getFilePaths().push_back(concat(getDriver().SysRoot, 
"/boot/system/develop/lib"));
 }

Should we also add the corresponding non-packaged paths, like 
`/boot/system/non-packaged/lib` and `/boot/system/non-packaged/develop/lib`?

The latest GCC toolchain on Haiku does **not** do it, but neither does it 
recognize non-packaged files for the headers.



Comment at: clang/lib/Driver/ToolChains/Haiku.cpp:85-86
+   "/boot/system/develop/headers/os/net"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,
+   "/boot/system/develop/headers/os/opengl"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,

Not included by GCC, and not found in my Haiku installation.

I don't know what this is for though, so probably just keep it until someone 
else says otherwise.



Comment at: clang/lib/Driver/ToolChains/Haiku.cpp:105-106
+   "/boot/system/develop/headers/os/add-ons/tracker"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,
+   "/boot/system/develop/headers/os/be_apps/Deskbar"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,

This seems to have been removed from Haiku, see: 
https://github.com/haiku/haiku/commit/a3e794ae459fec76826407f8ba8c94cd3535f128

The latest Haiku GCC toolchain does not include it.



Comment at: clang/test/Driver/haiku.c:28
+// CHECK-C-HEADER-PATH: "-internal-isystem" 
"/boot/system/develop/headers/os/add-ons/tracker"
+// CHECK-C-HEADER-PATH: "-internal-isystem" 
"/boot/system/develop/headers/os/be_apps/Deskbar"
+// CHECK-C-HEADER-PATH: "-internal-isystem" 
"/boot/system/develop/headers/os/be_apps/NetPositive"

Should also remove this if the corresponding line in `Haiku.cpp` is removed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159414

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

[clang] 23915fe - [Serialization] Modernize ModuleInfo (NFC)

2023-09-05 Thread Kazu Hirata via cfe-commits 

Author: Kazu Hirata
Date: 2023-09-05T00:22:51-07:00
New Revision: 23915fe3015d3601c1b50d0ecfb8bf43c2fb03f4

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

LOG: [Serialization] Modernize ModuleInfo (NFC)

Added: 


Modified: 
clang/include/clang/Serialization/GlobalModuleIndex.h

Removed: 




diff  --git a/clang/include/clang/Serialization/GlobalModuleIndex.h 
b/clang/include/clang/Serialization/GlobalModuleIndex.h
index d82e0dd294b901..93d674e4400345 100644
--- a/clang/include/clang/Serialization/GlobalModuleIndex.h
+++ b/clang/include/clang/Serialization/GlobalModuleIndex.h
@@ -67,20 +67,20 @@ class GlobalModuleIndex {
 
   /// Information about a given module file.
   struct ModuleInfo {
-ModuleInfo() : File(), Size(), ModTime() { }
+ModuleInfo() = default;
 
 /// The module file, once it has been resolved.
-ModuleFile *File;
+ModuleFile *File = nullptr;
 
 /// The module file name.
 std::string FileName;
 
 /// Size of the module file at the time the global index was built.
-off_t Size;
+off_t Size = 0;
 
 /// Modification time of the module file at the time the global
 /// index was built.
-time_t ModTime;
+time_t ModTime = 0;
 
 /// The module IDs on which this module directly depends.
 /// FIXME: We don't really need a vector here.



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

[clang] a6fa39d - [StaticAnalyzer] Modernize NSErrorMethodChecker (NFC)

2023-09-05 Thread Kazu Hirata via cfe-commits 

Author: Kazu Hirata
Date: 2023-09-05T00:22:53-07:00
New Revision: a6fa39da39c40c50a750de51cc6224195fd9f166

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

LOG: [StaticAnalyzer] Modernize NSErrorMethodChecker (NFC)

Added: 


Modified: 
clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp

Removed: 




diff  --git a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp 
b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
index 59741dde1eeafb..54870bcb4bb22b 100644
--- a/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/NSErrorChecker.cpp
@@ -39,10 +39,10 @@ static bool IsCFError(QualType T, IdentifierInfo *II);
 namespace {
 class NSErrorMethodChecker
 : public Checker< check::ASTDecl > {
-  mutable IdentifierInfo *II;
+  mutable IdentifierInfo *II = nullptr;
 
 public:
-  NSErrorMethodChecker() : II(nullptr) {}
+  NSErrorMethodChecker() = default;
 
   void checkASTDecl(const ObjCMethodDecl *D,
 AnalysisManager &mgr, BugReporter &BR) const;



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

[clang] [AST] Fix nested name specifiers printing as NamespaceNamespace (PR #65266)

2023-09-05 Thread kadir çetinkaya via cfe-commits 

https://github.com/kadircet approved this pull request.


https://github.com/llvm/llvm-project/pull/65266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159284: [clang][dataflow] Fix Record initialization with InitListExpr and inheritances

2023-09-05 Thread Martin Böhme via Phabricator via cfe-commits 
mboehme added inline comments.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:688-690
+  for ([[maybe_unused]] auto [Field, Loc] : FieldLocs) {
+assert(ModeledFields.contains(cast_or_null(Field)));
+  }

https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:653-654
+auto Init = Inits[InitIdx++];
+assert(GetCanonicalType(Base.getType()) ==
+   GetCanonicalType(Init->getType()));
+if (Base.getType()->getAsCXXRecordDecl()->isLambda()) {

mboehme wrote:
> I believe this can be
> 
> ```
> assert(Base.getType().getCanonicalType() ==
>Init->getType().getCanonicalType());
> ```
> 
> - The initializer for a base class should always be a prvalue
> - And I believe the type of a prvalue can't have `const` or `volatile` 
> qualifiers (neither of them really makes sense)
Do you actually need the `Unqualified` (see also comment above)? Do you have an 
example of a specific scenario where we need to drop the qualifiers?



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:655
+   GetCanonicalType(Init->getType()));
+if (Base.getType()->getAsCXXRecordDecl()->isLambda()) {
+  // FIXME: Figure out what to do for lambdas.

kinu wrote:
> mboehme wrote:
> > - Can this actually happen? (How do you make a lambda a base class?)
> > - Why does this need to be handled separately? (Isn't a lambda just a 
> > normal class, modulo sugar? What is different about lambdas that means we 
> > can't handle them like other classes?)
> > 
> > Depending on the answers to the above, would also be useful to capture them 
> > as comments in the source code.
> > 
> > I assume you encountered this scenario somewhere. Can you add a test (with 
> > incorrect behavior marked as a FIXME) that exercises this?
> I hit this case in one of real code so added this afterwards...
> 
> but actually can I drop this part from this patch but come back in a separate 
> patch once I clarify the behavior?  I somehow lost the note about in which 
> file this happened, and I need to confirm a few things before I can add a 
> test :(
Sounds good!



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:672-673
+  // The types are same, or
+  GetCanonicalType(Field->getType()) ==
+  GetCanonicalType(Init->getType()) ||
+  // The field's type is T&, and initializer is T

mboehme wrote:
> Again, this is the prvalue case, so I think you should simply be able to use 
> `QualType::getCanonicalType()`.
Thinking about this again -- the field can, of course, have qualifiers, while 
the `Init` as a prvalue should not, so I think this should read

```
Field->getType().getCanonicalType().getUnqualifiedType() == 
Init->getType().getCanonicalType()
```

Sorry for the back and forth!



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1479-1481
+for (auto [Field, _] : MDLoc.children()) {
+  Fields.push_back(Field);
+}





Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1528-1530
+  F.x4 = F.x2;
+  F.x6 = 1;
+  F.x8 = 1;

mboehme wrote:
> These lines seem unnecessary. The test only wants to check whether the fields 
> are modeled, not what their values are.
> 
> If anything, leaving out these operations makes the test stronger: It then 
> tests whether the fields are modeled even if they are only initialized with 
> an empty `{}`.
> 
> Then, once we leave out these operations, it's probably sufficient to give 
> each class in this test just a single member variable (which is another nice 
> simplification).
Reopening the comment to discuss this line:

> Then, once we leave out these operations, it's probably sufficient to give 
> each class in this test just a single member variable (which is another nice 
> simplification).

I see that giving each class two member variables makes this test consistent 
with the test above -- but OTOH, we're testing exactly the same thing for both 
member variables, so maybe it's better on the whole to simplify the test by 
putting just one member variable in each class?



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2191
+TEST(TransferTest, AssignmentOperatorWithInitAndInheritance) {
+  // This is a crash repro.
+  std::string Code = R"(

kinu wrote:
> mboehme wrote:
> > IIUC, the crash was happening because `copyRecord()` assumes that the 
> > source and destination have the same fields. More specifically, there was 
> > an unspoken invariant in the framework that a `RecordStorageLocation` 
> > should always contain exactly the fields returned by 
>

[PATCH] D159441: [include-cleaner] Weaken signal for boosting preferred headers

2023-09-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Putting preferred header signal above completeness implied we would
uprank forward declarations above complete ones in certain cases.

This can be desired in cases where:

- Complete definition is private. But this case is already governed by 
publicness signal.
- The library indeed intends to provide a forward declaring interface, like 
iosfwd vs ostream.

In all other cases, upranking is undesired as it means we've picked up prefered
headerness signal by mistake from an unrelated declaration to the library.

This change regresses the behavior for libraries that intentionally provide a
forward declaring interface. But that wasn't something we intended to support
explicitly, it was working incidentally when the forward declaring header had a
similar name to the symbol. Moreover, include-cleaner deliberately discourages
forward-declarations, so not working in this case is also more aligned with rest
of the components.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D159441

Files:
  clang-tools-extra/include-cleaner/lib/TypesInternal.h
  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
@@ -344,7 +344,7 @@
 }
 
 TEST_F(HeadersForSymbolTest, Ranking) {
-  // Sorting is done over (canonical, public, complete, origin)-tuple.
+  // Sorting is done over (public, complete, canonical, origin)-tuple.
   Inputs.Code = R"cpp(
 #include "private.h"
 #include "public.h"
@@ -363,11 +363,11 @@
   )cpp");
   Inputs.ExtraFiles["public_complete.h"] = guard("struct foo {};");
   buildAST();
-  EXPECT_THAT(headersForFoo(), ElementsAre(Header("\"canonical.h\""),
-   physicalHeader("public_complete.h"),
-   physicalHeader("public.h"),
-   physicalHeader("exporter.h"),
-   physicalHeader("private.h")));
+  EXPECT_THAT(headersForFoo(),
+  ElementsAre(physicalHeader("public_complete.h"),
+  Header("\"canonical.h\""), 
physicalHeader("public.h"),
+  physicalHeader("exporter.h"),
+  physicalHeader("private.h")));
 }
 
 TEST_F(HeadersForSymbolTest, PreferPublicOverComplete) {
@@ -391,14 +391,11 @@
 #include "public_complete.h"
 #include "test/foo.fwd.h"
   )cpp";
-  Inputs.ExtraFiles["public_complete.h"] = guard(R"cpp(
-struct foo {};
-  )cpp");
+  Inputs.ExtraFiles["public_complete.h"] = guard("struct foo {};");
   Inputs.ExtraFiles["test/foo.fwd.h"] = guard("struct foo;");
   buildAST();
-  EXPECT_THAT(headersForFoo(),
-  ElementsAre(physicalHeader("test/foo.fwd.h"),
-  physicalHeader("public_complete.h")));
+  EXPECT_THAT(headersForFoo(), ElementsAre(physicalHeader("public_complete.h"),
+   physicalHeader("test/foo.fwd.h")));
 }
 
 TEST_F(HeadersForSymbolTest, MainFile) {
Index: clang-tools-extra/include-cleaner/lib/TypesInternal.h
===
--- clang-tools-extra/include-cleaner/lib/TypesInternal.h
+++ clang-tools-extra/include-cleaner/lib/TypesInternal.h
@@ -66,13 +66,13 @@
   /// Symbol is directly originating from this header, rather than being
   /// exported or included transitively.
   OriginHeader = 1 << 0,
-  /// Provides a generally-usable definition for the symbol. (a function decl,
-  /// or class definition and not a forward declaration of a template).
-  CompleteSymbol = 1 << 1,
   /// Header providing the symbol is explicitly marked as preferred, with an
   /// IWYU private pragma that points at this provider or header and symbol has
   /// ~the same name.
-  PreferredHeader = 1 << 2,
+  PreferredHeader = 1 << 1,
+  /// Provides a generally-usable definition for the symbol. (a function decl,
+  /// or class definition and not a forward declaration of a template).
+  CompleteSymbol = 1 << 2,
   /// Symbol is provided by a public file. Only absent in the cases where file
   /// is explicitly marked as such, non self-contained or IWYU private
   /// pragmas.


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
@@ -344,7 +344,7 @@
 }
 
 TEST_F(HeadersForSymbolTest, Ranking) {
-  // Sor

[PATCH] D159414: Haiku: Fixes for header / library paths

2023-09-05 Thread Brad Smith via Phabricator via cfe-commits 
brad added inline comments.



Comment at: clang/lib/Driver/ToolChains/Haiku.cpp:85-86
+   "/boot/system/develop/headers/os/net"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,
+   "/boot/system/develop/headers/os/opengl"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,

trungnt2910 wrote:
> Not included by GCC, and not found in my Haiku installation.
> 
> I don't know what this is for though, so probably just keep it until someone 
> else says otherwise.
> Not included by GCC, and not found in my Haiku installation.
> 
> I don't know what this is for though, so probably just keep it until someone 
> else says otherwise.

This is referring to the os/opengl directory?



Comment at: clang/lib/Driver/ToolChains/Haiku.cpp:105-106
+   "/boot/system/develop/headers/os/add-ons/tracker"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,
+   "/boot/system/develop/headers/os/be_apps/Deskbar"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,

trungnt2910 wrote:
> This seems to have been removed from Haiku, see: 
> https://github.com/haiku/haiku/commit/a3e794ae459fec76826407f8ba8c94cd3535f128
> 
> The latest Haiku GCC toolchain does not include it.
> This seems to have been removed from Haiku, see: 
> https://github.com/haiku/haiku/commit/a3e794ae459fec76826407f8ba8c94cd3535f128
> 
> The latest Haiku GCC toolchain does not include it.

https://github.com/haikuports/haikuports/blob/master/sys-devel/gcc/patches/gcc-13.2.0_2023_08_10.patchset

I still see the path listed in the GCC configs.

But I see in the Haiku I have installed in my VM that the path indeed is gone.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159414

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

[PATCH] D156605: [clangd][CodeComplete] Improve FunctionCanBeCall

2023-09-05 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge requested changes to this revision.
nridge added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1297
+FunctionCanBeCall =
+MaybeDerived == MaybeBase || MaybeDerived->isDerivedFrom(MaybeBase);
+  }

nit: we can avoid the computation in this block if `FunctionCanBeCall` was 
already initialized to `true` above



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:1387
 
-  // When completing a non-static member function (and not via
-  // dot/arrow member access) and we're not inside that class' scope,
-  // it can't be a call.
+  // Decide whether or not a non-static member function can be a call.
   if (CompletionContext.getKind() == clang::CodeCompletionContext::CCC_Symbol) 
{

zyounan wrote:
> sammccall wrote:
> > This is confusing: the `CCC_Symbol` test is part of the specific heuristics 
> > being used (it's the "not via dot/arrow member access" part, right?) but 
> > you've moved the comment explaining what it does.
> > 
> > Also this function is just getting too long, and we're inlining more 
> > complicated control flow here.
> > Can we extract a function?
> > 
> > ```
> > const auto *Method = ...;
> > if (Method & !Method->isStatic()) {
> >   R.FunctionCanBeCall = canMethodBeCalled(...);
> > }
> > ```
> > it's the "not via dot/arrow member access" part
> 
> (Sorry for being unaware of the historical context). But I think `CCC_Symbol` 
> should mean "we're completing a name such as a function or type name" per its 
> comment. The heuristic for dot/arrow member access actually lies on the next 
> line, i.e., if the completing decl is a CXXMethodDecl.
> 
> > Can we extract a function?
> 
> Sure. 
The check for `CompletionContext.getKind()` is in fact a part of the heuristic:

 * for `f.method`, the kind is `CCC_DotMemberAccess`
 * for `f->method`, the kind is `CCC_ArrowMemberAccess`
 * for `f.Foo::method` and `f->Foo::method`, the kind is `CCC_Symbol`

(I realize that's a bit inconsistent. Maybe the `f.Foo::` and `f->Foo::` cases 
should be using `DotMemberAccess` and `ArrowMemberAccess` as well? Anyways, 
that's a pre-existing issue.)

So, the `if (CompletionContext.getKind() == 
clang::CodeCompletionContext::CCC_Symbol)` check is what currently makes sure 
that in the `f.method` and `f->method` cases, we just keep `FunctionCanBeCall = 
true` without having to check any context or expression type.

I think it may be clearest to move this entire `if` block into the new function 
(whose name can be generalized to `canBeCall` or similar), and here just 
unconditionally set `R.FunctionCanBeCall = canBeCall(CompletionContext, /* 
other things */)`.



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3577
   // containing all of the arguments up to the first deducible argument.
+  // Or, if this isn't a call, emit all the template arguments
+  // to disambiguate the (potential) overloads.

zyounan wrote:
> nridge wrote:
> > 1. If this is not a call, we can avoid running the 
> > `Sema::MarkDeducedTemplateParameters` operation altogether.
> > 
> > 2. A future improvement we could consider: if this is not a call, try to 
> > detect cases where the template parameters can be deduced from the 
> > surrounding context (["Deducing template arguments taking the address of a 
> > function template "](https://eel.is/c++draft/temp.deduct.funcaddr)). Maybe 
> > add a FIXME for this?
> > avoid running the Sema::MarkDeducedTemplateParameters operation altogether
> 
> I think doing so could cause too many adjustments to the flow, and I'm afraid 
> that the `Sema::MarkDeducedTemplateParameters` would be required again when 
> we decide to implement point 2.
> 
> I'm adding a FIXME anyway but leave the first intact. However, I'd be happy 
> to rearrange the logic flow if you insist.
I realized there's actually an open question here: if `FunctionCanBeCall == 
false`, do we want to include **all** the template parameters, or just the 
non-deducible ones?

Let's consider an example:

```
class Foo {
  template 
  T convertTo(U from);
};

void bar() {
  Foo::^
}
```

Here, `U` can be deduced but `T` cannot.

The current behaviour is to complete `convertTo`. That seems appropriate for 
a **call**, since `U` will be deduced from the call. But since this is not a 
call, wouldn't it be  better to complete `convertTo`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156605

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

[PATCH] D158363: [clang-format] Fix segmentation fault when formatting nested namespaces

2023-09-05 Thread Arkadiy Yudintsev via Phabricator via cfe-commits 
d0nc1h0t added a comment.

In D158363#463 , @owenpan wrote:

> We need the name and email you want to use in order to commit the patch for 
> you.

Arkadiy Yudintsev 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158363

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

[PATCH] D158926: [clangd] Show parameter hints for operator()

2023-09-05 Thread Nathan Ridge via Phabricator via cfe-commits 
nridge accepted this revision.
nridge added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:626
+  Method && Method->isInstance())
+Args = Args.drop_front(1);
+processCall(Callee, Args);

Huh, that's an interesting inconsistency between CXXMemberCallExpr and 
CXXOperatorCallExpr (that one include th implied object argument in getArgs() 
and the other doesn't)

As always, thank you for writing thorough tests that give us confidence we're 
doing the right thing :)



Comment at: clang-tools-extra/clangd/InlayHints.cpp:1222
 
+  bool isFunctionObjectCallExpr(CallExpr *E) const noexcept {
+if (auto *CallExpr = dyn_cast(E))

nit: this method can be static


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158926

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

[PATCH] D158156: [analyzer] Add C++ array delete checker

2023-09-05 Thread Discookie via Phabricator via cfe-commits 
Discookie updated this revision to Diff 555821.
Discookie added a comment.

Refactored the checkers to use a shared base class, otherwise I kept the same 
functionality.

I'll push this diff in a way where the blame transfers between the files, but I 
don't think keeping the file name the same makes sense, since this is 
definitely not a DeleteWithNonVirtualDtorChecker.


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

https://reviews.llvm.org/D158156

Files:
  clang/docs/analyzer/checkers.rst
  clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
  clang/lib/StaticAnalyzer/Checkers/CMakeLists.txt
  clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
  clang/test/Analysis/ArrayDelete.cpp
  clang/www/analyzer/alpha_checks.html

Index: clang/www/analyzer/alpha_checks.html
===
--- clang/www/analyzer/alpha_checks.html
+++ clang/www/analyzer/alpha_checks.html
@@ -330,6 +330,27 @@
 
 
 
+
+alpha.cplusplus.ArrayDelete
+(C++)
+Reports destructions of arrays of polymorphic objects that are destructed as
+their base class
+
+
+
+Base *create() {
+  Base *x = new Derived[10]; // note: conversion from derived to base
+ //   happened here
+  return x;
+}
+
+void sink(Base *x) {
+  delete[] x; // warn: Deleting an array of polymorphic objects is undefined
+}
+
+
+
+
 
 alpha.cplusplus.DeleteWithNonVirtualDtor
 (C++)
Index: clang/test/Analysis/ArrayDelete.cpp
===
--- /dev/null
+++ clang/test/Analysis/ArrayDelete.cpp
@@ -0,0 +1,72 @@
+// RUN: %clang_cc1 -analyze -analyzer-checker=alpha.cplusplus.ArrayDelete -std=c++11 -verify -analyzer-output=text %s
+
+struct Base {
+virtual ~Base() = default;
+};
+
+struct Derived : public Base {};
+
+struct DoubleDerived : public Derived {};
+
+Derived *get();
+
+Base *create() {
+Base *b = new Derived[3]; // expected-note{{Conversion from derived to base happened here}}
+return b;
+}
+
+void sink(Base *b) {
+delete[] b; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+}
+
+void sink_cast(Base *b) {
+delete[] reinterpret_cast(b); // no-warning
+}
+
+void sink_derived(Derived *d) {
+delete[] d; // no-warning
+}
+
+void same_function() {
+Base *sd = new Derived[10]; // expected-note{{Conversion from derived to base happened here}}
+delete[] sd; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *dd = new DoubleDerived[10]; // expected-note{{Conversion from derived to base happened here}}
+delete[] dd; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+}
+
+void different_function() {
+Base *assigned = get(); // expected-note{{Conversion from derived to base happened here}}
+delete[] assigned; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *indirect;
+indirect = get(); // expected-note{{Conversion from derived to base happened here}}
+delete[] indirect; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *created = create(); // expected-note{{Calling 'create'}}
+// expected-note@-1{{Returning from 'create'}}
+delete[] created; // expected-warning{{Deleting an array of polymorphic objects is undefined}}
+// expected-note@-1{{Deleting an array of polymorphic objects is undefined}}
+
+Base *sb = new Derived[10]; // expected-note{{Conversion from derived to base happened here}}
+sink(sb); // expected-note{{Calling 'sink'}}
+}
+
+void safe_function() {
+Derived *d = new Derived[10];
+delete[] d; // no-warning
+
+Base *b = new Derived[10];
+delete[] reinterpret_cast(b); // no-warning
+
+Base *sb = new Derived[10];
+sink_cast(sb); // no-warning
+
+Derived *sd = new Derived[10];
+sink_derived(sd); // no-warning
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeleteWithNonVirtualDtorChecker.cpp
+++ /dev/null
@@ -1,155 +0,0 @@
-//===-- DeleteWithNonVirtualDtorChecker.cpp ---*- 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
-//
-//===

[clang] 8ad37a8 - [clang][Interp][NFC] Return array element type in Pointer::getType()

2023-09-05 Thread Timm BĂ€der via cfe-commits 

Author: Timm BĂ€der
Date: 2023-09-05T10:28:47+02:00
New Revision: 8ad37a894b0b54fe71afa5ea3c003eea6ba76676

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

LOG: [clang][Interp][NFC] Return array element type in Pointer::getType()

This is irrelevant for the current tests, but makes sense and later
changes easier.

Added: 


Modified: 
clang/lib/AST/Interp/Pointer.h

Removed: 




diff  --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h
index 7012009c675af2..48aa4df1430fc1 100644
--- a/clang/lib/AST/Interp/Pointer.h
+++ b/clang/lib/AST/Interp/Pointer.h
@@ -212,7 +212,11 @@ class Pointer {
   }
 
   /// Returns the type of the innermost field.
-  QualType getType() const { return getFieldDesc()->getType(); }
+  QualType getType() const {
+if (inPrimitiveArray() && Offset != Base)
+  return 
getFieldDesc()->getType()->getAsArrayTypeUnsafe()->getElementType();
+return getFieldDesc()->getType();
+  }
 
   Pointer getDeclPtr() const { return Pointer(Pointee); }
 



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

[clang] c6ce32d - [clang][Interp][NFC] Fix a comment

2023-09-05 Thread Timm BĂ€der via cfe-commits 

Author: Timm BĂ€der
Date: 2023-09-05T10:28:47+02:00
New Revision: c6ce32d9c348f69b19d8b3bfb4e0e3c3ea3bf679

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

LOG: [clang][Interp][NFC] Fix a comment

Added: 


Modified: 
clang/lib/AST/Interp/Interp.h

Removed: 




diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 180bd5add2e262..a78b3921b0c7dd 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -166,7 +166,7 @@ bool CheckDivRem(InterpState &S, CodePtr OpPC, const T 
&LHS, const T &RHS) {
   return true;
 }
 
-/// Checks if the result is a floating-point operation is valid
+/// Checks if the result of a floating-point operation is valid
 /// in the current context.
 bool CheckFloatResult(InterpState &S, CodePtr OpPC, APFloat::opStatus Status);
 



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

[PATCH] D158156: [analyzer] Add C++ array delete checker

2023-09-05 Thread Discookie via Phabricator via cfe-commits 
Discookie marked 8 inline comments as done.
Discookie added a comment.

Fixed the formatting issues as well.




Comment at: clang/docs/analyzer/checkers.rst:1787-1804
+.. _alpha-cplusplus-ArrayDelete:
+
+alpha.cplusplus.ArrayDelete (C++)
+""
+Reports destructions of arrays of polymorphic objects that are destructed as 
their base class.
+
+.. code-block:: cpp

steakhal wrote:
> I think you should probably mention `EXP51-CPP CERT rule` somehow here.
Added a link to the CERT rules, similar to how others do it, thanks!



Comment at: clang/docs/analyzer/checkers.rst:1793-1803
+.. code-block:: cpp
+
+ Base *create() {
+   Base *x = new Derived[10]; // note: conversion from derived to base
+  //   happened here
+   return x;
+ }

steakhal wrote:
> Make sure in the example the `create` is related (e.g. called/used).
> Also, refrain from using `sink` in the docs. It's usually used in the context 
> of taint analysis.
Changed the example - should I change the DeleteWithNonVirtualDtor example as 
well? That has the same issues as you said here.


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

https://reviews.llvm.org/D158156

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

[PATCH] D159414: Haiku: Fixes for header / library paths

2023-09-05 Thread Trung Nguyen via Phabricator via cfe-commits 
trungnt2910 added inline comments.



Comment at: clang/lib/Driver/ToolChains/Haiku.cpp:85-86
+   "/boot/system/develop/headers/os/net"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,
+   "/boot/system/develop/headers/os/opengl"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,

brad wrote:
> trungnt2910 wrote:
> > Not included by GCC, and not found in my Haiku installation.
> > 
> > I don't know what this is for though, so probably just keep it until 
> > someone else says otherwise.
> > Not included by GCC, and not found in my Haiku installation.
> > 
> > I don't know what this is for though, so probably just keep it until 
> > someone else says otherwise.
> 
> This is referring to the os/opengl directory?
Yes, I was trying to select lines 85-86.



Comment at: clang/lib/Driver/ToolChains/Haiku.cpp:105-106
+   "/boot/system/develop/headers/os/add-ons/tracker"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,
+   "/boot/system/develop/headers/os/be_apps/Deskbar"));
+  addSystemInclude(DriverArgs, CC1Args, concat(D.SysRoot,

brad wrote:
> trungnt2910 wrote:
> > This seems to have been removed from Haiku, see: 
> > https://github.com/haiku/haiku/commit/a3e794ae459fec76826407f8ba8c94cd3535f128
> > 
> > The latest Haiku GCC toolchain does not include it.
> > This seems to have been removed from Haiku, see: 
> > https://github.com/haiku/haiku/commit/a3e794ae459fec76826407f8ba8c94cd3535f128
> > 
> > The latest Haiku GCC toolchain does not include it.
> 
> https://github.com/haikuports/haikuports/blob/master/sys-devel/gcc/patches/gcc-13.2.0_2023_08_10.patchset
> 
> I still see the path listed in the GCC configs.
> 
> But I see in the Haiku I have installed in my VM that the path indeed is gone.
I was comparing to the output of `gcc -v`, now I see what's happening:

```
ignoring nonexistent directory "/boot/system/non-packaged/develop/headers"
ignoring nonexistent directory "/boot/system/develop/headers/os/opengl"
ignoring nonexistent directory "/boot/system/develop/headers/os/be_apps/Deskbar"
ignoring nonexistent directory "/boot/system/develop/headers/3rdparty"
```

If GCC is doing that, I think we should keep things as they are.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159414

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

[clang] 12a7897 - [clang][Interp] BaseToDerived casts

2023-09-05 Thread Timm BĂ€der via cfe-commits 

Author: Timm BĂ€der
Date: 2023-09-05T10:53:54+02:00
New Revision: 12a789710e2bafe9bb49adea5e634784b086eb6a

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

LOG: [clang][Interp] BaseToDerived casts

We can implement these similarly to DerivedToBase casts. We just have to
walk the class hierarchy, sum the base offsets and subtract it from the
current base offset of the pointer.

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

Added: 


Modified: 
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/Interp.h
clang/lib/AST/Interp/Opcodes.td
clang/lib/AST/Interp/Pointer.h
clang/test/AST/Interp/records.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index b2177c29d30d5af..4708799773603de 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -92,8 +92,20 @@ bool ByteCodeExprGen::VisitCastExpr(const CastExpr 
*CE) {
 if (!this->visit(SubExpr))
   return false;
 
-return this->emitDerivedToBaseCasts(getRecordTy(SubExpr->getType()),
-getRecordTy(CE->getType()), CE);
+unsigned DerivedOffset = collectBaseOffset(getRecordTy(CE->getType()),
+   
getRecordTy(SubExpr->getType()));
+
+return this->emitGetPtrBasePop(DerivedOffset, CE);
+  }
+
+  case CK_BaseToDerived: {
+if (!this->visit(SubExpr))
+  return false;
+
+unsigned DerivedOffset = collectBaseOffset(getRecordTy(SubExpr->getType()),
+   getRecordTy(CE->getType()));
+
+return this->emitGetPtrDerivedPop(DerivedOffset, CE);
   }
 
   case CK_FloatingCast: {
@@ -2262,13 +2274,15 @@ void ByteCodeExprGen::emitCleanup() {
 }
 
 template 
-bool ByteCodeExprGen::emitDerivedToBaseCasts(
-const RecordType *DerivedType, const RecordType *BaseType, const Expr *E) {
-  // Pointer of derived type is already on the stack.
+unsigned
+ByteCodeExprGen::collectBaseOffset(const RecordType *BaseType,
+const RecordType *DerivedType) {
   const auto *FinalDecl = cast(BaseType->getDecl());
   const RecordDecl *CurDecl = DerivedType->getDecl();
   const Record *CurRecord = getRecord(CurDecl);
   assert(CurDecl && FinalDecl);
+
+  unsigned OffsetSum = 0;
   for (;;) {
 assert(CurRecord->getNumBases() > 0);
 // One level up
@@ -2276,21 +2290,18 @@ bool ByteCodeExprGen::emitDerivedToBaseCasts(
   const auto *BaseDecl = cast(B.Decl);
 
   if (BaseDecl == FinalDecl || BaseDecl->isDerivedFrom(FinalDecl)) {
-// This decl will lead us to the final decl, so emit a base cast.
-if (!this->emitGetPtrBasePop(B.Offset, E))
-  return false;
-
+OffsetSum += B.Offset;
 CurRecord = B.R;
 CurDecl = BaseDecl;
 break;
   }
 }
 if (CurDecl == FinalDecl)
-  return true;
+  break;
   }
 
-  llvm_unreachable("Couldn't find the base class?");
-  return false;
+  assert(OffsetSum > 0);
+  return OffsetSum;
 }
 
 /// When calling this, we have a pointer of the local-to-destroy

diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.h 
b/clang/lib/AST/Interp/ByteCodeExprGen.h
index 3a64a4f6fec0726..cd924e911759e01 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -267,6 +267,8 @@ class ByteCodeExprGen : public 
ConstStmtVisitor, bool>,
   bool emitRecordDestruction(const Descriptor *Desc);
   bool emitDerivedToBaseCasts(const RecordType *DerivedType,
   const RecordType *BaseType, const Expr *E);
+  unsigned collectBaseOffset(const RecordType *BaseType,
+ const RecordType *DerivedType);
 
 protected:
   /// Variable to storage mapping.

diff  --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 2b7f3bf35aa5982..23c54b3898e102b 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -213,6 +213,16 @@ bool CheckRange(InterpState &S, CodePtr OpPC, const 
Pointer &Ptr,
   return false;
 }
 
+bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+  CheckSubobjectKind CSK) {
+  if (!Ptr.isOnePastEnd())
+return true;
+
+  const SourceInfo &Loc = S.Current->getSource(OpPC);
+  S.FFDiag(Loc, diag::note_constexpr_past_end_subobject) << CSK;
+  return false;
+}
+
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
   assert(Ptr.isLive() && "Pointer is not live");
   if (!Ptr.isConst())

diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h

[PATCH] D149133: [clang][Interp] BaseToDerived casts

2023-09-05 Thread Timm BĂ€der 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 rG12a789710e2b: [clang][Interp] BaseToDerived casts (authored 
by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D149133?vs=516701&id=555828#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149133

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

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -625,6 +625,58 @@
// ref-note {{in call to 'testS()'}}
 }
 
+namespace BaseToDerived {
+namespace A {
+  struct A {};
+  struct B : A { int n; };
+  struct C : B {};
+  C c = {};
+  constexpr C *pb = (C*)((A*)&c + 1); // expected-error {{must be initialized by a constant expression}} \
+  // expected-note {{cannot access derived class of pointer past the end of object}} \
+  // ref-error {{must be initialized by a constant expression}} \
+  // ref-note {{cannot access derived class of pointer past the end of object}}
+}
+namespace B {
+  struct A {};
+  struct Z {};
+  struct B : Z, A {
+int n;
+   constexpr B() : n(10) {}
+  };
+  struct C : B {
+   constexpr C() : B() {}
+  };
+
+  constexpr C c = {};
+  constexpr const A *pa = &c;
+  constexpr const C *cp = (C*)pa;
+  constexpr const B *cb = (B*)cp;
+
+  static_assert(cb->n == 10);
+  static_assert(cp->n == 10);
+}
+
+namespace C {
+  struct Base { int *a; };
+  struct Base2 : Base { int f[12]; };
+
+  struct Middle1 { int b[3]; };
+  struct Middle2 : Base2 { char c; };
+  struct Middle3 : Middle2 { char g[3]; };
+  struct Middle4 { int f[3]; };
+  struct Middle5 : Middle4, Middle3 { char g2[3]; };
+
+  struct NotQuiteDerived : Middle1, Middle5 { bool d; };
+  struct Derived : NotQuiteDerived { int e; };
+
+  constexpr NotQuiteDerived NQD1 = {};
+
+  constexpr Middle5 *M4 = (Middle5*)((Base2*)&NQD1);
+  static_assert(M4->a == nullptr);
+  static_assert(M4->g2[0] == 0);
+}
+}
+
 
 namespace VirtualDtors {
   class A {
Index: clang/lib/AST/Interp/Pointer.h
===
--- clang/lib/AST/Interp/Pointer.h
+++ clang/lib/AST/Interp/Pointer.h
@@ -109,6 +109,14 @@
 return Pointer(Pointee, Field, Field);
   }
 
+  /// Subtract the given offset from the current Base and Offset
+  /// of the pointer.
+  Pointer atFieldSub(unsigned Off) const {
+assert(Offset >= Off);
+unsigned O = Offset - Off;
+return Pointer(Pointee, O, O);
+  }
+
   /// Restricts the scope of an array element pointer.
   Pointer narrow() const {
 // Null pointers cannot be narrowed.
Index: clang/lib/AST/Interp/Opcodes.td
===
--- clang/lib/AST/Interp/Opcodes.td
+++ clang/lib/AST/Interp/Opcodes.td
@@ -293,6 +293,10 @@
   let Args = [ArgUint32];
 }
 
+def GetPtrDerivedPop : Opcode {
+  let Args = [ArgUint32];
+}
+
 // [Pointer] -> [Pointer]
 def GetPtrVirtBase : Opcode {
   // RecordDecl of base class.
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -67,6 +67,10 @@
 bool CheckRange(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 CheckSubobjectKind CSK);
 
+/// Checks if accessing a base or derived record of the given pointer is valid.
+bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+  CheckSubobjectKind CSK);
+
 /// Checks if a pointer points to const storage.
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
 
@@ -1157,10 +1161,22 @@
   return true;
 }
 
+inline bool GetPtrDerivedPop(InterpState &S, CodePtr OpPC, uint32_t Off) {
+  const Pointer &Ptr = S.Stk.pop();
+  if (!CheckNull(S, OpPC, Ptr, CSK_Derived))
+return false;
+  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Derived))
+return false;
+  S.Stk.push(Ptr.atFieldSub(Off));
+  return true;
+}
+
 inline bool GetPtrBase(InterpState &S, CodePtr OpPC, uint32_t Off) {
   const Pointer &Ptr = S.Stk.peek();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
+  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+return false;
   S.Stk.push(Ptr.atField(Off));
   return true;
 }
@@ -1169,6 +1185,8 @@
   const Pointer &Ptr = S.Stk.pop();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
+  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+return false;
   S.

[PATCH] D155627: [clang][Interp] Handle SourceLocExprs

2023-09-05 Thread Timm BĂ€der via Phabricator via cfe-commits 
tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D155627

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

[clang] [Clang] Fix missing diagnostic for non-standard layout type in `offsetof` (PR #65246)

2023-09-05 Thread via cfe-commits 

kasuga-fj wrote:

The CI failed because `libcxx` uses `offsetof`.
https://github.com/kasuga-fj/llvm-project/blob/b9c767a6a128f57858273eb43d4383e83d0d086b/libcxx/include/__type_traits/datasizeof.h#L50
I investigated but couldn't figure out where to fix it (Should I change the 
compile time options?). Could someone please help me? It may be unrelated, but 
I am also concerned that "libc++ test suite in C++03" failed while "libc++ test 
suite in C++26" succeeded.

https://github.com/llvm/llvm-project/pull/65246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a13f036 - [clang][Interp] Check one-past-the-end pointers in GetPtrField

2023-09-05 Thread Timm BĂ€der via cfe-commits 

Author: Timm BĂ€der
Date: 2023-09-05T11:00:40+02:00
New Revision: a13f036949b05e5f9d1ac1e33fed465c939aca62

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

LOG: [clang][Interp] Check one-past-the-end pointers in GetPtrField

Rename CheckBaseDerived to something more general and call it in
GetPtrField() as well, so we don't crash later in Pointer::toAPValue().

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

Added: 


Modified: 
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/Interp.h
clang/test/AST/Interp/records.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 23c54b3898e102..719a96daaebdd7 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -213,8 +213,8 @@ bool CheckRange(InterpState &S, CodePtr OpPC, const Pointer 
&Ptr,
   return false;
 }
 
-bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
-  CheckSubobjectKind CSK) {
+bool CheckSubobject(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+CheckSubobjectKind CSK) {
   if (!Ptr.isOnePastEnd())
 return true;
 

diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 006abc0f0e94ac..b87a5c1cbd0e48 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -67,9 +67,9 @@ bool CheckRange(InterpState &S, CodePtr OpPC, const Pointer 
&Ptr,
 bool CheckRange(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 CheckSubobjectKind CSK);
 
-/// Checks if accessing a base or derived record of the given pointer is valid.
-bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
-  CheckSubobjectKind CSK);
+/// Checks if Ptr is a one-past-the-end pointer.
+bool CheckSubobject(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+CheckSubobjectKind CSK);
 
 /// Checks if a pointer points to const storage.
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
@@ -1121,6 +1121,9 @@ inline bool GetPtrField(InterpState &S, CodePtr OpPC, 
uint32_t Off) {
 return false;
   if (!CheckRange(S, OpPC, Ptr, CSK_Field))
 return false;
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Field))
+return false;
+
   S.Stk.push(Ptr.atField(Off));
   return true;
 }
@@ -1165,7 +1168,7 @@ inline bool GetPtrDerivedPop(InterpState &S, CodePtr 
OpPC, uint32_t Off) {
   const Pointer &Ptr = S.Stk.pop();
   if (!CheckNull(S, OpPC, Ptr, CSK_Derived))
 return false;
-  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Derived))
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Derived))
 return false;
   S.Stk.push(Ptr.atFieldSub(Off));
   return true;
@@ -1175,7 +1178,7 @@ inline bool GetPtrBase(InterpState &S, CodePtr OpPC, 
uint32_t Off) {
   const Pointer &Ptr = S.Stk.peek();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
-  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
 return false;
   S.Stk.push(Ptr.atField(Off));
   return true;
@@ -1185,7 +1188,7 @@ inline bool GetPtrBasePop(InterpState &S, CodePtr OpPC, 
uint32_t Off) {
   const Pointer &Ptr = S.Stk.pop();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
-  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
 return false;
   S.Stk.push(Ptr.atField(Off));
   return true;

diff  --git a/clang/test/AST/Interp/records.cpp 
b/clang/test/AST/Interp/records.cpp
index b559f1f8b95b0a..2b79ec8be0311d 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -531,6 +531,29 @@ namespace DeclRefs {
   //static_assert(b.a.f == 100, "");
 }
 
+namespace PointerArith {
+  struct A {};
+  struct B : A { int n; };
+
+  B b = {};
+  constexpr A *a1 = &b;
+  constexpr B *b1 = &b + 1;
+  constexpr B *b2 = &b + 0;
+
+#if 0
+  constexpr A *a2 = &b + 1; // expected-error {{must be initialized by a 
constant expression}} \
+// expected-note {{cannot access base class of 
pointer past the end of object}} \
+// ref-error {{must be initialized by a constant 
expression}} \
+// ref-note {{cannot access base class of pointer 
past the end of object}}
+
+#endif
+  constexpr const int *pn = &(&b + 1)->n; // expected-error {{must be 
initialized by a constant expression}} \
+  // expected-note {{cannot access 
field of pointer past the end of object}} \
+  // ref-error {{must be initialized 
by a constant expression}} \
+  // ref-note {{cannot access field of 
pointer past the end of o

[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField

2023-09-05 Thread Timm BĂ€der 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 rGa13f036949b0: [clang][Interp] Check one-past-the-end 
pointers in GetPtrField (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D149149?vs=516763&id=555834#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149149

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

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -531,6 +531,29 @@
   //static_assert(b.a.f == 100, "");
 }
 
+namespace PointerArith {
+  struct A {};
+  struct B : A { int n; };
+
+  B b = {};
+  constexpr A *a1 = &b;
+  constexpr B *b1 = &b + 1;
+  constexpr B *b2 = &b + 0;
+
+#if 0
+  constexpr A *a2 = &b + 1; // expected-error {{must be initialized by a constant expression}} \
+// expected-note {{cannot access base class of pointer past the end of object}} \
+// ref-error {{must be initialized by a constant expression}} \
+// ref-note {{cannot access base class of pointer past the end of object}}
+
+#endif
+  constexpr const int *pn = &(&b + 1)->n; // expected-error {{must be initialized by a constant expression}} \
+  // expected-note {{cannot access field of pointer past the end of object}} \
+  // ref-error {{must be initialized by a constant expression}} \
+  // ref-note {{cannot access field of pointer past the end of object}}
+
+}
+
 #if __cplusplus >= 202002L
 namespace VirtualCalls {
 namespace Obvious {
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -67,9 +67,9 @@
 bool CheckRange(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 CheckSubobjectKind CSK);
 
-/// Checks if accessing a base or derived record of the given pointer is valid.
-bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
-  CheckSubobjectKind CSK);
+/// Checks if Ptr is a one-past-the-end pointer.
+bool CheckSubobject(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+CheckSubobjectKind CSK);
 
 /// Checks if a pointer points to const storage.
 bool CheckConst(InterpState &S, CodePtr OpPC, const Pointer &Ptr);
@@ -1121,6 +1121,9 @@
 return false;
   if (!CheckRange(S, OpPC, Ptr, CSK_Field))
 return false;
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Field))
+return false;
+
   S.Stk.push(Ptr.atField(Off));
   return true;
 }
@@ -1165,7 +1168,7 @@
   const Pointer &Ptr = S.Stk.pop();
   if (!CheckNull(S, OpPC, Ptr, CSK_Derived))
 return false;
-  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Derived))
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Derived))
 return false;
   S.Stk.push(Ptr.atFieldSub(Off));
   return true;
@@ -1175,7 +1178,7 @@
   const Pointer &Ptr = S.Stk.peek();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
-  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
 return false;
   S.Stk.push(Ptr.atField(Off));
   return true;
@@ -1185,7 +1188,7 @@
   const Pointer &Ptr = S.Stk.pop();
   if (!CheckNull(S, OpPC, Ptr, CSK_Base))
 return false;
-  if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+  if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
 return false;
   S.Stk.push(Ptr.atField(Off));
   return true;
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -213,8 +213,8 @@
   return false;
 }
 
-bool CheckBaseDerived(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
-  CheckSubobjectKind CSK) {
+bool CheckSubobject(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
+CheckSubobjectKind CSK) {
   if (!Ptr.isOnePastEnd())
 return true;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D154581: [clang][Interp] Track existing InitMaps in InterpState

2023-09-05 Thread Timm BĂ€der via Phabricator via cfe-commits 
tbaeder added a comment.

Ping


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

https://reviews.llvm.org/D154581

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

[clang] [Clang] Fix missing diagnostic for non-standard layout type in `offsetof` (PR #65246)

2023-09-05 Thread Tobias Hieta via cfe-commits 

tru wrote:

Ping @llvm/pr-subscribers-libcxx 

https://github.com/llvm/llvm-project/pull/65246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155568: [clang][Interp] Make sure we push integers of the correct size

2023-09-05 Thread Timm BĂ€der via Phabricator via cfe-commits 
tbaeder closed this revision.
tbaeder added a comment.

This landed as 673ef8ceaece6c9a7194474ef7d97b3b240c0dc5 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155568

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

[clang] 6d79f98 - [clang][Interp] Implement zero-init of record types

2023-09-05 Thread Timm BĂ€der via cfe-commits 

Author: Timm BĂ€der
Date: 2023-09-05T11:20:35+02:00
New Revision: 6d79f985b53ef57f670ab7e4c7f59c953b49b4d6

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

LOG: [clang][Interp] Implement zero-init of record types

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

Added: 


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

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 4708799773603d..131525c98ca59c 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1302,7 +1302,15 @@ bool ByteCodeExprGen::VisitCXXConstructExpr(
   assert(!classify(T));
 
   if (T->isRecordType()) {
-const Function *Func = getFunction(E->getConstructor());
+const CXXConstructorDecl *Ctor = E->getConstructor();
+
+// Trivial zero initialization.
+if (E->requiresZeroInitialization() && Ctor->isTrivial()) {
+  const Record *R = getRecord(E->getType());
+  return this->visitZeroRecordInitializer(R, E);
+}
+
+const Function *Func = getFunction(Ctor);
 
 if (!Func)
   return false;
@@ -1479,6 +1487,77 @@ bool 
ByteCodeExprGen::visitZeroInitializer(QualType QT,
   llvm_unreachable("unknown primitive type");
 }
 
+template 
+bool ByteCodeExprGen::visitZeroRecordInitializer(const Record *R,
+  const Expr *E) {
+  assert(E);
+  assert(R);
+  // Fields
+  for (const Record::Field &Field : R->fields()) {
+const Descriptor *D = Field.Desc;
+if (D->isPrimitive()) {
+  QualType QT = D->getType();
+  PrimType T = classifyPrim(D->getType());
+  if (!this->visitZeroInitializer(QT, E))
+return false;
+  if (!this->emitInitField(T, Field.Offset, E))
+return false;
+  continue;
+}
+
+// TODO: Add GetPtrFieldPop and get rid of this dup.
+if (!this->emitDupPtr(E))
+  return false;
+if (!this->emitGetPtrField(Field.Offset, E))
+  return false;
+
+if (D->isPrimitiveArray()) {
+  QualType ET = D->getElemQualType();
+  PrimType T = classifyPrim(ET);
+  for (uint32_t I = 0, N = D->getNumElems(); I != N; ++I) {
+if (!this->visitZeroInitializer(ET, E))
+  return false;
+if (!this->emitInitElem(T, I, E))
+  return false;
+  }
+} else if (D->isCompositeArray()) {
+  const Record *ElemRecord = D->ElemDesc->ElemRecord;
+  assert(D->ElemDesc->ElemRecord);
+  for (uint32_t I = 0, N = D->getNumElems(); I != N; ++I) {
+if (!this->emitConstUint32(I, E))
+  return false;
+if (!this->emitArrayElemPtr(PT_Uint32, E))
+  return false;
+if (!this->visitZeroRecordInitializer(ElemRecord, E))
+  return false;
+if (!this->emitPopPtr(E))
+  return false;
+  }
+} else if (D->isRecord()) {
+  if (!this->visitZeroRecordInitializer(D->ElemRecord, E))
+return false;
+} else {
+  assert(false);
+}
+
+if (!this->emitPopPtr(E))
+  return false;
+  }
+
+  for (const Record::Base &B : R->bases()) {
+if (!this->emitGetPtrBase(B.Offset, E))
+  return false;
+if (!this->visitZeroRecordInitializer(B.R, E))
+  return false;
+if (!this->emitPopPtr(E))
+  return false;
+  }
+
+  // FIXME: Virtual bases.
+
+  return true;
+}
+
 template 
 bool ByteCodeExprGen::dereference(
 const Expr *LV, DerefKind AK, llvm::function_ref Direct,

diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.h 
b/clang/lib/AST/Interp/ByteCodeExprGen.h
index cd924e911759e0..dda954320cd24e 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.h
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -213,6 +213,7 @@ class ByteCodeExprGen : public 
ConstStmtVisitor, bool>,
 
   /// Emits a zero initializer.
   bool visitZeroInitializer(QualType QT, const Expr *E);
+  bool visitZeroRecordInitializer(const Record *R, const Expr *E);
 
   enum class DerefKind {
 /// Value is read and pushed to stack.

diff  --git a/clang/lib/AST/Interp/Descriptor.cpp 
b/clang/lib/AST/Interp/Descriptor.cpp
index b4c26ac88b5c6b..db49a569eff33e 100644
--- a/clang/lib/AST/Interp/Descriptor.cpp
+++ b/clang/lib/AST/Interp/Descriptor.cpp
@@ -285,6 +285,12 @@ QualType Descriptor::getType() const {
   llvm_unreachable("Invalid descriptor type");
 }
 
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  const auto *AT = cast(getType());
+  return AT->getElementType();
+}
+
 SourceLocation Descriptor::getLocation() const {
   if (auto *D = Source.dyn_cast())
 return D->getLocat

[PATCH] D154189: [clang][Interp] Implement zero-init of record types

2023-09-05 Thread Timm BĂ€der 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 rG6d79f985b53e: [clang][Interp] Implement zero-init of record 
types (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D154189?vs=545367&id=555836#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D154189

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

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -911,3 +911,115 @@
   }
   static_assert(foo(F()) == 0, "");
 }
+
+namespace ZeroInit {
+  struct F {
+int a;
+  };
+
+  namespace Simple {
+struct A {
+  char a;
+  bool b;
+  int c[4];
+  float d;
+};
+constexpr int foo(A x) {
+  return x.a + static_cast(x.b) + x.c[0] + x.c[3] + static_cast(x.d);
+}
+static_assert(foo(A()) == 0, "");
+  }
+
+  namespace Inheritance {
+struct F2 : F {
+  float f;
+};
+
+constexpr int foo(F2 f) {
+  return (int)f.f + f.a;
+}
+static_assert(foo(F2()) == 0, "");
+  }
+
+  namespace BitFields {
+struct F {
+  unsigned a : 6;
+};
+constexpr int foo(F f) {
+  return f.a;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+  namespace Nested {
+struct F2 {
+  float f;
+  char c;
+};
+
+struct F {
+  F2 f2;
+  int i;
+};
+
+constexpr int foo(F f) {
+  return f.i + f.f2.f + f.f2.c;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+  namespace CompositeArrays {
+struct F2 {
+  float f;
+  char c;
+};
+
+struct F {
+  F2 f2[2];
+  int i;
+};
+
+constexpr int foo(F f) {
+  return f.i + f.f2[0].f + f.f2[0].c + f.f2[1].f + f.f2[1].c;
+}
+static_assert(foo(F()) == 0, "");
+  }
+
+  /// FIXME: This needs support for unions on the new interpreter.
+  /// We diagnose an uninitialized object in c++14.
+#if __cplusplus > 201402L
+  namespace Unions {
+struct F {
+  union {
+int a;
+char c[4];
+float f;
+  } U;
+  int i;
+};
+
+constexpr int foo(F f) {
+  return f.i + f.U.f; // ref-note {{read of member 'f' of union with active member 'a'}}
+}
+static_assert(foo(F()) == 0, ""); // ref-error {{not an integral constant expression}} \
+  // ref-note {{in call to}}
+  }
+#endif
+
+#if __cplusplus >= 202002L
+  namespace Failure {
+struct S {
+  int a;
+  F f{12};
+};
+constexpr int foo(S x) {
+  return x.a; // expected-note {{read of uninitialized object}} \
+  // ref-note {{read of uninitialized object}}
+}
+static_assert(foo(S()) == 0, ""); // expected-error {{not an integral constant expression}} \
+  // expected-note {{in call to}} \
+  // ref-error {{not an integral constant expression}} \
+  // ref-note {{in call to}}
+  };
+#endif
+}
Index: clang/lib/AST/Interp/Descriptor.h
===
--- clang/lib/AST/Interp/Descriptor.h
+++ clang/lib/AST/Interp/Descriptor.h
@@ -138,6 +138,7 @@
  bool IsTemporary, bool IsMutable);
 
   QualType getType() const;
+  QualType getElemQualType() const;
   SourceLocation getLocation() const;
 
   const Decl *asDecl() const { return Source.dyn_cast(); }
Index: clang/lib/AST/Interp/Descriptor.cpp
===
--- clang/lib/AST/Interp/Descriptor.cpp
+++ clang/lib/AST/Interp/Descriptor.cpp
@@ -285,6 +285,12 @@
   llvm_unreachable("Invalid descriptor type");
 }
 
+QualType Descriptor::getElemQualType() const {
+  assert(isArray());
+  const auto *AT = cast(getType());
+  return AT->getElementType();
+}
+
 SourceLocation Descriptor::getLocation() const {
   if (auto *D = Source.dyn_cast())
 return D->getLocation();
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -213,6 +213,7 @@
 
   /// Emits a zero initializer.
   bool visitZeroInitializer(QualType QT, const Expr *E);
+  bool visitZeroRecordInitializer(const Record *R, const Expr *E);
 
   enum class DerefKind {
 /// Value is read and pushed to stack.
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1302,7 +

[PATCH] D158156: [analyzer] Add C++ array delete checker

2023-09-05 Thread BalĂĄzs Benics via Phabricator via cfe-commits 
steakhal added a comment.

Add a test where we have multiple imp derived to base casts but only one leads 
to a bug. Do it for both ways, once that the first is the offending, and once 
the second.




Comment at: clang/docs/analyzer/checkers.rst:1793-1803
+.. code-block:: cpp
+
+ Base *create() {
+   Base *x = new Derived[10]; // note: conversion from derived to base
+  //   happened here
+   return x;
+ }

Discookie wrote:
> steakhal wrote:
> > Make sure in the example the `create` is related (e.g. called/used).
> > Also, refrain from using `sink` in the docs. It's usually used in the 
> > context of taint analysis.
> Changed the example - should I change the DeleteWithNonVirtualDtor example as 
> well? That has the same issues as you said here.
I have no opinion. You could.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:126
+
+  if (!DerivedClass->isDerivedFrom(BaseClass))
+return;

Is this transitive?

BTW inheritance can only be expressed if the class is a definition, right?
Thus passing this should imply has definition.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:143
+  R->markInteresting(BaseClassRegion);
+  R->addVisitor(std::make_unique());
+  C.emitReport(std::move(R));

I think addVisitor already takes a template param, and it will call make unique 
for you.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:192
+
+  const Stmt *S = N->getStmtForDiagnostics();
+  if (!S)

Aren't you actually interested in N->getLocation().getAs().getStmt()?

The diag stmt can be fuzzy, but the PP is exact.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:196
+
+  const auto *CastE = dyn_cast(S);
+  if (!CastE)

If you dyncast to ImplicitCastExpr, couldn't you have done it here?



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:202
+  // Explicit casts can have different CastKinds.
+  if (const auto *ImplCastE = dyn_cast(CastE)) {
+if (ImplCastE->getCastKind() != CK_DerivedToBase)

How do you know that that this castexpr corresponds to the region for which you 
report the bug? To mez this might be some unrelated castexpr.
I was expecting the offending memregion being passed to the visitor, that it 
can check against.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:217
+  // Stop traversal on this path.
+  Satisfied = true;
+

There are so many early returns going on. I feel, we could miss the program 
point where it should have been marked satisfied. After this point, the visitor 
will never or should never emit a note.



Comment at: clang/lib/StaticAnalyzer/Checkers/CXXDeleteChecker.cpp:224
+ N->getLocationContext());
+  return std::make_shared(Pos, OS.str(), true);
+}

Use named argument passing.


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

https://reviews.llvm.org/D158156

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

[PATCH] D159115: [clang-repl] Adapt to the recent dylib-related changes in ORC.

2023-09-05 Thread MichaƂ Górny via Phabricator via cfe-commits 
mgorny added a comment.

This change is causing test regressions on 32-bit architectures (I've 
reproduced with `-m32` on amd64 and in 32-bit nspawn container on arm64):

  FAIL: Clang-Unit :: Interpreter/./ClangReplInterpreterTests/1/13 (1 of 1584)
   TEST 'Clang-Unit :: 
Interpreter/./ClangReplInterpreterTests/1/13' FAILED 
  Script(shard):
  --
  
GTEST_OUTPUT=json:/home/mgorny/llvm-project/build/tools/clang/unittests/Interpreter/./ClangReplInterpreterTests-Clang-Unit-1816215-1-13.json
 GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=13 GTEST_SHARD_INDEX=1 
/home/mgorny/llvm-project/build/tools/clang/unittests/Interpreter/./ClangReplInterpreterTests
  --
  
  Script:
  --
  
/home/mgorny/llvm-project/build/tools/clang/unittests/Interpreter/./ClangReplInterpreterTests
 --gtest_filter=IncrementalProcessing.FindMangledNameSymbol
  --
  
/home/mgorny/llvm-project/clang/unittests/Interpreter/InterpreterTest.cpp:246: 
Failure
  Expected equality of these values:
(unsigned long long)&printf
  Which is: 18446744073567794308
Addr->getValue()
  Which is: 4153209988
  
  /home/mgorny/llvm-project/clang/unittests/Interpreter/InterpreterTest.cpp:246
  Expected equality of these values:
(unsigned long long)&printf
  Which is: 18446744073567794308
Addr->getValue()
  Which is: 4153209988
  
  
  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159115

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

[PATCH] D159284: [clang][dataflow] Fix Record initialization with InitListExpr and inheritances

2023-09-05 Thread Kinuko Yasuda via Phabricator via cfe-commits 
kinu updated this revision to Diff 555838.
kinu marked an inline comment as done and an inline comment as not done.
kinu added a comment.

Updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159284

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.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
@@ -1436,6 +1436,116 @@
   llvm::Succeeded());
 }
 
+TEST(TransferTest, StructModeledFieldsWithComplicatedInheritance) {
+  std::string Code = R"(
+struct Base1 {
+  int base1_1;
+  int base1_2;
+};
+struct Intermediate : Base1 {
+  int intermediate_1;
+  int intermediate_2;
+};
+struct Base2 {
+  int base2_1;
+  int base2_2;
+};
+struct MostDerived : public Intermediate, Base2 {
+  int most_derived_1;
+  int most_derived_2;
+};
+
+void target() {
+  MostDerived MD;
+  MD.base1_2 = 1;
+  MD.intermediate_2 = 1;
+  MD.base2_2 = 1;
+  MD.most_derived_2 = 1;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+  getEnvironmentAtAnnotation(Results, "p");
+
+// Only the accessed fields should exist in the model.
+auto &MDLoc = getLocForDecl(ASTCtx, Env, "MD");
+std::vector Fields;
+for (auto [Field, _] : MDLoc.children()) {
+  Fields.push_back(Field);
+}
+ASSERT_THAT(Fields, UnorderedElementsAre(
+findValueDecl(ASTCtx, "base1_2"),
+findValueDecl(ASTCtx, "intermediate_2"),
+findValueDecl(ASTCtx, "base2_2"),
+findValueDecl(ASTCtx, "most_derived_2")));
+  });
+}
+
+TEST(TransferTest, StructInitializerListWithComplicatedInheritance) {
+  std::string Code = R"(
+struct Base1 {
+  int base1_1;
+  int base1_2;
+};
+struct Intermediate : Base1 {
+  int intermediate_1;
+  int intermediate_2;
+};
+struct Base2 {
+  int base2_1;
+  int base2_2;
+};
+struct MostDerived : public Intermediate, Base2 {
+  int most_derived_1;
+  int most_derived_2;
+};
+
+void target() {
+  MostDerived MD = {};
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+  getEnvironmentAtAnnotation(Results, "p");
+
+// When a struct is initialized with a initializer list, all the
+// fields are considered "accessed", and therefore do exist.
+auto &MD = getLocForDecl(ASTCtx, Env, "MD");
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base1_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base1_2"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "intermediate_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "intermediate_2"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base2_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base2_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "most_derived_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "most_derived_2"), Env)),
+NotNull());
+  });
+}
+
 TEST(TransferTest, ReferenceMember) {
   std::string Code = R"(
 struct A {
@@ -2041,6 +2151,26 @@
   });
 }
 
+TEST(TransferTest, AssignmentOperatorWithInitAndInheritance) {
+  // This is a crash repro.
+  std::string Code = R"(
+struct B { int Foo; };
+struct S : public B {};
+void target() {
+  S S1 = { 1 };
+  S S2;
+  S S3;
+  S1 = S2;  // Only Dst has InitListExpr.
+  S3 = S1;  // Only Src has InitListExpr.
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {});
+}
+
 TEST(TransferTest, CopyConstructor) {
   std::string Code = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -27,12 +27,12 @@
 #include "clang/Analysis/FlowSensitive/Value.h"

[clang] adb1fb4 - [clang][Interp] Handle mixed floating/integral compound assign operators

2023-09-05 Thread Timm BĂ€der via cfe-commits 

Author: Timm BĂ€der
Date: 2023-09-05T12:10:00+02:00
New Revision: adb1fb40e84d40f9ec3ebfdb0546eadd9a1dc1c9

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

LOG: [clang][Interp] Handle mixed floating/integral compound assign operators

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

Added: 


Modified: 
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/PrimType.h
clang/test/AST/Interp/floats.cpp

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index 131525c98ca59c..d13a805f0714ff 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -878,19 +878,22 @@ bool ByteCodeExprGen::VisitCharacterLiteral(
 template 
 bool ByteCodeExprGen::VisitFloatCompoundAssignOperator(
 const CompoundAssignOperator *E) {
-  assert(E->getType()->isFloatingType());
 
   const Expr *LHS = E->getLHS();
   const Expr *RHS = E->getRHS();
-  llvm::RoundingMode RM = getRoundingMode(E);
+  QualType LHSType = LHS->getType();
   QualType LHSComputationType = E->getComputationLHSType();
   QualType ResultType = E->getComputationResultType();
   std::optional LT = classify(LHSComputationType);
   std::optional RT = classify(ResultType);
 
+  assert(ResultType->isFloatingType());
+
   if (!LT || !RT)
 return false;
 
+  PrimType LHST = classifyPrim(LHSType);
+
   // C++17 onwards require that we evaluate the RHS first.
   // Compute RHS and save it in a temporary variable so we can
   // load it again later.
@@ -904,21 +907,19 @@ bool 
ByteCodeExprGen::VisitFloatCompoundAssignOperator(
   // First, visit LHS.
   if (!visit(LHS))
 return false;
-  if (!this->emitLoad(*LT, E))
+  if (!this->emitLoad(LHST, E))
 return false;
 
   // If necessary, convert LHS to its computation type.
-  if (LHS->getType() != LHSComputationType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHSComputationType);
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
-  return false;
-  }
+  if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType),
+  LHSComputationType, E))
+return false;
 
   // Now load RHS.
   if (!this->emitGetLocal(*RT, TempOffset, E))
 return false;
 
+  llvm::RoundingMode RM = getRoundingMode(E);
   switch (E->getOpcode()) {
   case BO_AddAssign:
 if (!this->emitAddf(RM, E))
@@ -940,17 +941,12 @@ bool 
ByteCodeExprGen::VisitFloatCompoundAssignOperator(
 return false;
   }
 
-  // If necessary, convert result to LHS's type.
-  if (LHS->getType() != ResultType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHS->getType());
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
-  return false;
-  }
+  if (!this->emitPrimCast(classifyPrim(ResultType), LHST, LHS->getType(), E))
+return false;
 
   if (DiscardResult)
-return this->emitStorePop(*LT, E);
-  return this->emitStore(*LT, E);
+return this->emitStorePop(LHST, E);
+  return this->emitStore(LHST, E);
 }
 
 template 
@@ -992,14 +988,6 @@ template 
 bool ByteCodeExprGen::VisitCompoundAssignOperator(
 const CompoundAssignOperator *E) {
 
-  // Handle floating point operations separately here, since they
-  // require special care.
-  if (E->getType()->isFloatingType())
-return VisitFloatCompoundAssignOperator(E);
-
-  if (E->getType()->isPointerType())
-return VisitPointerCompoundAssignOperator(E);
-
   const Expr *LHS = E->getLHS();
   const Expr *RHS = E->getRHS();
   std::optional LHSComputationT =
@@ -1011,6 +999,15 @@ bool 
ByteCodeExprGen::VisitCompoundAssignOperator(
   if (!LT || !RT || !ResultT || !LHSComputationT)
 return false;
 
+  // Handle floating point operations separately here, since they
+  // require special care.
+
+  if (ResultT == PT_Float || RT == PT_Float)
+return VisitFloatCompoundAssignOperator(E);
+
+  if (E->getType()->isPointerType())
+return VisitPointerCompoundAssignOperator(E);
+
   assert(!E->getType()->isPointerType() && "Handled above");
   assert(!E->getType()->isFloatingType() && "Handled above");
 
@@ -2383,6 +2380,39 @@ ByteCodeExprGen::collectBaseOffset(const 
RecordType *BaseType,
   return OffsetSum;
 }
 
+/// Emit casts from a PrimType to another PrimType.
+template 
+bool ByteCodeExprGen::emitPrimCast(PrimType FromT, PrimType ToT,
+QualType ToQT, const Expr *E) {
+
+  if (FromT == PT_Float) {
+// Floating to floating.
+if (ToT == PT_Float) {
+  const llvm::fltSemantics *ToSem = &Ctx.getFloatSemantics(ToQT);
+  return this->emitCastFP(ToSem, getRoundingMode(E), E);
+}
+
+// Float to integral.
+if (isIntegralType(ToT) || ToT == PT_Bool)
+

[PATCH] D157596: [clang][Interp] Handle mixed floating/integral compound assign operators

2023-09-05 Thread Timm BĂ€der 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 rGadb1fb40e84d: [clang][Interp] Handle mixed floating/integral 
compound assign operators (authored by tbaeder).

Changed prior to commit:
  https://reviews.llvm.org/D157596?vs=551434&id=555839#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157596

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

Index: clang/test/AST/Interp/floats.cpp
===
--- clang/test/AST/Interp/floats.cpp
+++ clang/test/AST/Interp/floats.cpp
@@ -102,6 +102,38 @@
 return a[1];
   }
   static_assert(ff() == 3, "");
+
+  constexpr float intPlusDouble() {
+   int a = 0;
+   a += 2.0;
+
+   return a;
+  }
+  static_assert(intPlusDouble() == 2, "");
+
+  constexpr double doublePlusInt() {
+   double a = 0.0;
+   a += 2;
+
+   return a;
+  }
+  static_assert(doublePlusInt() == 2, "");
+
+  constexpr float boolPlusDouble() {
+   bool a = 0;
+   a += 1.0;
+
+   return a;
+  }
+  static_assert(boolPlusDouble(), "");
+
+  constexpr bool doublePlusbool() {
+   double a = 0.0;
+   a += true;
+
+   return a;
+  }
+  static_assert(doublePlusbool() == 1.0, "");
 }
 
 namespace unary {
Index: clang/lib/AST/Interp/PrimType.h
===
--- clang/lib/AST/Interp/PrimType.h
+++ clang/lib/AST/Interp/PrimType.h
@@ -56,7 +56,7 @@
   return OS;
 }
 
-constexpr bool isIntegralType(PrimType T) { return T <= PT_Uint64; }
+constexpr bool isIntegralType(PrimType T) { return T <= PT_Bool; }
 
 /// Mapping from primitive types to their representation.
 template  struct PrimConv;
Index: clang/lib/AST/Interp/ByteCodeExprGen.h
===
--- clang/lib/AST/Interp/ByteCodeExprGen.h
+++ clang/lib/AST/Interp/ByteCodeExprGen.h
@@ -265,6 +265,7 @@
 return FPO.getRoundingMode();
   }
 
+  bool emitPrimCast(PrimType FromT, PrimType ToT, QualType ToQT, const Expr *E);
   bool emitRecordDestruction(const Descriptor *Desc);
   bool emitDerivedToBaseCasts(const RecordType *DerivedType,
   const RecordType *BaseType, const Expr *E);
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -878,19 +878,22 @@
 template 
 bool ByteCodeExprGen::VisitFloatCompoundAssignOperator(
 const CompoundAssignOperator *E) {
-  assert(E->getType()->isFloatingType());
 
   const Expr *LHS = E->getLHS();
   const Expr *RHS = E->getRHS();
-  llvm::RoundingMode RM = getRoundingMode(E);
+  QualType LHSType = LHS->getType();
   QualType LHSComputationType = E->getComputationLHSType();
   QualType ResultType = E->getComputationResultType();
   std::optional LT = classify(LHSComputationType);
   std::optional RT = classify(ResultType);
 
+  assert(ResultType->isFloatingType());
+
   if (!LT || !RT)
 return false;
 
+  PrimType LHST = classifyPrim(LHSType);
+
   // C++17 onwards require that we evaluate the RHS first.
   // Compute RHS and save it in a temporary variable so we can
   // load it again later.
@@ -904,21 +907,19 @@
   // First, visit LHS.
   if (!visit(LHS))
 return false;
-  if (!this->emitLoad(*LT, E))
+  if (!this->emitLoad(LHST, E))
 return false;
 
   // If necessary, convert LHS to its computation type.
-  if (LHS->getType() != LHSComputationType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHSComputationType);
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
-  return false;
-  }
+  if (!this->emitPrimCast(LHST, classifyPrim(LHSComputationType),
+  LHSComputationType, E))
+return false;
 
   // Now load RHS.
   if (!this->emitGetLocal(*RT, TempOffset, E))
 return false;
 
+  llvm::RoundingMode RM = getRoundingMode(E);
   switch (E->getOpcode()) {
   case BO_AddAssign:
 if (!this->emitAddf(RM, E))
@@ -940,17 +941,12 @@
 return false;
   }
 
-  // If necessary, convert result to LHS's type.
-  if (LHS->getType() != ResultType) {
-const auto *TargetSemantics = &Ctx.getFloatSemantics(LHS->getType());
-
-if (!this->emitCastFP(TargetSemantics, RM, E))
-  return false;
-  }
+  if (!this->emitPrimCast(classifyPrim(ResultType), LHST, LHS->getType(), E))
+return false;
 
   if (DiscardResult)
-return this->emitStorePop(*LT, E);
-  return this->emitStore(*LT, E);
+return this->emitStorePop(LHST, E);
+  return this->emitStore(LHST, E);
 }
 
 template 
@@ -992,14 +988,6 @@
 bool ByteCodeExprGen::VisitCompoundAssignOperator(
 const CompoundAssignOperator *E) {
 
-  // Handle floating point op

[PATCH] D159284: [clang][dataflow] Fix Record initialization with InitListExpr and inheritances

2023-09-05 Thread Kinuko Yasuda via Phabricator via cfe-commits 
kinu updated this revision to Diff 555840.
kinu marked 2 inline comments as done.
kinu added a comment.

More fixes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159284

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.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
@@ -1436,6 +1436,116 @@
   llvm::Succeeded());
 }
 
+TEST(TransferTest, StructModeledFieldsWithComplicatedInheritance) {
+  std::string Code = R"(
+struct Base1 {
+  int base1_1;
+  int base1_2;
+};
+struct Intermediate : Base1 {
+  int intermediate_1;
+  int intermediate_2;
+};
+struct Base2 {
+  int base2_1;
+  int base2_2;
+};
+struct MostDerived : public Intermediate, Base2 {
+  int most_derived_1;
+  int most_derived_2;
+};
+
+void target() {
+  MostDerived MD;
+  MD.base1_2 = 1;
+  MD.intermediate_2 = 1;
+  MD.base2_2 = 1;
+  MD.most_derived_2 = 1;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+  getEnvironmentAtAnnotation(Results, "p");
+
+// Only the accessed fields should exist in the model.
+auto &MDLoc = getLocForDecl(ASTCtx, Env, "MD");
+std::vector Fields;
+for (auto [Field, _] : MDLoc.children()) {
+  Fields.push_back(Field);
+}
+ASSERT_THAT(Fields, UnorderedElementsAre(
+findValueDecl(ASTCtx, "base1_2"),
+findValueDecl(ASTCtx, "intermediate_2"),
+findValueDecl(ASTCtx, "base2_2"),
+findValueDecl(ASTCtx, "most_derived_2")));
+  });
+}
+
+TEST(TransferTest, StructInitializerListWithComplicatedInheritance) {
+  std::string Code = R"(
+struct Base1 {
+  int base1_1;
+  int base1_2;
+};
+struct Intermediate : Base1 {
+  int intermediate_1;
+  int intermediate_2;
+};
+struct Base2 {
+  int base2_1;
+  int base2_2;
+};
+struct MostDerived : public Intermediate, Base2 {
+  int most_derived_1;
+  int most_derived_2;
+};
+
+void target() {
+  MostDerived MD = {};
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+  getEnvironmentAtAnnotation(Results, "p");
+
+// When a struct is initialized with a initializer list, all the
+// fields are considered "accessed", and therefore do exist.
+auto &MD = getLocForDecl(ASTCtx, Env, "MD");
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base1_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base1_2"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "intermediate_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "intermediate_2"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base2_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base2_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "most_derived_1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "most_derived_2"), Env)),
+NotNull());
+  });
+}
+
 TEST(TransferTest, ReferenceMember) {
   std::string Code = R"(
 struct A {
@@ -2041,6 +2151,26 @@
   });
 }
 
+TEST(TransferTest, AssignmentOperatorWithInitAndInheritance) {
+  // This is a crash repro.
+  std::string Code = R"(
+struct B { int Foo; };
+struct S : public B {};
+void target() {
+  S S1 = { 1 };
+  S S2;
+  S S3;
+  S1 = S2;  // Only Dst has InitListExpr.
+  S3 = S1;  // Only Src has InitListExpr.
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {});
+}
+
 TEST(TransferTest, CopyConstructor) {
   std::string Code = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -27,12 +27,12 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/Builtin

[clang] 5704dde - [clang][Interp] Reject calling virtual constexpr functions in pre-c++20

2023-09-05 Thread Timm BĂ€der via cfe-commits 

Author: Timm BĂ€der
Date: 2023-09-05T12:21:26+02:00
New Revision: 5704dde307359e9155e2fcd99c413c0cf932aa13

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

LOG: [clang][Interp] Reject calling virtual constexpr functions in pre-c++20

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

Added: 


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

Removed: 




diff  --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp 
b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index d13a805f0714ff..244290dc6393f4 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1816,24 +1816,7 @@ Record *ByteCodeExprGen::getRecord(const 
RecordDecl *RD) {
 
 template 
 const Function *ByteCodeExprGen::getFunction(const FunctionDecl *FD) {
-  assert(FD);
-  const Function *Func = P.getFunction(FD);
-  bool IsBeingCompiled = Func && !Func->isFullyCompiled();
-  bool WasNotDefined = Func && !Func->isConstexpr() && !Func->hasBody();
-
-  if (IsBeingCompiled)
-return Func;
-
-  if (!Func || WasNotDefined) {
-if (auto R = ByteCodeStmtGen(Ctx, P).compileFunc(FD))
-  Func = *R;
-else {
-  llvm::consumeError(R.takeError());
-  return nullptr;
-}
-  }
-
-  return Func;
+  return Ctx.getOrCreateFunction(FD);
 }
 
 template 

diff  --git a/clang/lib/AST/Interp/Context.cpp 
b/clang/lib/AST/Interp/Context.cpp
index 6e0d949457d673..1a732b6c1a092a 100644
--- a/clang/lib/AST/Interp/Context.cpp
+++ b/clang/lib/AST/Interp/Context.cpp
@@ -210,3 +210,24 @@ Context::getOverridingFunction(const CXXRecordDecl 
*DynamicDecl,
   "Couldn't find an overriding function in the class hierarchy?");
   return nullptr;
 }
+
+const Function *Context::getOrCreateFunction(const FunctionDecl *FD) {
+  assert(FD);
+  const Function *Func = P->getFunction(FD);
+  bool IsBeingCompiled = Func && !Func->isFullyCompiled();
+  bool WasNotDefined = Func && !Func->isConstexpr() && !Func->hasBody();
+
+  if (IsBeingCompiled)
+return Func;
+
+  if (!Func || WasNotDefined) {
+if (auto R = ByteCodeStmtGen(*this, *P).compileFunc(FD))
+  Func = *R;
+else {
+  llvm::consumeError(R.takeError());
+  return nullptr;
+}
+  }
+
+  return Func;
+}

diff  --git a/clang/lib/AST/Interp/Context.h b/clang/lib/AST/Interp/Context.h
index f5dc80af757a14..617fbaa928f40e 100644
--- a/clang/lib/AST/Interp/Context.h
+++ b/clang/lib/AST/Interp/Context.h
@@ -72,6 +72,9 @@ class Context final {
   getOverridingFunction(const CXXRecordDecl *DynamicDecl,
 const CXXRecordDecl *StaticDecl,
 const CXXMethodDecl *InitialFunction) const;
+
+  const Function *getOrCreateFunction(const FunctionDecl *FD);
+
   /// Returns whether we should create a global variable for the
   /// given ValueDecl.
   static bool shouldBeGloballyIndexed(const ValueDecl *VD) {

diff  --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index b87a5c1cbd0e48..5006f72fe7237f 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1765,7 +1765,15 @@ inline bool CallVirt(InterpState &S, CodePtr OpPC, const 
Function *Func) {
   DynamicDecl, StaticDecl, InitialFunction);
 
   if (Overrider != InitialFunction) {
-Func = S.P.getFunction(Overrider);
+// DR1872: An instantiated virtual constexpr function can't be called in a
+// constant expression (prior to C++20). We can still constant-fold such a
+// call.
+if (!S.getLangOpts().CPlusPlus20 && Overrider->isVirtual()) {
+  const Expr *E = S.Current->getExpr(OpPC);
+  S.CCEDiag(E, diag::note_constexpr_virtual_call) << E->getSourceRange();
+}
+
+Func = S.getContext().getOrCreateFunction(Overrider);
 
 const CXXRecordDecl *ThisFieldDecl =
 ThisPtr.getFieldDesc()->getType()->getAsCXXRecordDecl();

diff  --git a/clang/test/AST/Interp/records.cpp 
b/clang/test/AST/Interp/records.cpp
index 7976f2d91e99c0..4e39ded41ad25f 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -871,6 +871,35 @@ namespace VirtualFunctionPointers {
 };
 #endif
 
+#if __cplusplus < 202002L
+namespace VirtualFromBase {
+  struct S1 {
+virtual int f() const;
+  };
+  struct S2 {
+virtual int f();
+  };
+  template  struct X : T {
+constexpr X() {}
+double d = 0.0;
+constexpr int f() { return sizeof(T); }
+  };
+
+  // Non-virtual f(), OK.
+  constexpr X> xxs1;
+  constexpr X *p = const_cast>*>(&xxs1);
+  static_assert(p->f() == sizeof(S1), "");
+
+  // Virtual f(), not OK.
+  constexpr X> xxs2;
+  constexpr X *q = const_cast>*>(&xxs2);
+  static_assert(

[PATCH] D157619: [clang][Interp] Reject calling virtual constexpr functions in pre-c++20

2023-09-05 Thread Timm BĂ€der 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 rG5704dde30735: [clang][Interp] Reject calling virtual 
constexpr functions in pre-c++20 (authored by tbaeder).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157619

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

Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -871,6 +871,35 @@
 };
 #endif
 
+#if __cplusplus < 202002L
+namespace VirtualFromBase {
+  struct S1 {
+virtual int f() const;
+  };
+  struct S2 {
+virtual int f();
+  };
+  template  struct X : T {
+constexpr X() {}
+double d = 0.0;
+constexpr int f() { return sizeof(T); }
+  };
+
+  // Non-virtual f(), OK.
+  constexpr X> xxs1;
+  constexpr X *p = const_cast>*>(&xxs1);
+  static_assert(p->f() == sizeof(S1), "");
+
+  // Virtual f(), not OK.
+  constexpr X> xxs2;
+  constexpr X *q = const_cast>*>(&xxs2);
+  static_assert(q->f() == sizeof(X), ""); // ref-error {{not an integral constant expression}} \
+  // ref-note {{cannot evaluate call to virtual function}} \
+  // expected-error {{not an integral constant expression}} \
+  // expected-note {{cannot evaluate call to virtual function}}
+}
+#endif
+
 namespace CompositeDefaultArgs {
   struct Foo {
 int a;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -1765,7 +1765,15 @@
   DynamicDecl, StaticDecl, InitialFunction);
 
   if (Overrider != InitialFunction) {
-Func = S.P.getFunction(Overrider);
+// DR1872: An instantiated virtual constexpr function can't be called in a
+// constant expression (prior to C++20). We can still constant-fold such a
+// call.
+if (!S.getLangOpts().CPlusPlus20 && Overrider->isVirtual()) {
+  const Expr *E = S.Current->getExpr(OpPC);
+  S.CCEDiag(E, diag::note_constexpr_virtual_call) << E->getSourceRange();
+}
+
+Func = S.getContext().getOrCreateFunction(Overrider);
 
 const CXXRecordDecl *ThisFieldDecl =
 ThisPtr.getFieldDesc()->getType()->getAsCXXRecordDecl();
Index: clang/lib/AST/Interp/Context.h
===
--- clang/lib/AST/Interp/Context.h
+++ clang/lib/AST/Interp/Context.h
@@ -72,6 +72,9 @@
   getOverridingFunction(const CXXRecordDecl *DynamicDecl,
 const CXXRecordDecl *StaticDecl,
 const CXXMethodDecl *InitialFunction) const;
+
+  const Function *getOrCreateFunction(const FunctionDecl *FD);
+
   /// Returns whether we should create a global variable for the
   /// given ValueDecl.
   static bool shouldBeGloballyIndexed(const ValueDecl *VD) {
Index: clang/lib/AST/Interp/Context.cpp
===
--- clang/lib/AST/Interp/Context.cpp
+++ clang/lib/AST/Interp/Context.cpp
@@ -210,3 +210,24 @@
   "Couldn't find an overriding function in the class hierarchy?");
   return nullptr;
 }
+
+const Function *Context::getOrCreateFunction(const FunctionDecl *FD) {
+  assert(FD);
+  const Function *Func = P->getFunction(FD);
+  bool IsBeingCompiled = Func && !Func->isFullyCompiled();
+  bool WasNotDefined = Func && !Func->isConstexpr() && !Func->hasBody();
+
+  if (IsBeingCompiled)
+return Func;
+
+  if (!Func || WasNotDefined) {
+if (auto R = ByteCodeStmtGen(*this, *P).compileFunc(FD))
+  Func = *R;
+else {
+  llvm::consumeError(R.takeError());
+  return nullptr;
+}
+  }
+
+  return Func;
+}
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -1816,24 +1816,7 @@
 
 template 
 const Function *ByteCodeExprGen::getFunction(const FunctionDecl *FD) {
-  assert(FD);
-  const Function *Func = P.getFunction(FD);
-  bool IsBeingCompiled = Func && !Func->isFullyCompiled();
-  bool WasNotDefined = Func && !Func->isConstexpr() && !Func->hasBody();
-
-  if (IsBeingCompiled)
-return Func;
-
-  if (!Func || WasNotDefined) {
-if (auto R = ByteCodeStmtGen(Ctx, P).compileFunc(FD))
-  Func = *R;
-else {
-  llvm::consumeError(R.takeError());
-  return nullptr;
-}
-  }
-
-  return Func;
+  return Ctx.getOrCreateFunction(FD);
 }
 
 template 
___
cfe-commits m

[clang] fde2b0d - [clang][Interp][NFC] Add [[nodiscard]] to a few Pointer methods

2023-09-05 Thread Timm BĂ€der via cfe-commits 

Author: Timm BĂ€der
Date: 2023-09-05T12:35:06+02:00
New Revision: fde2b0d6dba709ee6d699a4d67657604b8c0a026

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

LOG: [clang][Interp][NFC] Add [[nodiscard]] to a few Pointer methods

These methods are all const and return a new Pointer instance instead of
modifying the given instance.

Added: 


Modified: 
clang/lib/AST/Interp/Pointer.h

Removed: 




diff  --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h
index f2d8922f896552..3834237f11d131 100644
--- a/clang/lib/AST/Interp/Pointer.h
+++ b/clang/lib/AST/Interp/Pointer.h
@@ -92,7 +92,7 @@ class Pointer {
   APValue toRValue(const Context &Ctx) const;
 
   /// Offsets a pointer inside an array.
-  Pointer atIndex(unsigned Idx) const {
+  [[nodiscard]] Pointer atIndex(unsigned Idx) const {
 if (Base == RootPtrMark)
   return Pointer(Pointee, RootPtrMark, getDeclDesc()->getSize());
 unsigned Off = Idx * elemSize();
@@ -104,21 +104,21 @@ class Pointer {
   }
 
   /// Creates a pointer to a field.
-  Pointer atField(unsigned Off) const {
+  [[nodiscard]] Pointer atField(unsigned Off) const {
 unsigned Field = Offset + Off;
 return Pointer(Pointee, Field, Field);
   }
 
   /// Subtract the given offset from the current Base and Offset
   /// of the pointer.
-  Pointer atFieldSub(unsigned Off) const {
+  [[nodiscard]]  Pointer atFieldSub(unsigned Off) const {
 assert(Offset >= Off);
 unsigned O = Offset - Off;
 return Pointer(Pointee, O, O);
   }
 
   /// Restricts the scope of an array element pointer.
-  Pointer narrow() const {
+  [[nodiscard]] Pointer narrow() const {
 // Null pointers cannot be narrowed.
 if (isZero() || isUnknownSizeArray())
   return *this;
@@ -154,7 +154,7 @@ class Pointer {
   }
 
   /// Expands a pointer to the containing array, undoing narrowing.
-  Pointer expand() const {
+  [[nodiscard]] Pointer expand() const {
 if (isElementPastEnd()) {
   // Revert to an outer one-past-end pointer.
   unsigned Adjust;
@@ -193,7 +193,7 @@ class Pointer {
   SourceLocation getDeclLoc() const { return getDeclDesc()->getLocation(); }
 
   /// Returns a pointer to the object of which this pointer is a field.
-  Pointer getBase() const {
+  [[nodiscard]] Pointer getBase() const {
 if (Base == RootPtrMark) {
   assert(Offset == PastEndMark && "cannot get base of a block");
   return Pointer(Pointee, Base, 0);
@@ -203,7 +203,7 @@ class Pointer {
 return Pointer(Pointee, NewBase, NewBase);
   }
   /// Returns the parent array.
-  Pointer getArray() const {
+  [[nodiscard]] Pointer getArray() const {
 if (Base == RootPtrMark) {
   assert(Offset != 0 && Offset != PastEndMark && "not an array element");
   return Pointer(Pointee, Base, 0);
@@ -226,7 +226,7 @@ class Pointer {
 return getFieldDesc()->getType();
   }
 
-  Pointer getDeclPtr() const { return Pointer(Pointee); }
+  [[nodiscard]] Pointer getDeclPtr() const { return Pointer(Pointee); }
 
   /// Returns the element size of the innermost field.
   size_t elemSize() const {



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

[PATCH] D159284: [clang][dataflow] Fix Record initialization with InitListExpr and inheritances

2023-09-05 Thread Kinuko Yasuda via Phabricator via cfe-commits 
kinu updated this revision to Diff 555842.
kinu marked 5 inline comments as done.
kinu added a comment.

Updated again... very easy to overlook unaddressed comments!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159284

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.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
@@ -1436,6 +1436,99 @@
   llvm::Succeeded());
 }
 
+TEST(TransferTest, StructModeledFieldsWithComplicatedInheritance) {
+  std::string Code = R"(
+struct Base1 {
+  int base1_1;
+  int base1_2;
+};
+struct Intermediate : Base1 {
+  int intermediate_1;
+  int intermediate_2;
+};
+struct Base2 {
+  int base2_1;
+  int base2_2;
+};
+struct MostDerived : public Intermediate, Base2 {
+  int most_derived_1;
+  int most_derived_2;
+};
+
+void target() {
+  MostDerived MD;
+  MD.base1_2 = 1;
+  MD.intermediate_2 = 1;
+  MD.base2_2 = 1;
+  MD.most_derived_2 = 1;
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+  getEnvironmentAtAnnotation(Results, "p");
+
+// Only the accessed fields should exist in the model.
+auto &MDLoc = getLocForDecl(ASTCtx, Env, "MD");
+std::vector Fields;
+for (auto [Field, _] : MDLoc.children())
+  Fields.push_back(Field);
+ASSERT_THAT(Fields, UnorderedElementsAre(
+findValueDecl(ASTCtx, "base1_2"),
+findValueDecl(ASTCtx, "intermediate_2"),
+findValueDecl(ASTCtx, "base2_2"),
+findValueDecl(ASTCtx, "most_derived_2")));
+  });
+}
+
+TEST(TransferTest, StructInitializerListWithComplicatedInheritance) {
+  std::string Code = R"(
+struct Base1 {
+  int base1;
+};
+struct Intermediate : Base1 {
+  int intermediate;
+};
+struct Base2 {
+  int base2;
+};
+struct MostDerived : public Intermediate, Base2 {
+  int most_derived;
+};
+
+void target() {
+  MostDerived MD = {};
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {
+const Environment &Env =
+  getEnvironmentAtAnnotation(Results, "p");
+
+// When a struct is initialized with a initializer list, all the
+// fields are considered "accessed", and therefore do exist.
+auto &MD = getLocForDecl(ASTCtx, Env, "MD");
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base1"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "intermediate"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "base2"), Env)),
+NotNull());
+ASSERT_THAT(cast(
+getFieldValue(&MD, *findValueDecl(ASTCtx, "most_derived"), Env)),
+NotNull());
+  });
+}
+
 TEST(TransferTest, ReferenceMember) {
   std::string Code = R"(
 struct A {
@@ -2041,6 +2134,26 @@
   });
 }
 
+TEST(TransferTest, AssignmentOperatorWithInitAndInheritance) {
+  // This is a crash repro.
+  std::string Code = R"(
+struct B { int Foo; };
+struct S : public B {};
+void target() {
+  S S1 = { 1 };
+  S S2;
+  S S3;
+  S1 = S2;  // Only Dst has InitListExpr.
+  S3 = S1;  // Only Src has InitListExpr.
+  // [[p]]
+}
+  )";
+  runDataflow(
+  Code,
+  [](const llvm::StringMap> &Results,
+ ASTContext &ASTCtx) {});
+}
+
 TEST(TransferTest, CopyConstructor) {
   std::string Code = R"(
 struct A {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -27,12 +27,12 @@
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/Builtins.h"
 #include "clang/Basic/OperatorKinds.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
-#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Debug.h"
+#include 
 #include 
-#include 
-#include 
+
+#define DEBUG_TYPE "dataflow"
 
 namespace clang {
 namespace dataflow {
@@ -629,17 +629,66 @@
   return;
 }
 
-std::vector Fields =
-getFieldsForInitListExpr(Type->getAsRecordDecl());
 llvm::DenseMap FieldLocs;
 
-for (auto [Field, Init] : llvm::zip(Fields, S->inits())) {
-  assert(Field != nullptr);
-  assert(Init != n

[PATCH] D159284: [clang][dataflow] Fix Record initialization with InitListExpr and inheritances

2023-09-05 Thread Kinuko Yasuda via Phabricator via cfe-commits 
kinu added a comment.

Thanks, addressed the comments, PTAL~




Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:688-690
+  for ([[maybe_unused]] auto [Field, Loc] : FieldLocs) {
+assert(ModeledFields.contains(cast_or_null(Field)));
+  }

mboehme wrote:
> https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
Does it work even if assert() is dropped? ... looks like it does.  Ok, applied.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:672-673
+  // The types are same, or
+  GetCanonicalType(Field->getType()) ==
+  GetCanonicalType(Init->getType()) ||
+  // The field's type is T&, and initializer is T

mboehme wrote:
> mboehme wrote:
> > Again, this is the prvalue case, so I think you should simply be able to 
> > use `QualType::getCanonicalType()`.
> Thinking about this again -- the field can, of course, have qualifiers, while 
> the `Init` as a prvalue should not, so I think this should read
> 
> ```
> Field->getType().getCanonicalType().getUnqualifiedType() == 
> Init->getType().getCanonicalType()
> ```
> 
> Sorry for the back and forth!
No problem, thanks for clarifying the details, done.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:1528-1530
+  F.x4 = F.x2;
+  F.x6 = 1;
+  F.x8 = 1;

mboehme wrote:
> mboehme wrote:
> > These lines seem unnecessary. The test only wants to check whether the 
> > fields are modeled, not what their values are.
> > 
> > If anything, leaving out these operations makes the test stronger: It then 
> > tests whether the fields are modeled even if they are only initialized with 
> > an empty `{}`.
> > 
> > Then, once we leave out these operations, it's probably sufficient to give 
> > each class in this test just a single member variable (which is another 
> > nice simplification).
> Reopening the comment to discuss this line:
> 
> > Then, once we leave out these operations, it's probably sufficient to give 
> > each class in this test just a single member variable (which is another 
> > nice simplification).
> 
> I see that giving each class two member variables makes this test consistent 
> with the test above -- but OTOH, we're testing exactly the same thing for 
> both member variables, so maybe it's better on the whole to simplify the test 
> by putting just one member variable in each class?
That's true for this test... now I made each class have only one member.



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2191
+TEST(TransferTest, AssignmentOperatorWithInitAndInheritance) {
+  // This is a crash repro.
+  std::string Code = R"(

mboehme wrote:
> kinu wrote:
> > mboehme wrote:
> > > IIUC, the crash was happening because `copyRecord()` assumes that the 
> > > source and destination have the same fields. More specifically, there was 
> > > an unspoken invariant in the framework that a `RecordStorageLocation` 
> > > should always contain exactly the fields returned by 
> > > `DataflowAnalysisContext::getModeledFields()`), but our treatment of 
> > > `InitListExpr` was violating this invariant.
> > > 
> > > IOW, this wasn't really an issue with the way we treat copy/move 
> > > operations but with `InitlistExpr`.
> > > 
> > > I understand that we want to have a test the reproduces the exact 
> > > circumstances that led to the crash, but maybe it's enough to have one of 
> > > these, and have the focus of the testing be on testing the actual 
> > > invariant that we want to maintain? IOW, maybe keep this test but delete 
> > > the additional tests below?
> > Dropped the additional tests below.
> I still see more tests below? Suggest dropping these.
Ah I see what you mean.  I dropped more!



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:2498
+TEST(TransferTest, ReturnStructWithInheritance) {
+  // This is a crash repro. (Returning a struct involves copy/move constructor)
+  std::string Code = R"(

mboehme wrote:
> kinu wrote:
> > mboehme wrote:
> > > I don't think this is true -- the `return S1` in `target()` will use NRVO.
> > > 
> > > But I also don't think it's relevant -- I believe the crash this is 
> > > reproducing was triggered by the `InitListExpr`, not the return statement?
> > > 
> > > Given the other tests added in this patch, do we need this test?
> > It will use NRVO but in AST this still seems to generate CXXConstructExpr 
> > with isCopyOrMoveConstructor() == true because omitting the ctor is not 
> > mandatory in compilers.
> > 
> > I can drop this one, but I'm also a bit torn because this was the original 
> > crash repro that puzzled me a bit.
> > 
> > I refined the comment to explain it a bit better; how does it look now?
> > It will use NRVO but in AST this still seems to generate

[PATCH] D158730: [DebugMetadata][DwarfDebug] Don't retain local types with -fno-eliminate-unused-debug-types

2023-09-05 Thread Vladislav Dzhidzhoev via Phabricator via cfe-commits 
dzhidzhoev updated this revision to Diff 555844.
dzhidzhoev added a comment.

Rebased on top of https://reviews.llvm.org/D155818.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158730

Files:
  clang/test/CodeGen/debug-info-unused-types.c
  clang/test/CodeGen/debug-info-unused-types.cpp
  llvm/lib/IR/DIBuilder.cpp


Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -625,7 +625,8 @@
   assert((isa(T) || (isa(T) &&
  cast(T)->isDefinition() == false)) 
&&
  "Expected type or subprogram declaration");
-  AllRetainTypes.emplace_back(T);
+  if (!isa_and_nonnull(T->getScope()))
+AllRetainTypes.emplace_back(T);
 }
 
 DIBasicType *DIBuilder::createUnspecifiedParameter() { return nullptr; }
Index: clang/test/CodeGen/debug-info-unused-types.cpp
===
--- clang/test/CodeGen/debug-info-unused-types.cpp
+++ clang/test/CodeGen/debug-info-unused-types.cpp
@@ -13,12 +13,14 @@
 // CHECK: !DICompileUnit{{.+}}retainedTypes: [[RETTYPES:![0-9]+]]
 // CHECK: [[TYPE0:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, 
name: "baz"
 // CHECK: [[TYPE1:![0-9]+]] = !DIEnumerator(name: "BAZ"
-// CHECK: [[TYPE2:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, 
name: "z"
-// CHECK: [[TYPE3:![0-9]+]] = !DIEnumerator(name: "Z"
-// CHECK: [[RETTYPES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]], [[TYPE0]], 
{{![0-9]+}}, [[TYPE6:![0-9]+]], [[TYPE2]]}
-// CHECK: [[TYPE4]] = !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
-// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: 
"bar"
-// CHECK: [[TYPE6]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: 
"y"
+// CHECK: [[RETTYPES]] = !{[[TYPE2:![0-9]+]], [[TYPE3:![0-9]+]], [[TYPE0]], 
{{![0-9]+}}}
+// CHECK: [[TYPE2]] = !DIDerivedType(tag: DW_TAG_typedef, name: "foo"
+// CHECK: [[TYPE3]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: 
"bar"
+// CHECK: [[SP:![0-9]+]] = distinct !DISubprogram(name: "quux", {{.*}}, 
retainedNodes: [[SPRETNODES:![0-9]+]]
+// CHECK: [[SPRETNODES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]]}
+// CHECK: [[TYPE4]] = distinct !DICompositeType(tag: DW_TAG_class_type, name: 
"y", scope: [[SP]]
+// CHECK: [[TYPE5]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: 
"z", scope: [[SP]]
+// CHECK: [[TYPE6:![0-9]+]] = !DIEnumerator(name: "Z"
 
 // NODBG-NOT: !DI{{CompositeType|Enumerator|DerivedType}}
 
Index: clang/test/CodeGen/debug-info-unused-types.c
===
--- clang/test/CodeGen/debug-info-unused-types.c
+++ clang/test/CodeGen/debug-info-unused-types.c
@@ -18,13 +18,15 @@
 // CHECK: !DICompileUnit{{.+}}retainedTypes: [[RETTYPES:![0-9]+]]
 // CHECK: [[TYPE0:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, 
name: "bar"
 // CHECK: [[TYPE1:![0-9]+]] = !DIEnumerator(name: "BAR"
-// CHECK: [[TYPE2:![0-9]+]] = !DICompositeType(tag: DW_TAG_enumeration_type, 
name: "z"
-// CHECK: [[TYPE3:![0-9]+]] = !DIEnumerator(name: "Z"
-// CHECK: [[RETTYPES]] = !{[[TYPE4:![0-9]+]], [[TYPE5:![0-9]+]], [[TYPE0]], 
[[TYPE6:![0-9]+]], {{![0-9]+}}, [[TYPE7:![0-9]+]], [[TYPE2]], [[TYPE8:![0-9]+]]}
-// CHECK: [[TYPE4]] = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int"
-// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "foo"
-// CHECK: [[TYPE6]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: 
"baz"
-// CHECK: [[TYPE7]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "y"
+// CHECK: [[RETTYPES]] = !{[[TYPE2:![0-9]+]], [[TYPE3:![0-9]+]], [[TYPE0]], 
[[TYPE4:![0-9]+]], {{![0-9]+}}}
+// CHECK: [[TYPE2]] = !DIDerivedType(tag: DW_TAG_typedef, name: "my_int"
+// CHECK: [[TYPE3]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "foo"
+// CHECK: [[TYPE4]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: 
"baz"
+// CHECK: [[SP:![0-9]+]] = distinct !DISubprogram(name: "quux", {{.*}}, 
retainedNodes: [[SPRETNODES:![0-9]+]]
+// CHECK: [[SPRETNODES]] = !{[[TYPE5:![0-9]+]], [[TYPE6:![0-9]+]], 
[[TYPE8:![0-9]+]]}
+// CHECK: [[TYPE5]] = distinct !DICompositeType(tag: DW_TAG_structure_type, 
name: "y"
+// CHECK: [[TYPE6]] = !DICompositeType(tag: DW_TAG_enumeration_type, name: "z"
+// CHECK: [[TYPE7:![0-9]+]] = !DIEnumerator(name: "Z"
 // CHECK: [[TYPE8]] = distinct !DICompositeType(tag: DW_TAG_union_type, name: 
"w"
 
 // Check that debug info is not emitted for the typedef, struct, enum, and


Index: llvm/lib/IR/DIBuilder.cpp
===
--- llvm/lib/IR/DIBuilder.cpp
+++ llvm/lib/IR/DIBuilder.cpp
@@ -625,7 +625,8 @@
   assert((isa(T) || (isa(T) &&
  cast(T)->isDefinition() == false)) &&
  "Expected type or subprogram

[PATCH] D144006: [DebugMetadata][DwarfDebug] Support function-local types in lexical block scopes (4/7)

2023-09-05 Thread Vladislav Dzhidzhoev via Phabricator via cfe-commits 
dzhidzhoev updated this revision to Diff 555845.
dzhidzhoev added a comment.

Rebased on top of https://reviews.llvm.org/D158730.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144006

Files:
  clang/test/CodeGen/debug-info-codeview-unnamed.c
  clang/test/CodeGenCXX/debug-info-access.cpp
  clang/test/CodeGenCXX/debug-info-anon-union-vars.cpp
  clang/test/CodeGenCXX/debug-info-codeview-unnamed.cpp
  clang/test/CodeGenCXX/debug-info-gline-tables-only-codeview.cpp
  clang/test/CodeGenCXX/debug-lambda-this.cpp
  llvm/include/llvm/IR/DIBuilder.h
  llvm/lib/Bitcode/Reader/MetadataLoader.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
  llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
  llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
  llvm/lib/IR/DIBuilder.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Bitcode/upgrade-cu-locals.ll
  llvm/test/Bitcode/upgrade-cu-locals.ll.bc
  llvm/test/DebugInfo/Generic/inlined-local-type.ll
  llvm/test/DebugInfo/Generic/lexical-block-retained-types.ll
  llvm/test/DebugInfo/Generic/lexical-block-types.ll
  llvm/test/DebugInfo/Generic/split-dwarf-local-import.ll
  llvm/test/DebugInfo/Generic/split-dwarf-local-import2.ll
  llvm/test/DebugInfo/Generic/split-dwarf-local-import3.ll
  llvm/test/DebugInfo/Generic/verifier-invalid-disubprogram.ll
  llvm/test/DebugInfo/X86/local-type-as-template-parameter.ll
  llvm/test/DebugInfo/X86/set.ll
  llvm/test/DebugInfo/X86/split-dwarf-local-import.ll
  llvm/test/DebugInfo/X86/split-dwarf-local-import2.ll
  llvm/test/DebugInfo/X86/split-dwarf-local-import3.ll

Index: llvm/test/DebugInfo/Generic/split-dwarf-local-import3.ll
===
--- /dev/null
+++ llvm/test/DebugInfo/Generic/split-dwarf-local-import3.ll
@@ -1,70 +0,0 @@
-; REQUIRES: x86_64-linux
-; RUN: %llc_dwarf -O1 -filetype=obj -split-dwarf-file=%t.dwo < %s \
-; RUN:   | llvm-dwarfdump -debug-info -   \
-; RUN:   | FileCheck %s --implicit-check-not "{{DW_TAG|NULL}}"
-
-target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
-target triple = "x86_64-unknown-linux-gnu"
-
-; Ensure that the imported entity 'nn::A' gets emitted in 'foo()'s abstract tree
-; in the destination (where 'foo()' was inlined) compile unit.
-
-; CHECK-LABEL: .debug_info contents
-; CHECK: DW_TAG_skeleton_unit
-; CHECK:   DW_AT_dwo_name
-
-; CHECK-LABEL: .debug_info.dwo contents:
-; CHECK: DW_TAG_compile_unit
-; CHECK:   DW_AT_name ("test.cpp")
-; CHECK:   DW_AT_dwo_name
-
-; CHECK:   [[ABSTRACT_FOO:0x[0-9a-f]+]]: DW_TAG_subprogram
-; CHECK: DW_AT_name	("foo")
-; CHECK: DW_TAG_imported_declaration
-; CHECK:   DW_AT_import  ([[A:0x[0-9a-f]+]])
-; CHECK: NULL
-
-; CHECK:   DW_TAG_base_type
-; CHECK: DW_AT_name	("int")
-
-; CHECK:   DW_TAG_subprogram
-; CHECK: DW_AT_name	("main")
-; CHECK: DW_TAG_inlined_subroutine
-; CHECK:   DW_AT_abstract_origin ([[ABSTRACT_FOO]] "_Z3foov")
-; CHECK: NULL
-
-; CHECK:   DW_TAG_namespace
-; CHECK: DW_AT_name	("nn")
-; CHECK: [[A]]: DW_TAG_variable
-; CHECK:   DW_AT_name	("A")
-; CHECK: NULL
-; CHECK:   NULL
-
-define dso_local noundef i32 @main() local_unnamed_addr !dbg !20 {
-entry:
-  ret i32 42, !dbg !21
-}
-
-!llvm.dbg.cu = !{!0, !2}
-!llvm.module.flags = !{!13, !14}
-!llvm.ident = !{!19, !19}
-
-!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 15.0.0", isOptimized: true, runtimeVersion: 0, splitDebugFilename: "test.dwo", emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: GNU)
-!1 = !DIFile(filename: "test.cpp", directory: "/", checksumkind: CSK_MD5, checksum: "e7c2808ee27614e496499d55e4b37962")
-!2 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !3, producer: "clang version 15.0.0", isOptimized: true, runtimeVersion: 0, splitDebugFilename: "cu1.dwo", emissionKind: FullDebug, splitDebugInlining: false, nameTableKind: GNU)
-!3 = !DIFile(filename: "cu1.cpp", directory: "/", checksumkind: CSK_MD5, checksum: "c0b84240ef5682b87083b33cf9038171")
-!4 = !{!5}
-!5 = !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !6, entity: !11, file: !3, line: 5)
-!6 = distinct !DISubprogram(name: "foo", linkageName: "_Z3foov", scope: !3, file: !3, line: 5, type: !7, scopeLine: 5, flags: DIFlagPrototyped | DIFlagAllCallsDescribed, spFlags: DISPFlagDefinition | DISPFlagOptimized, unit: !2, retainedNodes: !4)
-!7 = !DISubroutineType(types: !8)
-!8 = !{!9}
-!9 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
-!10 = !{}
-!11 = distinct !DIGlobalVariable(name: "A", linkageName: "_ZN2nn1AE", scope: !12, file: !3, line: 2, type: !9, isLocal: false, isDefinition: true)
-!12 = !DINamespace(name: "nn", scope: null)
-!13 = !{i32 7, !"Dwarf Version", i32 5}
-!14 = !{i32 2, !"Debug Info Version", i32 3}
-!19 = !{!"clang version 15.0.0"}
-!20 = distinct !DISubprogram(name: "main", scope: !1, file:

[PATCH] D156042: [clang][Interp] Implement __builtin_strlen

2023-09-05 Thread Timm BĂ€der via Phabricator via cfe-commits 
tbaeder closed this revision.
tbaeder added a comment.

This has been pushed already.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156042

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

[PATCH] D153653: [clang][Interp] Make CXXTemporaryObjectExprs leave a value behind

2023-09-05 Thread Timm BĂ€der via Phabricator via cfe-commits 
tbaeder abandoned this revision.
tbaeder added a comment.

This was pushed as part of the initializer rework.


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

https://reviews.llvm.org/D153653

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

[clang] clang-linker-wrapper/LinkerWrapperOpts.td: "--sysroot" => "--sysroot=" (PR #65313)

2023-09-05 Thread via cfe-commits 

https://github.com/conversy review_requested 
https://github.com/llvm/llvm-project/pull/65313
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] clang-linker-wrapper/LinkerWrapperOpts.td: "--sysroot" => "--sysroot=" (PR #65313)

2023-09-05 Thread via cfe-commits 

https://github.com/conversy created 
https://github.com/llvm/llvm-project/pull/65313:

"--sysroot"
should be 
"--sysroot="

since it's related to
OPT_sysroot_EQ
and not a
OPT_sysroot


>From 972be4aba034d35a10ee0f5e8c82e2be3cdf0d3a Mon Sep 17 00:00:00 2001
From: conversy 
Date: Tue, 5 Sep 2023 13:20:36 +0200
Subject: [PATCH] clang-linker-wrapper/LinkerWrapperOpts.td: "sysroot" should
 be followed with an equal sign

---
 clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td 
b/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
index abab4d0b39b9059..2ce6ee0d21d1d6e 100644
--- a/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
+++ b/clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
@@ -95,7 +95,7 @@ def offload_opt_eq_minus : Joined<["--", "-"], 
"offload-opt=-">, Flags<[HelpHidd
   HelpText<"Options passed to LLVM">;
 
 // Standard linker flags also used by the linker wrapper.
-def sysroot_EQ : Joined<["--"], "sysroot">, HelpText<"Set the system root">;
+def sysroot_EQ : Joined<["--"], "sysroot=">, HelpText<"Set the system root">;
 
 def o : JoinedOrSeparate<["-"], "o">, MetaVarName<"">,
   HelpText<"Path to file to write output">;

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

[clang] 2fbd132 - [Sema] Make C++ functional-style cast warn about dropped qualifiers (-Wcast-qual)

2023-09-05 Thread Kristina Bessonova via cfe-commits 

Author: Kristina Bessonova
Date: 2023-09-05T12:48:38+02:00
New Revision: 2fbd1323e7bf6ec204867774db8aa9b7dc976da5

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

LOG: [Sema] Make C++ functional-style cast warn about dropped qualifiers 
(-Wcast-qual)

Functional-style cast (i.e. a simple-type-speciïŹer or typename-speciïŹer
followed by a parenthesize single expression [expr.type.conv]) is equivalent
to the C-style cast, so that makes sense they have identical behavior
including warnings.

This also matches GCC https://godbolt.org/z/b8Ma9Thjb.

Reviewed By: rnk, aaron.ballman

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaCast.cpp
clang/test/Sema/warn-cast-qual.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 84681ec9554d21..b2b68cc2aeb5f6 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -165,6 +165,7 @@ Improvements to Clang's diagnostics
   (`#64871: `_).
   Also clang no longer emits false positive warnings about the output length of
   ``%g`` format specifier.
+- Clang now emits ``-Wcast-qual`` for functional-style cast expressions.
 
 Bug Fixes in This Version
 -

diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index d4648b2d16eaac..98b5879456e217 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -3374,6 +3374,9 @@ ExprResult 
Sema::BuildCXXFunctionalCastExpr(TypeSourceInfo *CastTypeInfo,
   if (auto *ConstructExpr = dyn_cast(SubExpr))
 ConstructExpr->setParenOrBraceRange(SourceRange(LPLoc, RPLoc));
 
+  // -Wcast-qual
+  DiagnoseCastQual(Op.Self, Op.SrcExpr, Op.DestType);
+
   return Op.complete(CXXFunctionalCastExpr::Create(
   Context, Op.ResultType, Op.ValueKind, CastTypeInfo, Op.Kind,
   Op.SrcExpr.get(), &Op.BasePath, CurFPFeatureOverrides(), LPLoc, RPLoc));

diff  --git a/clang/test/Sema/warn-cast-qual.c 
b/clang/test/Sema/warn-cast-qual.c
index 104f9a3faf47c8..ec140180ffa12f 100644
--- a/clang/test/Sema/warn-cast-qual.c
+++ b/clang/test/Sema/warn-cast-qual.c
@@ -36,6 +36,39 @@ void foo(void) {
   char *charptr = (char *)constcharptr; // expected-warning {{cast from 'const 
char *' to 'char *' drops const qualifier}}
   const char *constcharptr2 = (char *)constcharptr; // expected-warning {{cast 
from 'const char *' to 'char *' drops const qualifier}}
   const char *charptr2 = (char *)charptr; // no warning
+
+#ifdef __cplusplus
+  using CharPtr = char *;
+  using CharPtrPtr = char **;
+  using ConstCharPtrPtr = const char **;
+  using CharPtrConstPtr = char *const *;
+
+  char *fy = CharPtr(ptr); // expected-warning {{cast from 'const char *' 
to 'char *' drops const qualifier}}
+  char **fy1 = CharPtrPtr(ptrptr); // expected-warning {{cast from 'const 
char *const *' to 'char **' drops const qualifier}}
+  const char **fy2 = ConstCharPtrPtr(ptrptr);  // expected-warning {{cast from 
'const char *const *' to 'const char **' drops const qualifier}}
+  char *const *fy3 = CharPtrConstPtr(ptrptr);  // expected-warning {{cast from 
'const char *const' to 'char *const' drops const qualifier}}
+  const char **fy4 = ConstCharPtrPtr(ptrcptr); // expected-warning {{cast from 
'char *const *' to 'const char **' drops const qualifier}}
+
+  using ConstVoidPtr = const void *;
+  char *fz = CharPtr(uintptr_t(ConstVoidPtr(ptr)));// no warning
+  char *fz1 = CharPtr(ConstVoidPtr(ptr));  // expected-warning {{cast from 
'const void *' to 'char *' drops const qualifier}}
+
+  char *fvol2 = CharPtr(vol); // expected-warning {{cast from 'volatile char 
*' to 'char *' drops volatile qualifier}}
+  char *fvolc2 = CharPtr(volc); // expected-warning {{cast from 'const 
volatile char *' to 'char *' drops const and volatile qualifiers}}
+
+  using ConstIntPtrPtr = const int **;
+  using VolitileIntPtrPtr = volatile int **;
+  const int **fintptrptrc = ConstIntPtrPtr(intptrptr); // expected-warning 
{{cast from 'int **' to 'ConstIntPtrPtr' (aka 'const int **') must have all 
intermediate pointers const qualified}}
+  volatile int **fintptrptrv = VolitileIntPtrPtr(intptrptr); // 
expected-warning {{cast from 'int **' to 'VolitileIntPtrPtr' (aka 'volatile int 
**') must have all intermediate pointers const qualified}}
+
+  using ConstIntPtr = const int *;
+  const int *fintptrc = ConstIntPtr(intptr);// no warning
+
+  char **fcharptrptr = CharPtrPtr(charptrptrc); // expected-warning {{cast 
from 'const char *' to 'char *' drops const qualifier}}
+
+  char *fcharptr = CharPtr(constcharptr); // expected-warning {{cast from 
'const char *' to 'char *' drops const qualifier}}
+  const

[PATCH] D159133: [Sema] Make C++ functional-style cast warn about dropped qualifiers (-Wcast-qual)

2023-09-05 Thread Kristina Bessonova via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2fbd1323e7bf: [Sema] Make C++ functional-style cast warn 
about dropped qualifiers (-Wcast
 (authored by krisb).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159133

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaCast.cpp
  clang/test/Sema/warn-cast-qual.c


Index: clang/test/Sema/warn-cast-qual.c
===
--- clang/test/Sema/warn-cast-qual.c
+++ clang/test/Sema/warn-cast-qual.c
@@ -36,6 +36,39 @@
   char *charptr = (char *)constcharptr; // expected-warning {{cast from 'const 
char *' to 'char *' drops const qualifier}}
   const char *constcharptr2 = (char *)constcharptr; // expected-warning {{cast 
from 'const char *' to 'char *' drops const qualifier}}
   const char *charptr2 = (char *)charptr; // no warning
+
+#ifdef __cplusplus
+  using CharPtr = char *;
+  using CharPtrPtr = char **;
+  using ConstCharPtrPtr = const char **;
+  using CharPtrConstPtr = char *const *;
+
+  char *fy = CharPtr(ptr); // expected-warning {{cast from 'const char *' 
to 'char *' drops const qualifier}}
+  char **fy1 = CharPtrPtr(ptrptr); // expected-warning {{cast from 'const 
char *const *' to 'char **' drops const qualifier}}
+  const char **fy2 = ConstCharPtrPtr(ptrptr);  // expected-warning {{cast from 
'const char *const *' to 'const char **' drops const qualifier}}
+  char *const *fy3 = CharPtrConstPtr(ptrptr);  // expected-warning {{cast from 
'const char *const' to 'char *const' drops const qualifier}}
+  const char **fy4 = ConstCharPtrPtr(ptrcptr); // expected-warning {{cast from 
'char *const *' to 'const char **' drops const qualifier}}
+
+  using ConstVoidPtr = const void *;
+  char *fz = CharPtr(uintptr_t(ConstVoidPtr(ptr)));// no warning
+  char *fz1 = CharPtr(ConstVoidPtr(ptr));  // expected-warning {{cast from 
'const void *' to 'char *' drops const qualifier}}
+
+  char *fvol2 = CharPtr(vol); // expected-warning {{cast from 'volatile char 
*' to 'char *' drops volatile qualifier}}
+  char *fvolc2 = CharPtr(volc); // expected-warning {{cast from 'const 
volatile char *' to 'char *' drops const and volatile qualifiers}}
+
+  using ConstIntPtrPtr = const int **;
+  using VolitileIntPtrPtr = volatile int **;
+  const int **fintptrptrc = ConstIntPtrPtr(intptrptr); // expected-warning 
{{cast from 'int **' to 'ConstIntPtrPtr' (aka 'const int **') must have all 
intermediate pointers const qualified}}
+  volatile int **fintptrptrv = VolitileIntPtrPtr(intptrptr); // 
expected-warning {{cast from 'int **' to 'VolitileIntPtrPtr' (aka 'volatile int 
**') must have all intermediate pointers const qualified}}
+
+  using ConstIntPtr = const int *;
+  const int *fintptrc = ConstIntPtr(intptr);// no warning
+
+  char **fcharptrptr = CharPtrPtr(charptrptrc); // expected-warning {{cast 
from 'const char *' to 'char *' drops const qualifier}}
+
+  char *fcharptr = CharPtr(constcharptr); // expected-warning {{cast from 
'const char *' to 'char *' drops const qualifier}}
+  const char *fcharptr2 = CharPtr(charptr); // no warning
+#endif
 }
 
 void bar_0(void) {
@@ -48,6 +81,12 @@
 
   *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' 
drops const qualifier}}
   *(int *)(&S.b) = 0; // expected-warning {{cast from 'const int *' to 'int *' 
drops const qualifier}}
+
+#ifdef __cplusplus
+  using IntPtr = int *;
+  *(IntPtr(&S.a)) = 0; // expected-warning {{cast from 'const int *' to 'int 
*' drops const qualifier}}
+  *(IntPtr(&S.b)) = 0; // expected-warning {{cast from 'const int *' to 'int 
*' drops const qualifier}}
+#endif
 }
 
 void bar_1(void) {
@@ -61,4 +100,10 @@
 
   *(int *)(&S.a) = 0; // expected-warning {{cast from 'const int *' to 'int *' 
drops const qualifier}}
   *(int *)(&S.b) = 0; // no warning
+
+#ifdef __cplusplus
+  using IntPtr = int *;
+  *(IntPtr(&S.a)) = 0; // expected-warning {{cast from 'const int *' to 'int 
*' drops const qualifier}}
+  *(IntPtr(&S.b)) = 0; // no warning
+#endif
 }
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -3374,6 +3374,9 @@
   if (auto *ConstructExpr = dyn_cast(SubExpr))
 ConstructExpr->setParenOrBraceRange(SourceRange(LPLoc, RPLoc));
 
+  // -Wcast-qual
+  DiagnoseCastQual(Op.Self, Op.SrcExpr, Op.DestType);
+
   return Op.complete(CXXFunctionalCastExpr::Create(
   Context, Op.ResultType, Op.ValueKind, CastTypeInfo, Op.Kind,
   Op.SrcExpr.get(), &Op.BasePath, CurFPFeatureOverrides(), LPLoc, RPLoc));
Index: clang/docs/ReleaseNotes.rst
===
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -165,6 +165,7 @@
   (`#64871: `_).
   Also

[PATCH] D155627: [clang][Interp] Handle SourceLocExprs

2023-09-05 Thread Corentin Jabot via Phabricator via cfe-commits 
cor3ntin added a comment.

So you can enable some of the sema source location tests?


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

https://reviews.llvm.org/D155627

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

[PATCH] D158156: [analyzer] Add C++ array delete checker

2023-09-05 Thread DonĂĄt Nagy via Phabricator via cfe-commits 
donat.nagy added inline comments.



Comment at: clang/docs/analyzer/checkers.rst:1793-1803
+.. code-block:: cpp
+
+ Base *create() {
+   Base *x = new Derived[10]; // note: conversion from derived to base
+  //   happened here
+   return x;
+ }

steakhal wrote:
> Discookie wrote:
> > steakhal wrote:
> > > Make sure in the example the `create` is related (e.g. called/used).
> > > Also, refrain from using `sink` in the docs. It's usually used in the 
> > > context of taint analysis.
> > Changed the example - should I change the DeleteWithNonVirtualDtor example 
> > as well? That has the same issues as you said here.
> I have no opinion. You could.
I think you should definitely update it, for the sake of consistency and just 
improving things whenever we can. This commit already touches the code of that 
checker, there is no point in leaving bad documentation behind us...


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

https://reviews.llvm.org/D158156

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

[clang] [clang][MSExtentions] Fix invalid overload failure about the loss of `__unaligned` qualifier (PR #65248)

2023-09-05 Thread via cfe-commits 

https://github.com/cor3ntin approved this pull request.

LGTM, thanks

https://github.com/llvm/llvm-project/pull/65248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][MSExtentions] Fix invalid overload failure about the loss of `__unaligned` qualifier (PR #65248)

2023-09-05 Thread via cfe-commits 

https://github.com/cor3ntin approved this pull request.


https://github.com/llvm/llvm-project/pull/65248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155627: [clang][Interp] Handle SourceLocExprs

2023-09-05 Thread Timm BĂ€der via Phabricator via cfe-commits 
tbaeder added a comment.

I do in https://reviews.llvm.org/D156045, but that depends on other commits 
(not sure which exactly right now though).


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

https://reviews.llvm.org/D155627

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

[PATCH] D159414: Haiku: Fixes for header / library paths

2023-09-05 Thread Brad Smith via Phabricator via cfe-commits 
brad added inline comments.



Comment at: clang/lib/Driver/ToolChains/Haiku.cpp:24-25
 
+  getFilePaths().push_back(concat(getDriver().SysRoot, "/boot/system/lib"));
+  getFilePaths().push_back(concat(getDriver().SysRoot, 
"/boot/system/develop/lib"));
 }

trungnt2910 wrote:
> Should we also add the corresponding non-packaged paths, like 
> `/boot/system/non-packaged/lib` and `/boot/system/non-packaged/develop/lib`?
> 
> The latest GCC toolchain on Haiku does **not** do it, but neither does it 
> recognize non-packaged files for the headers.
> Should we also add the corresponding non-packaged paths, like 
> `/boot/system/non-packaged/lib` and `/boot/system/non-packaged/develop/lib`?
> 
> The latest GCC toolchain on Haiku does **not** do it, but neither does it 
> recognize non-packaged files for the headers.

The second path does not exist and the first path has some sub-dirs (Perl / 
Python) but does not have any libraries.

I'm using what the LLVM patches had. I think we're good as is as a start.



Comment at: clang/lib/Driver/ToolChains/Haiku.cpp:24-25
 
+  getFilePaths().push_back(concat(getDriver().SysRoot, "/boot/system/lib"));
+  getFilePaths().push_back(concat(getDriver().SysRoot, 
"/boot/system/develop/lib"));
 }

brad wrote:
> trungnt2910 wrote:
> > Should we also add the corresponding non-packaged paths, like 
> > `/boot/system/non-packaged/lib` and `/boot/system/non-packaged/develop/lib`?
> > 
> > The latest GCC toolchain on Haiku does **not** do it, but neither does it 
> > recognize non-packaged files for the headers.
> > Should we also add the corresponding non-packaged paths, like 
> > `/boot/system/non-packaged/lib` and `/boot/system/non-packaged/develop/lib`?
> > 
> > The latest GCC toolchain on Haiku does **not** do it, but neither does it 
> > recognize non-packaged files for the headers.
> 
> The second path does not exist and the first path has some sub-dirs (Perl / 
> Python) but does not have any libraries.
> 
> I'm using what the LLVM patches had. I think we're good as is as a start.
> Should we also add the corresponding non-packaged paths, like 
> `/boot/system/non-packaged/lib` and `/boot/system/non-packaged/develop/lib`?
> 
> The latest GCC toolchain on Haiku does **not** do it, but neither does it 
> recognize non-packaged files for the headers.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159414

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

[PATCH] D159263: [clang-tidy] misc-include-cleaner: avoid duplicated fixes

2023-09-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet added a comment.

Thanks a lot for the patch @danix800 !

I initially was rather focused on behavior of this check in workflows that 
require seeing "self-contained-diags", but also I rather see the bulk-apply use 
cases as always requiring clang-format afterwards. Hence didn't really try to 
polish that use case a lot, but I believe changes in this patch improve those 
use cases reasonably. But users do still need to run clang-format afterwards, 
e.g. if we first generate a finding that inserts "b.h" and then "a.h", they'll 
be in wrong order. So this is only fixing  some cases. If you'd like to work on 
a more complete approach, we can prepare all the edits in a single 
`tooling::Replacements` and run `clang::format::cleanupAroundReplacements` on 
top of it to generate proper edits, and emit FixIts with those merged 
replacements. Note that this still won't be enough in all cases, as there can 
be other checks that emit fixes that are touching includes :/

Regarding usage of `IncludeInserter`; we were deliberately not using 
`IncludeInserter` here, as it has slightly different semantics to 
`HeaderIncludes`, which is used by all the other applications of 
include-cleaner when producing edits. That way it's easier for us to reason 
about the behavior in various places (and also to fix them). But moreover, 
`HeaderIncludes` uses clang-format config to figure out include-styles and 
works with a bigger set of projects without requiring extra configurations 
(hence this patch will actually be a regression for those). Therefore can you 
keep using `HeaderIncludes`, while skipping generation of duplicate fixits when 
we're in non-self-contained-diags mode (assuming you don't want to generalize 
the approach as I explained above).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159263

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

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

2023-09-05 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, I like this as a simplification, too!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159247

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

[PATCH] D152279: [Driver] Default -msmall-data-limit= to 0

2023-09-05 Thread Alex Bradbury via Phabricator via cfe-commits 
asb added a comment.

In D152279#4612099 , @craig.topper 
wrote:

> In D152279#4612087 , @MaskRay wrote:
>
>> I am still interested in moving this forward. What should be done here? If 
>> the decision is to keep the current odd default 8 for 
>> `toolchains::RISCVToolChain`, I guess I'll have to take the compromise as 
>> making a step forward is better than nothing.
>
> On 1 RV64 CPU I tried in our RTL simulator, changing from 8 to 0 reduced 
> dhrystone score by 2.7%. Using 16, or 32 gave the same score as 8. Reducing 8 
> to 4 improved the score by 0.5%.

Thanks for testing - do you have a view one way or another on this default? I'm 
somewhat torn personally.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152279

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

[clang] [clang][MSExtentions] Fix invalid overload failure about the loss of `__unaligned` qualifier (PR #65248)

2023-09-05 Thread Aaron Ballman via cfe-commits 

https://github.com/AaronBallman approved this pull request.

LGTM; I think this can be held out of 17.0.0 but would be fine to backport to a 
later 17.x release if desired.

https://github.com/llvm/llvm-project/pull/65248
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme created 
https://github.com/llvm/llvm-project/pull/65319:

Now that prvalue expressions map directly to values (see
https://reviews.llvm.org/D158977), it's no longer guaranteed that `RecordValue`s
associated with the same expression will always have the same storage location.

In other words, D158977 invalidated the assertion in `mergeDistinctValues()`.
The newly added test causes this assertion to fail without the other changes in
the patch.

This patch fixes the issue. However, the real fix will be to eliminate the
`StorageLocation` from `RecordValue` entirely.


>From 5c5723da55fb26204c7ca9ed170c4703d3f3927b Mon Sep 17 00:00:00 2001
From: Martin Braenne 
Date: Tue, 5 Sep 2023 12:32:05 +
Subject: [PATCH] [clang][dataflow] Merge `RecordValue`s with different
 locations correctly.

Now that prvalue expressions map directly to values (see
https://reviews.llvm.org/D158977), it's no longer guaranteed that `RecordValue`s
associated with the same expression will always have the same storage location.

In other words, D158977 invalidated the assertion in `mergeDistinctValues()`.
The newly added test causes this assertion to fail without the other changes in
the patch.

This patch fixes the issue. However, the real fix will be to eliminate the
`StorageLocation` from `RecordValue` entirely.
---
 .../FlowSensitive/DataflowEnvironment.cpp | 24 +++
 .../FlowSensitive/DataflowEnvironmentTest.cpp | 69 +++
 2 files changed, 81 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index b40fbbc991c8f8e..7ce7ee7e579eec9 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -121,18 +121,18 @@ static Value *mergeDistinctValues(QualType Type, Value 
&Val1,
 
   Value *MergedVal = nullptr;
   if (auto *RecordVal1 = dyn_cast(&Val1)) {
-[[maybe_unused]] auto *RecordVal2 = cast(&Val2);
-
-// Values to be merged are always associated with the same location in
-// `LocToVal`. The location stored in `RecordVal` should therefore also
-// be the same.
-assert(&RecordVal1->getLoc() == &RecordVal2->getLoc());
-
-// `RecordVal1` and `RecordVal2` may have different properties associated
-// with them. Create a new `RecordValue` without any properties so that we
-// soundly approximate both values. If a particular analysis needs to merge
-// properties, it should do so in `DataflowAnalysis::merge()`.
-MergedVal = &MergedEnv.create(RecordVal1->getLoc());
+auto *RecordVal2 = cast(&Val2);
+
+if (&RecordVal1->getLoc() == &RecordVal2->getLoc())
+  // `RecordVal1` and `RecordVal2` may have different properties associated
+  // with them. Create a new `RecordValue` without any properties so that 
we
+  // soundly approximate both values. If a particular analysis needs to
+  // merge properties, it should do so in `DataflowAnalysis::merge()`.
+  MergedVal = &MergedEnv.create(RecordVal1->getLoc());
+else
+  // If the locations for the two records are different, need to create a
+  // completely new value.
+  MergedVal = MergedEnv.createValue(Type);
   } else {
 MergedVal = MergedEnv.createValue(Type);
   }
diff --git a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
index a318d9ab7391aa1..6e37011a052c5e4 100644
--- a/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -96,6 +96,75 @@ TEST_F(EnvironmentTest, CreateValueRecursiveType) {
   EXPECT_THAT(PV, NotNull());
 }
 
+TEST_F(EnvironmentTest, JoinRecords) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+struct S {};
+// Need to use the type somewhere so that the `QualType` gets created;
+S s;
+  )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(qualType(hasDeclaration(recordDecl(hasName("S".bind("SType"),
+Context);
+  const QualType *TyPtr = selectFirst("SType", Results);
+  ASSERT_THAT(TyPtr, NotNull());
+  QualType Ty = *TyPtr;
+  ASSERT_FALSE(Ty.isNull());
+
+  auto *ConstructExpr = CXXConstructExpr::CreateEmpty(Context, 0);
+  ConstructExpr->setType(Ty);
+  ConstructExpr->setValueKind(VK_PRValue);
+
+  // Two different `RecordValue`s with the same location are joined into a
+  // third `RecordValue` with that same location.
+  {
+Environment Env1(DAContext);
+auto &Val1 = *cast(Env1.createValue(Ty));
+RecordStorageLocation &Loc = Val1.getLoc();
+Env1.setValue(*ConstructExpr, Val1);
+
+Environment Env2(DAContext);
+

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme review_requested 
https://github.com/llvm/llvm-project/pull/65319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme review_requested 
https://github.com/llvm/llvm-project/pull/65319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme review_requested 
https://github.com/llvm/llvm-project/pull/65319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme review_requested 
https://github.com/llvm/llvm-project/pull/65319
___
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.

2023-09-05 Thread Budimir Aranđelović via Phabricator via cfe-commits 
budimirarandjelovicsyrmia added a comment.

@chestnykh Are you still working on this issue? If not, I'm interested to 
continue working on issue.


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] D159435: [NFC] remove unneded header includes

2023-09-05 Thread Aaron Ballman via Phabricator via cfe-commits 
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159435

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

[clang] [clang][doc] Don't escape _ in .rst files. (PR #65277)

2023-09-05 Thread Aaron Ballman via cfe-commits 

https://github.com/AaronBallman approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/65277
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow][NFC] Remove stale comment. (PR #65322)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme created 
https://github.com/llvm/llvm-project/pull/65322:

None

>From a92d3cf331714c4d1e83073f8674feb2ef419d66 Mon Sep 17 00:00:00 2001
From: Martin Braenne 
Date: Tue, 5 Sep 2023 12:53:04 +
Subject: [PATCH] [clang][dataflow][NFC] Remove stale comment.

---
 clang/unittests/Analysis/FlowSensitive/TestingSupport.h | 4 
 1 file changed, 4 deletions(-)

diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h 
b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index 44dbf27a745867..c61e9f26beff40 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -465,10 +465,6 @@ inline Value *getFieldValue(const RecordStorageLocation 
*Loc,
 
 /// Returns the value of a `Field` on a `Struct.
 /// Returns null if `Struct` is null.
-///
-/// Note: This function currently does not use the `Env` parameter, but it will
-/// soon be needed to look up the `Value` when `setChild()` changes to return a
-/// `StorageLocation *`.
 inline Value *getFieldValue(const RecordValue *Struct, const ValueDecl &Field,
 const Environment &Env) {
   if (Struct == nullptr)

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

[clang] [clang][dataflow][NFC] Remove stale comment. (PR #65322)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme review_requested 
https://github.com/llvm/llvm-project/pull/65322
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow][NFC] Remove stale comment. (PR #65322)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme review_requested 
https://github.com/llvm/llvm-project/pull/65322
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [APINotes] Support `SwiftImportAs` for C++ structs (PR #65323)

2023-09-05 Thread Egor Zhdan via cfe-commits 

https://github.com/egorzhdan review_requested 
https://github.com/llvm/llvm-project/pull/65323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [APINotes] Support `SwiftImportAs` for C++ structs (PR #65323)

2023-09-05 Thread Egor Zhdan via cfe-commits 

https://github.com/egorzhdan review_requested 
https://github.com/llvm/llvm-project/pull/65323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [APINotes] Support `SwiftImportAs` for C++ structs (PR #65323)

2023-09-05 Thread Egor Zhdan via cfe-commits 

https://github.com/egorzhdan created 
https://github.com/llvm/llvm-project/pull/65323:

This upstreams a few Clang API Notes attributes that were recently added 
downstream in the Apple fork (https://github.com/apple/llvm-project/pull/7386).

>From c0f45f6dcb2fc142df4f2f0ea30c03881b2126d7 Mon Sep 17 00:00:00 2001
From: Egor Zhdan 
Date: Tue, 5 Sep 2023 14:02:04 +0100
Subject: [PATCH] [APINotes] Support `SwiftImportAs` for C++ structs

This upstreams a few Clang API Notes attributes that were recently added 
downstream in the Apple fork (https://github.com/apple/llvm-project/pull/7386).
---
 clang/include/clang/APINotes/Types.h| 14 
 clang/lib/APINotes/APINotesFormat.h |  2 +-
 clang/lib/APINotes/APINotesWriter.cpp   | 24 -
 clang/lib/APINotes/APINotesYAMLCompiler.cpp |  6 ++
 4 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/APINotes/Types.h 
b/clang/include/clang/APINotes/Types.h
index 79c8079191fef3b..354458588e30932 100644
--- a/clang/include/clang/APINotes/Types.h
+++ b/clang/include/clang/APINotes/Types.h
@@ -660,6 +660,10 @@ class TagInfo : public CommonTypeInfo {
   unsigned IsFlagEnum : 1;
 
 public:
+  std::optional SwiftImportAs;
+  std::optional SwiftRetainOp;
+  std::optional SwiftReleaseOp;
+
   std::optional EnumExtensibility;
 
   TagInfo() : HasFlagEnum(0), IsFlagEnum(0) {}
@@ -677,6 +681,13 @@ class TagInfo : public CommonTypeInfo {
   TagInfo &operator|=(const TagInfo &RHS) {
 static_cast(*this) |= RHS;
 
+if (!SwiftImportAs)
+  SwiftImportAs = RHS.SwiftImportAs;
+if (!SwiftRetainOp)
+  SwiftRetainOp = RHS.SwiftRetainOp;
+if (!SwiftReleaseOp)
+  SwiftReleaseOp = RHS.SwiftReleaseOp;
+
 if (!HasFlagEnum)
   setFlagEnum(RHS.isFlagEnum());
 
@@ -693,6 +704,9 @@ class TagInfo : public CommonTypeInfo {
 
 inline bool operator==(const TagInfo &LHS, const TagInfo &RHS) {
   return static_cast(LHS) == RHS &&
+ LHS.SwiftImportAs == RHS.SwiftImportAs &&
+ LHS.SwiftRetainOp == RHS.SwiftRetainOp &&
+ LHS.SwiftReleaseOp == RHS.SwiftReleaseOp &&
  LHS.isFlagEnum() == RHS.isFlagEnum() &&
  LHS.EnumExtensibility == RHS.EnumExtensibility;
 }
diff --git a/clang/lib/APINotes/APINotesFormat.h 
b/clang/lib/APINotes/APINotesFormat.h
index b52f017901dbcab..5897b45d3796d0e 100644
--- a/clang/lib/APINotes/APINotesFormat.h
+++ b/clang/lib/APINotes/APINotesFormat.h
@@ -24,7 +24,7 @@ const uint16_t VERSION_MAJOR = 0;
 /// API notes file minor version number.
 ///
 /// When the format changes IN ANY WAY, this number should be incremented.
-const uint16_t VERSION_MINOR = 24; // EnumExtensibility + FlagEnum
+const uint16_t VERSION_MINOR = 25; // SwiftImportAs
 
 using IdentifierID = llvm::PointerEmbeddedInt;
 using IdentifierIDField = llvm::BCVBR<16>;
diff --git a/clang/lib/APINotes/APINotesWriter.cpp 
b/clang/lib/APINotes/APINotesWriter.cpp
index 0126c5ec0798dc5..a92b379a8e56acb 100644
--- a/clang/lib/APINotes/APINotesWriter.cpp
+++ b/clang/lib/APINotes/APINotesWriter.cpp
@@ -1124,7 +1124,10 @@ class CommonTypeTableInfo
 class TagTableInfo : public CommonTypeTableInfo {
 public:
   unsigned getUnversionedInfoSize(const TagInfo &TI) {
-return 1 + getCommonTypeInfoSize(TI);
+return 2 + (TI.SwiftImportAs ? TI.SwiftImportAs->size() : 0) +
+   2 + (TI.SwiftRetainOp ? TI.SwiftRetainOp->size() : 0) +
+   2 + (TI.SwiftReleaseOp ? TI.SwiftReleaseOp->size() : 0) +
+   1 + getCommonTypeInfoSize(TI);
   }
 
   void emitUnversionedInfo(raw_ostream &OS, const TagInfo &TI) {
@@ -1142,6 +1145,25 @@ class TagTableInfo : public 
CommonTypeTableInfo {
 
 writer.write(Flags);
 
+if (auto ImportAs = TI.SwiftImportAs) {
+  writer.write(ImportAs->size() + 1);
+  OS.write(ImportAs->c_str(), ImportAs->size());
+} else {
+  writer.write(0);
+}
+if (auto RetainOp = TI.SwiftRetainOp) {
+  writer.write(RetainOp->size() + 1);
+  OS.write(RetainOp->c_str(), RetainOp->size());
+} else {
+  writer.write(0);
+}
+if (auto ReleaseOp = TI.SwiftReleaseOp) {
+  writer.write(ReleaseOp->size() + 1);
+  OS.write(ReleaseOp->c_str(), ReleaseOp->size());
+} else {
+  writer.write(0);
+}
+
 emitCommonTypeInfo(OS, TI);
   }
 };
diff --git a/clang/lib/APINotes/APINotesYAMLCompiler.cpp 
b/clang/lib/APINotes/APINotesYAMLCompiler.cpp
index e430956e28d0808..647455111214c59 100644
--- a/clang/lib/APINotes/APINotesYAMLCompiler.cpp
+++ b/clang/lib/APINotes/APINotesYAMLCompiler.cpp
@@ -410,6 +410,9 @@ struct Tag {
   std::optional SwiftPrivate;
   std::optional SwiftBridge;
   std::optional NSErrorDomain;
+  std::optional SwiftImportAs;
+  std::optional SwiftRetainOp;
+  std::optional SwiftReleaseOp;
   std::optional EnumExtensibility;
   std::optional FlagEnum;
   std::optional EnumConvenienceKind;
@@ -440,6 +443,9 @@ template <> struct MappingTraits {
 IO.mapOptional("SwiftName", T.SwiftName, S

[clang] [APINotes] Support `SwiftImportAs` for C++ structs (PR #65323)

2023-09-05 Thread Egor Zhdan via cfe-commits 

https://github.com/egorzhdan labeled 
https://github.com/llvm/llvm-project/pull/65323
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D159263: [clang-tidy] misc-include-cleaner: avoid duplicated fixes

2023-09-05 Thread Ding Fei via Phabricator via cfe-commits 
danix800 added a comment.

In D159263#4638167 , @kadircet wrote:

> Thanks a lot for the patch @danix800 !
>
> I initially was rather focused on behavior of this check in workflows that 
> require seeing "self-contained-diags", but also I rather see the bulk-apply 
> use cases as always requiring clang-format afterwards. Hence didn't really 
> try to polish that use case a lot, but I believe changes in this patch 
> improve those use cases reasonably. But users do still need to run 
> clang-format afterwards, e.g. if we first generate a finding that inserts 
> "b.h" and then "a.h", they'll be in wrong order. So this is only fixing  some 
> cases. If you'd like to work on a more complete approach, we can prepare all 
> the edits in a single `tooling::Replacements` and run 
> `clang::format::cleanupAroundReplacements` on top of it to generate proper 
> edits, and emit FixIts with those merged replacements. Note that this still 
> won't be enough in all cases, as there can be other checks that emit fixes 
> that are touching includes :/
>
> Regarding usage of `IncludeInserter`; we were deliberately not using 
> `IncludeInserter` here, as it has slightly different semantics to 
> `HeaderIncludes`, which is used by all the other applications of 
> include-cleaner when producing edits. That way it's easier for us to reason 
> about the behavior in various places (and also to fix them). But moreover, 
> `HeaderIncludes` uses clang-format config to figure out include-styles and 
> works with a bigger set of projects without requiring extra configurations 
> (hence this patch will actually be a regression for those). Therefore can you 
> keep using `HeaderIncludes`, while skipping generation of duplicate fixits 
> when we're in non-self-contained-diags mode (assuming you don't want to 
> generalize the approach as I explained above).

Thanks for the background!

If we have further plans from upstream on these checker/clang-format related 
logic, I can wait and abandon this revision.

Or if such issue does need fixing then coud you take a look at the previous 
diff 1 
which uses an internal set to prevent this issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159263

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

[clang] [AST] Fix nested name specifiers printing as NamespaceNamespace (PR #65266)

2023-09-05 Thread Aaron Ballman via cfe-commits 

https://github.com/AaronBallman approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/65266
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

2023-09-05 Thread dewen via Phabricator via cfe-commits 
dewen added inline comments.



Comment at: clang/test/CXX/class.derived/class.member.lookup/p11.cpp:15
+struct D: I1, I2, B2 {
+  using B1::f;
+  using B2::f;

```
bool correctness{true};
struct A {
bool operator==(A const&oth) const {
return true;
}
};

struct B {
bool operator==(B const&oth) const {
return false;
}
};

struct C {
bool operator==(C const&oth) const {
correctness=false;
return false;
}
};

typedef std::tuple tuple_t ;

int test_it (void) {
tuple_t x,y;
return ( !(x==y) ) && correctness ;
}
```
This test case does not report an error in GCC11 and later versions. The tuple 
class overloads the operator==() method. Different structs also overload the 
operator==() method. Due to the introduction of the patch, the llvm18 reports 
an error when compiling the test case. Is this reasonable?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155387

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

[PATCH] D141714: Fix ast print of variables with attributes

2023-09-05 Thread Erich Keane via Phabricator via cfe-commits 
erichkeane accepted this revision.
erichkeane added a comment.
This revision is now accepted and ready to land.

Looks fine to me, please don't commit for a day or two to give @aaron.ballman a 
chance to make a final comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141714

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

[PATCH] D159263: [clang-tidy] misc-include-cleaner: avoid duplicated fixes

2023-09-05 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet added a comment.

In D159263#4638199 , @danix800 wrote:

> If we have further plans from upstream on these checker/clang-format related 
> logic, I can wait and abandon this revision.

Not at the moment, so please don't!

> Or if such issue does need fixing then coud you take a look at the previous 
> diff 1 
> which uses an internal set to prevent this issue?

Yes, I think that approach makes sense. But we should make it conditional on 
`areDiagsSelfContained` (basically de-duplicate only when diags are not self 
contained).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159263

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

[clang] [NVPTX] Add a warning that device debug info does not work with optimizations (PR #65327)

2023-09-05 Thread Georgi Mirazchiyski via cfe-commits 

https://github.com/GeorgeWeb created 
https://github.com/llvm/llvm-project/pull/65327:

Passing `-g` to `ptxas` with any optimizations enabled is not allowed because, 
`ptxas` does not support optimized debugging.

>From 0efb5b6f3042224cf89978f607d5ff9cd1665351 Mon Sep 17 00:00:00 2001
From: Georgi Mirazchiyski 
Date: Tue, 5 Sep 2023 14:21:24 +0100
Subject: [PATCH] [NVPTX] Add a warning that device debug info does not work
 with optimizations

Passing -g to ptxas with any optimizations enabled is not allowed because,
ptxas does not support optimized debugging.
---
 clang/lib/Driver/ToolChains/Cuda.cpp | 14 +-
 clang/test/Driver/cuda-external-tools.cu | 15 +++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Driver/ToolChains/Cuda.cpp 
b/clang/lib/Driver/ToolChains/Cuda.cpp
index 97472a302715bb..3f76aa49822c26 100644
--- a/clang/lib/Driver/ToolChains/Cuda.cpp
+++ b/clang/lib/Driver/ToolChains/Cuda.cpp
@@ -413,13 +413,25 @@ void NVPTX::Assembler::ConstructJob(Compilation &C, const 
JobAction &JA,
 // TODO: Perhaps we should map host -O2 to ptxas -O3. -O3 is ptxas's
 // default, so it may correspond more closely to the spirit of clang -O2.
 
+bool noOptimization = A->getOption().matches(options::OPT_O0);
+// Emit a driver diagnostic as warning if any -O option different from -O0,
+// is passed since ptxas does not support optimized debugging.
+if (!noOptimization) {
+  auto &Diags = TC.getDriver().getDiags();
+  unsigned DiagID = Diags.getCustomDiagID(
+  DiagnosticsEngine::Warning,
+  "ptxas cannot emit debug info with optimization "
+  "level ('%0') different than O0.");
+  Diags.Report(DiagID) << A->getAsString(Args);
+}
+
 // -O3 seems like the least-bad option when -Osomething is specified to
 // clang but it isn't handled below.
 StringRef OOpt = "3";
 if (A->getOption().matches(options::OPT_O4) ||
 A->getOption().matches(options::OPT_Ofast))
   OOpt = "3";
-else if (A->getOption().matches(options::OPT_O0))
+else if (noOptimization)
   OOpt = "0";
 else if (A->getOption().matches(options::OPT_O)) {
   // -Os, -Oz, and -O(anything else) map to -O2, for lack of better 
options.
diff --git a/clang/test/Driver/cuda-external-tools.cu 
b/clang/test/Driver/cuda-external-tools.cu
index 1aa87cc09982c6..c49ba51b660ef5 100644
--- a/clang/test/Driver/cuda-external-tools.cu
+++ b/clang/test/Driver/cuda-external-tools.cu
@@ -28,6 +28,17 @@
 // RUN:   --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \
 // RUN: | FileCheck -check-prefixes=CHECK,ARCH64,SM35,RDC %s
 
+// Compiling -O{1,2,3,4,fast,s,z} with -g does not pass -g debug info to ptxas.
+// NOTE: This is because ptxas does not support optimized debugging.
+// RUN: %clang -### --target=x86_64-linux-gnu -O3 -g -c %s 2>&1 \
+// RUN:   --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \
+// RUN: | FileCheck -check-prefixes=CHECK,ARCH64,SM35,OPT3-DBG %s
+
+// Compiling -O0 with -g passes -g debug info to ptxas.
+// RUN: %clang -### --target=x86_64-linux-gnu -O0 -g -c %s 2>&1 \
+// RUN:   --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \
+// RUN: | FileCheck -check-prefixes=CHECK,ARCH64,SM35,OPT0-DBG %s
+
 // With debugging enabled, ptxas should be run with with no ptxas 
optimizations.
 // RUN: %clang -### --target=x86_64-linux-gnu --cuda-noopt-device-debug -O2 -g 
-c %s 2>&1 \
 // RUN:   --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \
@@ -134,6 +145,10 @@
 // OPT2-NOT: "-g"
 // OPT3-SAME: "-O3"
 // OPT3-NOT: "-g"
+// OPT3-DBG-SAME: "-O3" "-lineinfo"
+// OPT3-DBG-NOT: "-g"
+// OPT0-DBG-SAME: "-g" "--dont-merge-basicblocks" "--return-at-end"
+// OPT0-DBG-NOT: "-O0"
 // DBG-SAME: "-g" "--dont-merge-basicblocks" "--return-at-end"
 // SM35-SAME: "--gpu-name" "sm_35"
 // SM35-SAME: "--output-file" "[[CUBINFILE:[^"]*]]"

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

[clang] [NVPTX] Add a warning that device debug info does not work with optimizations (PR #65327)

2023-09-05 Thread Georgi Mirazchiyski via cfe-commits 

https://github.com/GeorgeWeb review_requested 
https://github.com/llvm/llvm-project/pull/65327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NVPTX] Add a warning that device debug info does not work with optimizations (PR #65327)

2023-09-05 Thread via cfe-commits 

https://github.com/github-actions[bot] labeled 
https://github.com/llvm/llvm-project/pull/65327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread Kinuko Yasuda via cfe-commits 

https://github.com/kinu edited https://github.com/llvm/llvm-project/pull/65319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread Kinuko Yasuda via cfe-commits 

https://github.com/kinu commented:

Thanks, this LGTM (confirmed that it makes it go through with one of the real 
code I hit this)

https://github.com/llvm/llvm-project/pull/65319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread Kinuko Yasuda via cfe-commits 


@@ -121,18 +121,18 @@ static Value *mergeDistinctValues(QualType Type, Value 
&Val1,
 
   Value *MergedVal = nullptr;
   if (auto *RecordVal1 = dyn_cast(&Val1)) {
-[[maybe_unused]] auto *RecordVal2 = cast(&Val2);
-
-// Values to be merged are always associated with the same location in
-// `LocToVal`. The location stored in `RecordVal` should therefore also
-// be the same.
-assert(&RecordVal1->getLoc() == &RecordVal2->getLoc());
-
-// `RecordVal1` and `RecordVal2` may have different properties associated
-// with them. Create a new `RecordValue` without any properties so that we
-// soundly approximate both values. If a particular analysis needs to merge
-// properties, it should do so in `DataflowAnalysis::merge()`.
-MergedVal = &MergedEnv.create(RecordVal1->getLoc());
+auto *RecordVal2 = cast(&Val2);
+
+if (&RecordVal1->getLoc() == &RecordVal2->getLoc())
+  // `RecordVal1` and `RecordVal2` may have different properties associated
+  // with them. Create a new `RecordValue` without any properties so that 
we
+  // soundly approximate both values. If a particular analysis needs to
+  // merge properties, it should do so in `DataflowAnalysis::merge()`.

kinu wrote:

'If a particular analysis needs to merge...' 
this part can be out of the if / else because the model's merge may do 
something for the else case too?

https://github.com/llvm/llvm-project/pull/65319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread Kinuko Yasuda via cfe-commits 


@@ -121,18 +121,18 @@ static Value *mergeDistinctValues(QualType Type, Value 
&Val1,
 
   Value *MergedVal = nullptr;
   if (auto *RecordVal1 = dyn_cast(&Val1)) {
-[[maybe_unused]] auto *RecordVal2 = cast(&Val2);
-
-// Values to be merged are always associated with the same location in
-// `LocToVal`. The location stored in `RecordVal` should therefore also
-// be the same.
-assert(&RecordVal1->getLoc() == &RecordVal2->getLoc());
-
-// `RecordVal1` and `RecordVal2` may have different properties associated
-// with them. Create a new `RecordValue` without any properties so that we
-// soundly approximate both values. If a particular analysis needs to merge
-// properties, it should do so in `DataflowAnalysis::merge()`.
-MergedVal = &MergedEnv.create(RecordVal1->getLoc());
+auto *RecordVal2 = cast(&Val2);
+
+if (&RecordVal1->getLoc() == &RecordVal2->getLoc())
+  // `RecordVal1` and `RecordVal2` may have different properties associated
+  // with them. Create a new `RecordValue` without any properties so that 
we
+  // soundly approximate both values. If a particular analysis needs to
+  // merge properties, it should do so in `DataflowAnalysis::merge()`.
+  MergedVal = &MergedEnv.create(RecordVal1->getLoc());
+else
+  // If the locations for the two records are different, need to create a
+  // completely new value.

kinu wrote:

Given that how cryptic when this could happen, it might be helpful to have a 
brief comment that both could happen depending on a subtle order of CFG and 
therefore how merge happens.

https://github.com/llvm/llvm-project/pull/65319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Merge `RecordValue`s with different locations correctly. (PR #65319)

2023-09-05 Thread Kinuko Yasuda via cfe-commits 

https://github.com/kinu edited https://github.com/llvm/llvm-project/pull/65319
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [NVPTX] Add a warning that device debug info does not work with optimizations (PR #65327)

2023-09-05 Thread Georgi Mirazchiyski via cfe-commits 

https://github.com/GeorgeWeb updated 
https://github.com/llvm/llvm-project/pull/65327:

>From 757383fd9a9783c290fd4957e4e7526b854c9920 Mon Sep 17 00:00:00 2001
From: Georgi Mirazchiyski 
Date: Tue, 5 Sep 2023 14:21:24 +0100
Subject: [PATCH] [Driver][NVPTX] Add a warning that device debug info does not
 work with optimizations

Passing -g to ptxas with any optimizations enabled is not allowed because,
ptxas does not support optimized debugging.
---
 clang/lib/Driver/ToolChains/Cuda.cpp | 14 +-
 clang/test/Driver/cuda-external-tools.cu | 15 +++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Driver/ToolChains/Cuda.cpp 
b/clang/lib/Driver/ToolChains/Cuda.cpp
index 97472a302715bb..3f76aa49822c26 100644
--- a/clang/lib/Driver/ToolChains/Cuda.cpp
+++ b/clang/lib/Driver/ToolChains/Cuda.cpp
@@ -413,13 +413,25 @@ void NVPTX::Assembler::ConstructJob(Compilation &C, const 
JobAction &JA,
 // TODO: Perhaps we should map host -O2 to ptxas -O3. -O3 is ptxas's
 // default, so it may correspond more closely to the spirit of clang -O2.
 
+bool noOptimization = A->getOption().matches(options::OPT_O0);
+// Emit a driver diagnostic as warning if any -O option different from -O0,
+// is passed since ptxas does not support optimized debugging.
+if (!noOptimization) {
+  auto &Diags = TC.getDriver().getDiags();
+  unsigned DiagID = Diags.getCustomDiagID(
+  DiagnosticsEngine::Warning,
+  "ptxas cannot emit debug info with optimization "
+  "level ('%0') different than O0.");
+  Diags.Report(DiagID) << A->getAsString(Args);
+}
+
 // -O3 seems like the least-bad option when -Osomething is specified to
 // clang but it isn't handled below.
 StringRef OOpt = "3";
 if (A->getOption().matches(options::OPT_O4) ||
 A->getOption().matches(options::OPT_Ofast))
   OOpt = "3";
-else if (A->getOption().matches(options::OPT_O0))
+else if (noOptimization)
   OOpt = "0";
 else if (A->getOption().matches(options::OPT_O)) {
   // -Os, -Oz, and -O(anything else) map to -O2, for lack of better 
options.
diff --git a/clang/test/Driver/cuda-external-tools.cu 
b/clang/test/Driver/cuda-external-tools.cu
index 1aa87cc09982c6..c49ba51b660ef5 100644
--- a/clang/test/Driver/cuda-external-tools.cu
+++ b/clang/test/Driver/cuda-external-tools.cu
@@ -28,6 +28,17 @@
 // RUN:   --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \
 // RUN: | FileCheck -check-prefixes=CHECK,ARCH64,SM35,RDC %s
 
+// Compiling -O{1,2,3,4,fast,s,z} with -g does not pass -g debug info to ptxas.
+// NOTE: This is because ptxas does not support optimized debugging.
+// RUN: %clang -### --target=x86_64-linux-gnu -O3 -g -c %s 2>&1 \
+// RUN:   --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \
+// RUN: | FileCheck -check-prefixes=CHECK,ARCH64,SM35,OPT3-DBG %s
+
+// Compiling -O0 with -g passes -g debug info to ptxas.
+// RUN: %clang -### --target=x86_64-linux-gnu -O0 -g -c %s 2>&1 \
+// RUN:   --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \
+// RUN: | FileCheck -check-prefixes=CHECK,ARCH64,SM35,OPT0-DBG %s
+
 // With debugging enabled, ptxas should be run with with no ptxas 
optimizations.
 // RUN: %clang -### --target=x86_64-linux-gnu --cuda-noopt-device-debug -O2 -g 
-c %s 2>&1 \
 // RUN:   --offload-arch=sm_35 --cuda-path=%S/Inputs/CUDA/usr/local/cuda \
@@ -134,6 +145,10 @@
 // OPT2-NOT: "-g"
 // OPT3-SAME: "-O3"
 // OPT3-NOT: "-g"
+// OPT3-DBG-SAME: "-O3" "-lineinfo"
+// OPT3-DBG-NOT: "-g"
+// OPT0-DBG-SAME: "-g" "--dont-merge-basicblocks" "--return-at-end"
+// OPT0-DBG-NOT: "-O0"
 // DBG-SAME: "-g" "--dont-merge-basicblocks" "--return-at-end"
 // SM35-SAME: "--gpu-name" "sm_35"
 // SM35-SAME: "--output-file" "[[CUBINFILE:[^"]*]]"

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

[clang] [Driver][NVPTX] Add a warning that device debug info does not work with optimizations (PR #65327)

2023-09-05 Thread Georgi Mirazchiyski via cfe-commits 

https://github.com/GeorgeWeb edited 
https://github.com/llvm/llvm-project/pull/65327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [GlobalISel][AArch64] Combine unmerge(G_EXT v, undef) to unmerge(v). (PR #65263)

2023-09-05 Thread Vladislav Dzhidzhoev via cfe-commits 

https://github.com/dzhidzhoev updated 
https://github.com/llvm/llvm-project/pull/65263:

>From a88ee53623a3ff4774bc7a5df13379e55a0f9945 Mon Sep 17 00:00:00 2001
From: Vladislav Dzhidzhoev 
Date: Mon, 4 Sep 2023 12:35:48 +0200
Subject: [PATCH 1/2] [GlobalISel][AArch64] Combine unmerge(G_EXT v, undef) to
 unmerge(v).

When having  d1, unused = unmerge(G_EXT <2*N x t> v1, undef, N),
it is possible to express it just as unused, d1 = unmerge v1.

It is useful for tackling regressions in arm64-vcvt_f.ll, introduced in
https://reviews.llvm.org/D144670.
---
 llvm/lib/Target/AArch64/AArch64Combine.td | 11 ++-
 .../GISel/AArch64PostLegalizerLowering.cpp| 47 
 .../AArch64/arm64-neon-add-pairwise.ll|  6 +-
 llvm/test/CodeGen/AArch64/arm64-vabs.ll   | 76 +++
 llvm/test/CodeGen/AArch64/arm64-vcvt_f.ll | 17 ++---
 5 files changed, 93 insertions(+), 64 deletions(-)

diff --git a/llvm/lib/Target/AArch64/AArch64Combine.td 
b/llvm/lib/Target/AArch64/AArch64Combine.td
index 6bcb014d22035e..699310d0627dba 100644
--- a/llvm/lib/Target/AArch64/AArch64Combine.td
+++ b/llvm/lib/Target/AArch64/AArch64Combine.td
@@ -206,6 +206,14 @@ def vector_sext_inreg_to_shift : GICombineRule<
   (apply [{ applyVectorSextInReg(*${d}, MRI, B, Observer); }])
 >;
 
+def unmerge_ext_to_unmerge_matchdata : GIDefMatchData<"Register">;
+def unmerge_ext_to_unmerge : GICombineRule<
+  (defs root:$d, unmerge_ext_to_unmerge_matchdata:$matchinfo),
+  (match (wip_match_opcode G_UNMERGE_VALUES):$d,
+  [{ return matchUnmergeExtToUnmerge(*${d}, MRI, ${matchinfo}); }]),
+  (apply [{ applyUnmergeExtToUnmerge(*${d}, MRI, B, Observer, ${matchinfo}); 
}])
+>;
+
 // Post-legalization combines which should happen at all optimization levels.
 // (E.g. ones that facilitate matching for the selector) For example, matching
 // pseudos.
@@ -214,7 +222,8 @@ def AArch64PostLegalizerLowering
[shuffle_vector_lowering, vashr_vlshr_imm,
 icmp_lowering, build_vector_lowering,
 lower_vector_fcmp, form_truncstore,
-vector_sext_inreg_to_shift]> {
+vector_sext_inreg_to_shift,
+unmerge_ext_to_unmerge]> {
 }
 
 // Post-legalization combines which are primarily optimizations.
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp 
b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
index 57117ea143d7e9..e9386d77b2559f 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64PostLegalizerLowering.cpp
@@ -1066,6 +1066,53 @@ void applyVectorSextInReg(MachineInstr &MI, 
MachineRegisterInfo &MRI,
   Helper.lower(MI, 0, /* Unused hint type */ LLT());
 }
 
+/// Combine , unused = unmerge(G_EXT <2*N x t> v, undef, N)
+///   => unused,  = unmerge v
+bool matchUnmergeExtToUnmerge(MachineInstr &MI, MachineRegisterInfo &MRI,
+  Register &MatchInfo) {
+  assert(MI.getOpcode() == TargetOpcode::G_UNMERGE_VALUES);
+  if (MI.getNumDefs() != 2)
+return false;
+  if (!MRI.use_nodbg_empty(MI.getOperand(1).getReg()))
+return false;
+
+  LLT DstTy = MRI.getType(MI.getOperand(0).getReg());
+  if (!DstTy.isVector())
+return false;
+
+  MachineInstr *Ext = getDefIgnoringCopies(
+  MI.getOperand(MI.getNumExplicitDefs()).getReg(), MRI);
+  if (!Ext || Ext->getOpcode() != AArch64::G_EXT)
+return false;
+
+  Register ExtSrc1 = Ext->getOperand(1).getReg();
+  Register ExtSrc2 = Ext->getOperand(2).getReg();
+  auto LowestVal =
+  getIConstantVRegValWithLookThrough(Ext->getOperand(3).getReg(), MRI);
+  if (!LowestVal || LowestVal->Value.getZExtValue() != DstTy.getSizeInBytes())
+return false;
+
+  MachineInstr *Undef = getDefIgnoringCopies(ExtSrc2, MRI);
+  if (!Undef)
+return false;
+
+  MatchInfo = ExtSrc1;
+
+  return Undef->getOpcode() == TargetOpcode::G_IMPLICIT_DEF;
+}
+
+void applyUnmergeExtToUnmerge(MachineInstr &MI, MachineRegisterInfo &MRI,
+  MachineIRBuilder &B,
+  GISelChangeObserver &Observer, Register &SrcReg) 
{
+  Observer.changingInstr(MI);
+  // Swap dst registers.
+  Register Dst1 = MI.getOperand(0).getReg();
+  MI.getOperand(0).setReg(MI.getOperand(1).getReg());
+  MI.getOperand(1).setReg(Dst1);
+  MI.getOperand(2).setReg(SrcReg);
+  Observer.changedInstr(MI);
+}
+
 class AArch64PostLegalizerLoweringImpl : public Combiner {
 protected:
   // TODO: Make CombinerHelper methods const.
diff --git a/llvm/test/CodeGen/AArch64/arm64-neon-add-pairwise.ll 
b/llvm/test/CodeGen/AArch64/arm64-neon-add-pairwise.ll
index aa048eea302c97..17fb312c63754d 100644
--- a/llvm/test/CodeGen/AArch64/arm64-neon-add-pairwise.ll
+++ b/llvm/test/CodeGen/AArch64/arm64-neon-add-pairwise.ll
@@ -137,7 +137,7 @@ define i32 @addp_v4i32(<4 x i32> %a, <4 x i32> %b) {
 ; CHECK-GI-LABEL: addp_v4i32:
 ; CHECK-GI:

[clang] [GlobalISel][AArch64] Combine unmerge(G_EXT v, undef) to unmerge(v). (PR #65263)

2023-09-05 Thread via cfe-commits 

https://github.com/github-actions[bot] labeled 
https://github.com/llvm/llvm-project/pull/65263
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [GlobalISel][AArch64] Combine unmerge(G_EXT v, undef) to unmerge(v). (PR #65263)

2023-09-05 Thread via cfe-commits 

https://github.com/github-actions[bot] labeled 
https://github.com/llvm/llvm-project/pull/65263
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Eliminate uses of `RecordValue::getChild()`. (PR #65329)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme created 
https://github.com/llvm/llvm-project/pull/65329:

We want to work towards eliminating the `RecordStorageLocation` from
`RecordValue`. These particular uses of `RecordValue::getChild()` can simply
be replaced with `RecordStorageLocation::getChild()`.


>From 6f03364c053e70a19fdefdb6dd38ce8a3cd0941b Mon Sep 17 00:00:00 2001
From: Martin Braenne 
Date: Tue, 5 Sep 2023 14:12:38 +
Subject: [PATCH] [clang][dataflow] Eliminate uses of
 `RecordValue::getChild()`.

We want to work towards eliminating the `RecordStorageLocation` from
`RecordValue`. These particular uses of `RecordValue::getChild()` can simply
be replaced with `RecordStorageLocation::getChild()`.
---
 .../unittests/Analysis/FlowSensitive/TransferTest.cpp  | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 0a5cf62e5ea233..ff3618e999f004 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2860,11 +2860,11 @@ TEST(TransferTest, AggregateInitialization) {
 
   // Check that fields initialized in an initializer list are always
   // modeled in other instances of the same type.
-  const auto &OtherBVal =
-  getValueForDecl(ASTCtx, Env, "OtherB");
-  EXPECT_THAT(OtherBVal.getChild(*BarDecl), NotNull());
-  EXPECT_THAT(OtherBVal.getChild(*BazDecl), NotNull());
-  EXPECT_THAT(OtherBVal.getChild(*QuxDecl), NotNull());
+  const auto &OtherBLoc =
+  getLocForDecl(ASTCtx, Env, "OtherB");
+  EXPECT_THAT(OtherBLoc.getChild(*BarDecl), NotNull());
+  EXPECT_THAT(OtherBLoc.getChild(*BazDecl), NotNull());
+  EXPECT_THAT(OtherBLoc.getChild(*QuxDecl), NotNull());
 });
   }
 }

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

[clang] [clang][dataflow] Eliminate uses of `RecordValue::getChild()`. (PR #65329)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme review_requested 
https://github.com/llvm/llvm-project/pull/65329
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Eliminate uses of `RecordValue::getChild()`. (PR #65329)

2023-09-05 Thread via cfe-commits 

https://github.com/github-actions[bot] labeled 
https://github.com/llvm/llvm-project/pull/65329
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Eliminate uses of `RecordValue::getChild()`. (PR #65329)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme review_requested 
https://github.com/llvm/llvm-project/pull/65329
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][dataflow] Eliminate uses of `RecordValue::getChild()`. (PR #65329)

2023-09-05 Thread via cfe-commits 

https://github.com/martinboehme review_requested 
https://github.com/llvm/llvm-project/pull/65329
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix missing diagnostic for non-standard layout type in `offsetof` (PR #65246)

2023-09-05 Thread Aaron Ballman via cfe-commits 

https://github.com/AaronBallman edited 
https://github.com/llvm/llvm-project/pull/65246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix missing diagnostic for non-standard layout type in `offsetof` (PR #65246)

2023-09-05 Thread Aaron Ballman via cfe-commits 


@@ -50,9 +50,9 @@ struct Derived2 : public Base1, public Base2 {
   int z; 
 };
 
-int derived1[__builtin_offsetof(Derived2, x) == 0? 1 : -1];
-int derived2[__builtin_offsetof(Derived2, y)  == 4? 1 : -1];
-int derived3[__builtin_offsetof(Derived2, z)  == 8? 1 : -1];
+int derived1[__builtin_offsetof(Derived2, x) == 0? 1 : -1]; // 
expected-warning{{offset of on non-POD type 'Derived2'}}

AaronBallman wrote:

This also seems like an improvement in diagnostic behavior.

https://github.com/llvm/llvm-project/pull/65246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix missing diagnostic for non-standard layout type in `offsetof` (PR #65246)

2023-09-05 Thread Aaron Ballman via cfe-commits 

https://github.com/AaronBallman approved this pull request.

Hmmm, in some ways I think this is making the diagnostic needlessly chatty, and 
in other ways the new diagnostic behavior seems reasonable.

On balance, I think the improvements outweigh the regressions, especially for 
code users are likely to hit. e.g., I suspect there's way more use of offsetof 
in a static assertion than there are uses of offsetof as an operand to sizeof.

https://github.com/llvm/llvm-project/pull/65246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix missing diagnostic for non-standard layout type in `offsetof` (PR #65246)

2023-09-05 Thread Aaron Ballman via cfe-commits 


@@ -17,7 +17,7 @@ struct Base { int x; };
 struct Derived : Base { int y; };
 int o = __builtin_offsetof(Derived, x); // expected-warning{{offset of on 
non-POD type}}
 
-const int o2 = sizeof(__builtin_offsetof(Derived, x));
+const int o2 = sizeof(__builtin_offsetof(Derived, x)); // 
expected-warning{{offset of on non-POD type 'Derived'}}

AaronBallman wrote:

This is an example of why I think the existing behavior is correct. The fact 
that we're asking for the offset of a field on a non-POD type is not harmful 
because the call is never evaluated.

https://github.com/llvm/llvm-project/pull/65246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix missing diagnostic for non-standard layout type in `offsetof` (PR #65246)

2023-09-05 Thread Aaron Ballman via cfe-commits 


@@ -25,6 +25,7 @@ struct B : public A {
 
 static_assert(__builtin_offsetof(B, d) == 12,
   "We can't allocate the bitfield into the padding under ms_struct");
+// expected-warning@-2 {{offset of on non-standard-layout type 'B'}}

AaronBallman wrote:

I think this diagnostic is an improvement over the status quo.

https://github.com/llvm/llvm-project/pull/65246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix missing diagnostic for non-standard layout type in `offsetof` (PR #65246)

2023-09-05 Thread via cfe-commits 


@@ -17,7 +17,7 @@ struct Base { int x; };
 struct Derived : Base { int y; };
 int o = __builtin_offsetof(Derived, x); // expected-warning{{offset of on 
non-POD type}}
 
-const int o2 = sizeof(__builtin_offsetof(Derived, x));
+const int o2 = sizeof(__builtin_offsetof(Derived, x)); // 
expected-warning{{offset of on non-POD type 'Derived'}}

cor3ntin wrote:

So you think we should ie, check for `!isUnevaluated()` - ie diag in constant 
expressions but not in unevaluated contexts?

https://github.com/llvm/llvm-project/pull/65246
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

  1   2   3   4   >