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.
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 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.
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 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
<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
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.
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.
This reverts commit 08ea1ee2db5f9d6460fef1d79d0d1d1a5eb78982.
It broke ./ClangdTests/FindExplicitReferencesTest.All
on the bots, see comments on https://reviews.llvm.org/D69360
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
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
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
A ConstantExpr class represents a full expression that's in a context where a
constant expression is required. This class reflects the path the evaluator
took to reach the expression rather than the syntactic context in which the
expression occurs.
In the future, the class will be expanded to cache the result of the evaluated
expression so that it's not needlessly re-evaluated
Reviewed By: rsmith
Differential Revision: https://reviews.llvm.org/D53475
llvm-svn: 345692
Not used productively, so no observable functional change.
Note that printSCFG doesn't yet work reliably, it seems to crash
sometimes.
llvm-svn: 342790
For function pointers, the FunctionDecl of the callee is unknown, so
getDirectCallee will return nullptr. We have to catch that case to avoid
crashing. We assume there is no attribute then.
llvm-svn: 342519
This is similar to the LLVM change https://reviews.llvm.org/D46290.
We've been running doxygen with the autobrief option for a couple of
years now. This makes the \brief markers into our comments
redundant. Since they are a visual distraction and we don't want to
encourage more \brief markers in new code either, this patch removes
them all.
Patch produced by
for i in $(git grep -l '\@brief'); do perl -pi -e 's/\@brief //g' $i & done
for i in $(git grep -l '\\brief'); do perl -pi -e 's/\\brief //g' $i & done
Differential Revision: https://reviews.llvm.org/D46320
llvm-svn: 331834
Found via codespell -q 3 -I ../clang-whitelist.txt
Where whitelist consists of:
archtype
cas
classs
checkk
compres
definit
frome
iff
inteval
ith
lod
methode
nd
optin
ot
pres
statics
te
thru
Patch by luzpaz! (This is a subset of D44188 that applies cleanly with a few
files that have dubious fixes reverted.)
Differential revision: https://reviews.llvm.org/D44188
llvm-svn: 329399
Adding the new enumerator forced a bunch more changes into this patch than I
would have liked. The -Wtautological-compare warning was extended to properly
check the new comparison operator, clang-format needed updating because it uses
precedence levels as weights for determining where to break lines (and several
operators increased their precedence levels with this change), thread-safety
analysis needed changes to build its own IL properly for the new operator.
All "real" semantic checking for this operator has been deferred to a future
patch. For now, we use the relational comparison rules and arbitrarily give
the builtin form of the operator a return type of 'void'.
llvm-svn: 320707
The implementation is in AnalysisDeclContext.cpp and the class is called
AnalysisDeclContext.
Making those match up has numerous benefits, including:
- Easier jump from header to/from implementation.
- Easily identify filename from class.
Differential Revision: https://reviews.llvm.org/D37500
llvm-svn: 312671
Sorry for the noise, I managed to miss a bunch of recent regressions of
include orderings here. This should actually sort all the includes for
Clang. Again, no functionality changed, this is just a mechanical
cleanup that I try to run periodically to keep the #include lines as
regular as possible across the project.
llvm-svn: 225979
Numerous changes, including:
* Changed the way variables and instructions are handled in basic blocks to
be more efficient.
* Eliminated SExprRef.
* Simplified futures.
* Fixed documentation.
* Compute dominator and post dominator trees.
llvm-svn: 217556
til::SExpr. This is a large patch, with many small changes to pretty printing
and expression lowering to make the new SExpr representation equivalent in
functionality to the old.
llvm-svn: 214089
The thread safety analysis isn't very useful in ObjC (you can't annotate
ObjC classes or methods) but we can still analyze the actual code and
show violations in usage of C/C++ functions.
Fixes PR19541, which does not use thread safety attributes but crashes
with -Weverything.
llvm-svn: 208436
