[Analysis] Migrate away from PointerUnion::dyn_cast (NFC) (#122652)

Note that PointerUnion::dyn_cast has been soft deprecated in
PointerUnion.h:

// FIXME: Replace the uses of is(), get() and dyn_

[Analysis] Migrate away from PointerUnion::dyn_cast (NFC) (#122652)

Note that PointerUnion::dyn_cast has been soft deprecated in
PointerUnion.h:

// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>

Literal migration would result in dyn_cast_if_present (see the
definition of PointerUnion::dyn_cast), but this patch uses dyn_cast
because we expect Ctx->FunArgs to be nonnull.

show more ...

Thread safety analysis: Fix substitution for operator calls (#116487)

For operator calls that go to methods we need to substitute the first
parameter for "this" and the following parameters into th

Thread safety analysis: Fix substitution for operator calls (#116487)

For operator calls that go to methods we need to substitute the first
parameter for "this" and the following parameters into the function
parameters, instead of substituting all of them into the parameters.

This revealed an issue about lambdas. An existing test accidentally
worked because the substitution bug was covered by a speciality of
lambdas: a CXXThisExpr in a lambda CXXMethodDecl does not refer to the
implicit this argument of the method, but to a captured "this" from the
context the lambda was created in. This can happen for operator calls,
where it worked due to the substitution bug (we treated the implicit
this argument incorrectly as parameter), and for regular calls (i.e.
obj.operator()(args) instead of obj(args)), where it didn't work.

The correct fix seems to be to clear the self-argument on a lambda call.
Lambdas can only capture "this" inside methods, and calls to the lambda
in that scope cannot substitute anything for "this".

show more ...

[clang] Migrate away from PointerUnion::{is,get} (NFC) (#119724)

Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:

// FIXME: Replace the uses of is(), get() and dyn

[clang] Migrate away from PointerUnion::{is,get} (NFC) (#119724)

Note that PointerUnion::{is,get} have been soft deprecated in
PointerUnion.h:

// FIXME: Replace the uses of is(), get() and dyn_cast() with
// isa<T>, cast<T> and the llvm::dyn_cast<T>

I'm not touching PointerUnion::dyn_cast for now because it's a bit
complicated; we could blindly migrate it to dyn_cast_if_present, but
we should probably use dyn_cast when the operand is known to be
non-null.

show more ...

[clang][NFC] Add Type::isPointerOrReferenceType() (#101206)

Seems to be a common pattern.

[clang][ThreadSafety] Skip past implicit cast in `translateAttrExpr`

Ignore `ImplicitCastExpr` when building `AttrExp` for capability
attribute diagnostics.

Fixes: https://github.com/llvm/llvm-proj

[clang][ThreadSafety] Skip past implicit cast in `translateAttrExpr`

Ignore `ImplicitCastExpr` when building `AttrExp` for capability
attribute diagnostics.

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

show more ...

[clang] Drop explicit conversions of string literals to StringRef (NFC)

We routinely rely on implicit conversions of string literals to
StringRef so that we can use operator==(StringRef, StringRef).

[clang] Drop explicit conversions of string literals to StringRef (NFC)

We routinely rely on implicit conversions of string literals to
StringRef so that we can use operator==(StringRef, StringRef).

The LHS here are all known to be of StringRef.

show more ...

Thread safety analysis: provide printSCFG definition. (#80277)

I called this function when investigating the issue
(https://github.com/llvm/llvm-project/issues/78131), and I was surprised
to see t

Thread safety analysis: provide printSCFG definition. (#80277)

I called this function when investigating the issue
(https://github.com/llvm/llvm-project/issues/78131), and I was surprised
to see the definition is commented out.

I think it makes sense to provide the definition even though the
implementation is not stable.

show more ...

[C++] Implement "Deducing this" (P0847R7)

This patch implements P0847R7 (partially),
CWG2561 and CWG2653.

Reviewed By: aaron.ballman, #clang-language-wg

Differential Revision: https://reviews.llvm

[C++] Implement "Deducing this" (P0847R7)

This patch implements P0847R7 (partially),
CWG2561 and CWG2653.

Reviewed By: aaron.ballman, #clang-language-wg

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

show more ...

[clang][TSA] Thread safety cleanup functions

Consider cleanup functions in thread safety analysis.

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

[clang] Use DenseMapBase::lookup (NFC)

Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

When support for copy elision was initially added in e97654b2f2807, it
was taking attributes fro

Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

When support for copy elision was initially added in e97654b2f2807, it
was taking attributes from a constructor call, although that constructor
call is actually not involved. It seems more natural to use attributes
on the function returning the scoped capability, which is where it's
actually coming from. This would also support a number of interesting
use cases, like producing different scope kinds without the need for tag
types, or producing scopes from a private mutex.

Changing the behavior was surprisingly difficult: we were not handling
CXXConstructorExpr calls like regular calls but instead handled them
through the DeclStmt they're contained in. This was based on the
assumption that constructors are basically only called in variable
declarations (not true because of temporaries), and that variable
declarations necessitate constructors (not true with C++17 anymore).

Untangling this required separating construction from assigning a
variable name. When a call produces an object, we use a placeholder
til::LiteralPtr for `this`, and we collect the call expression and
placeholder in a map. Later when going through a DeclStmt, we look up
the call expression and set the placeholder to the new VarDecl.

The change has a couple of nice side effects:
* We don't miss constructor calls not contained in DeclStmts anymore,
allowing patterns like
MutexLock{&mu}, requiresMutex();
The scoped lock temporary will be destructed at the end of the full
statement, so it protects the following call without the need for a
scope, but with the ability to unlock in case of an exception.
* We support lifetime extension of temporaries. While unusual, one can
now write
const MutexLock &scope = MutexLock(&mu);
and have it behave as expected.
* Destructors used to be handled in a weird way: since there is no
expression in the AST for implicit destructor calls, we instead
provided a made-up DeclRefExpr to the variable being destructed, and
passed that instead of a CallExpr. Then later in translateAttrExpr
there was special code that knew that destructor expressions worked a
bit different.
* We were producing dummy DeclRefExprs in a number of places, this has
been eliminated. We now use til::SExprs instead.

Technically this could break existing code, but the current handling
seems unexpected enough to justify this change.

Reviewed By: aaron.ballman

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

show more ...

Revert "Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls"

This caused false positives, see comment on the code review.

> When support for copy e

Revert "Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls"

This caused false positives, see comment on the code review.

> When support for copy elision was initially added in e97654b2f2807, it
> was taking attributes from a constructor call, although that constructor
> call is actually not involved. It seems more natural to use attributes
> on the function returning the scoped capability, which is where it's
> actually coming from. This would also support a number of interesting
> use cases, like producing different scope kinds without the need for tag
> types, or producing scopes from a private mutex.
>
> Changing the behavior was surprisingly difficult: we were not handling
> CXXConstructorExpr calls like regular calls but instead handled them
> through the DeclStmt they're contained in. This was based on the
> assumption that constructors are basically only called in variable
> declarations (not true because of temporaries), and that variable
> declarations necessitate constructors (not true with C++17 anymore).
>
> Untangling this required separating construction from assigning a
> variable name. When a call produces an object, we use a placeholder
> til::LiteralPtr for `this`, and we collect the call expression and
> placeholder in a map. Later when going through a DeclStmt, we look up
> the call expression and set the placeholder to the new VarDecl.
>
> The change has a couple of nice side effects:
> * We don't miss constructor calls not contained in DeclStmts anymore,
> allowing patterns like
> MutexLock{&mu}, requiresMutex();
> The scoped lock temporary will be destructed at the end of the full
> statement, so it protects the following call without the need for a
> scope, but with the ability to unlock in case of an exception.
> * We support lifetime extension of temporaries. While unusual, one can
> now write
> const MutexLock &scope = MutexLock(&mu);
> and have it behave as expected.
> * Destructors used to be handled in a weird way: since there is no
> expression in the AST for implicit destructor calls, we instead
> provided a made-up DeclRefExpr to the variable being destructed, and
> passed that instead of a CallExpr. Then later in translateAttrExpr
> there was special code that knew that destructor expressions worked a
> bit different.
> * We were producing dummy DeclRefExprs in a number of places, this has
> been eliminated. We now use til::SExprs instead.
>
> Technically this could break existing code, but the current handling
> seems unexpected enough to justify this change.
>
> Reviewed By: aaron.ballman
>
> Differential Revision: https://reviews.llvm.org/D129755

This reverts commit 0041a69495f828f6732803cfb0f1e3fddd7fbf2a and the follow-up
warning fix in 83d93d3c11ac9727bf3d4c5c956de44233cc7f87.

show more ...

Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

When support for copy elision was initially added in e97654b2f2807, it
was taking attributes fro

Thread safety analysis: Support copy-elided production of scoped capabilities through arbitrary calls

When support for copy elision was initially added in e97654b2f2807, it
was taking attributes from a constructor call, although that constructor
call is actually not involved. It seems more natural to use attributes
on the function returning the scoped capability, which is where it's
actually coming from. This would also support a number of interesting
use cases, like producing different scope kinds without the need for tag
types, or producing scopes from a private mutex.

Changing the behavior was surprisingly difficult: we were not handling
CXXConstructorExpr calls like regular calls but instead handled them
through the DeclStmt they're contained in. This was based on the
assumption that constructors are basically only called in variable
declarations (not true because of temporaries), and that variable
declarations necessitate constructors (not true with C++17 anymore).

Untangling this required separating construction from assigning a
variable name. When a call produces an object, we use a placeholder
til::LiteralPtr for `this`, and we collect the call expression and
placeholder in a map. Later when going through a DeclStmt, we look up
the call expression and set the placeholder to the new VarDecl.

The change has a couple of nice side effects:
* We don't miss constructor calls not contained in DeclStmts anymore,
allowing patterns like
MutexLock{&mu}, requiresMutex();
The scoped lock temporary will be destructed at the end of the full
statement, so it protects the following call without the need for a
scope, but with the ability to unlock in case of an exception.
* We support lifetime extension of temporaries. While unusual, one can
now write
const MutexLock &scope = MutexLock(&mu);
and have it behave as expected.
* Destructors used to be handled in a weird way: since there is no
expression in the AST for implicit destructor calls, we instead
provided a made-up DeclRefExpr to the variable being destructed, and
passed that instead of a CallExpr. Then later in translateAttrExpr
there was special code that knew that destructor expressions worked a
bit different.
* We were producing dummy DeclRefExprs in a number of places, this has
been eliminated. We now use til::SExprs instead.

Technically this could break existing code, but the current handling
seems unexpected enough to justify this change.

Reviewed By: aaron.ballman

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

show more ...

[clang] Qualify auto in range-based for loops (NFC)

Thread safety analysis: Store capability kind in CapabilityExpr

This should make us print the right capability kind in many more cases,
especially when attributes name multiple capabilities of diffe

Thread safety analysis: Store capability kind in CapabilityExpr

This should make us print the right capability kind in many more cases,
especially when attributes name multiple capabilities of different kinds.

Previously we were trying to deduce the capability kind from the
original attribute, but most attributes can name multiple capabilities,
which could be of different kinds. So instead we derive the kind when
translating the attribute expression, and then store it in the returned
CapabilityExpr. Then we can extract the corresponding capability name
when we need it, which saves us lots of plumbing and almost guarantees
that the name is right.

I didn't bother adding any tests for this because it's just a usability
improvement and it's pretty much evident from the code that we don't
fall back to "mutex" anymore (save for a few cases that I'll address in
a separate change).

Reviewed By: aaron.ballman

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

show more ...

[ADT] Remove APInt/APSInt toString() std::string variants

<string> is currently the highest impact header in a clang+llvm build:

https://commondatastorage.googleapis.com/chromium-browser-clang/llvm

[ADT] Remove APInt/APSInt toString() std::string variants

<string> is currently the highest impact header in a clang+llvm build:

https://commondatastorage.googleapis.com/chromium-browser-clang/llvm-include-analysis.html

One of the most common places this is being included is the APInt.h header, which needs it for an old toString() implementation that returns std::string - an inefficient method compared to the SmallString versions that it actually wraps.

This patch replaces these APInt/APSInt methods with a pair of llvm::toString() helpers inside StringExtras.h, adjusts users accordingly and removes the <string> from APInt.h - I was hoping that more of these users could be converted to use the SmallString methods, but it appears that most end up creating a std::string anyhow. I avoided trying to use the raw_ostream << operators as well as I didn't want to lose having the integer radix explicit in the code.

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

show more ...

Thread safety analysis: Nullability improvements in TIL, NFCI

The constructor of Project asserts that the contained ValueDecl is not
null, use that in the ThreadSafetyAnalyzer. In the case of Litera

Thread safety analysis: Nullability improvements in TIL, NFCI

The constructor of Project asserts that the contained ValueDecl is not
null, use that in the ThreadSafetyAnalyzer. In the case of LiteralPtr
it's the other way around.

Also dyn_cast<> is sufficient if we know something isn't null.

show more ...

Temporairly revert "Thread safety analysis: Consider global variables in scope" & followup

This appears to cause false-positives because it started to warn on local non-global variables.

Repro post

Temporairly revert "Thread safety analysis: Consider global variables in scope" & followup

This appears to cause false-positives because it started to warn on local non-global variables.

Repro posted to https://reviews.llvm.org/D84604#2262745

This reverts commit 9dcc82f34ea9b623d82d2577b93aaf67d36dabd2.
This reverts commit b2ce79ef66157dd752e3864ece57915e23a73f5d.

show more ...

Thread safety analysis: ValueDecl in Project is non-null

The constructor asserts that, use it in the ThreadSafetyAnalyzer.
Also note that the result of a cast<> cannot be null.

[NFC] Refactor representation of materialized temporaries

Summary:
this patch refactor representation of materialized temporaries to prevent an issue raised by rsmith in https://reviews.llvm.org/D63

[NFC] Refactor representation of materialized temporaries

Summary:
this patch refactor representation of materialized temporaries to prevent an issue raised by rsmith in https://reviews.llvm.org/D63640#inline-612718

Reviewers: rsmith, martong, shafik

Reviewed By: rsmith

Subscribers: thakis, sammccall, ilya-biryukov, rnkovacs, arphaman, cfe-commits

Tags: #clang

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

show more ...

Revert "[NFC] Refactor representation of materialized temporaries"

This reverts commit 08ea1ee2db5f9d6460fef1d79d0d1d1a5eb78982.
It broke ./ClangdTests/FindExplicitReferencesTest.All
on the bots, se

Revert "[NFC] Refactor representation of materialized temporaries"

This reverts commit 08ea1ee2db5f9d6460fef1d79d0d1d1a5eb78982.
It broke ./ClangdTests/FindExplicitReferencesTest.All
on the bots, see comments on https://reviews.llvm.org/D69360

show more ...

[NFC] Refactor representation of materialized temporaries

Summary:
this patch refactor representation of materialized temporaries to prevent an issue raised by rsmith in https://reviews.llvm.org/D63

[NFC] Refactor representation of materialized temporaries

Summary:
this patch refactor representation of materialized temporaries to prevent an issue raised by rsmith in https://reviews.llvm.org/D63640#inline-612718

Reviewers: rsmith, martong, shafik

Reviewed By: rsmith

Subscribers: rnkovacs, arphaman, cfe-commits

Tags: #clang

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

show more ...

Thread Safety: also look at ObjC methods

Summary:
SExprBuilder::translateDeclRefExpr was only looking at FunctionDecl and not also looking at ObjCMethodDecl. It should consider both because the attr

Thread Safety: also look at ObjC methods

Summary:
SExprBuilder::translateDeclRefExpr was only looking at FunctionDecl and not also looking at ObjCMethodDecl. It should consider both because the attributes can be used on Objective-C as well.

<rdar://problem/48941331>

Reviewers: dexonsmith, erik.pilkington

Subscribers: jkorous, jdoerfert, cfe-commits

Tags: #clang

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

llvm-svn: 356940

show more ...

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the ne

Update the file headers across all of the LLVM projects in the monorepo
to reflect the new license.

We understand that people may be surprised that we're moving the header
entirely to discuss the new license. We checked this carefully with the
Foundation's lawyer and we believe this is the correct approach.

Essentially, all code in the project is now made available by the LLVM
project under our new license, so you will see that the license headers
include that license only. Some of our contributors have contributed
code under our old license, and accordingly, we have retained a copy of
our old license notice in the top-level files in each project and
repository.

llvm-svn: 351636

show more ...

[AST][NFC] Pass the AST context to one of the ctor of DeclRefExpr.

All of the other constructors already take a reference to the AST context.
This avoids calling Decl::getASTContext in most cases. A

[AST][NFC] Pass the AST context to one of the ctor of DeclRefExpr.

All of the other constructors already take a reference to the AST context.
This avoids calling Decl::getASTContext in most cases. Additionally move
the definition of the constructor from Expr.h to Expr.cpp since it is calling
DeclRefExpr::computeDependence. NFC.

llvm-svn: 349901

show more ...