This is the unadorned result of:
git grep -En "\w.*return .*;" -- ':!tests/*' | grep-format
Having return statements on their own line makes it easier to set
breakpoints in most debuggers.
No functional change.
This function had some bad indentation that disagrees with clang-format.
To prepare for some changes and make their diff more focused, reformat
up-front.
In very particular circumstances (see test case), VisitFunctionDecl
would see a function definition that was an implicit constructor of a
builtin (va_list_tag).
Rather than crashing for implicit code, just say it's not in a header.
This feels a little questionable, because depending on how IsInHeader is
used, it might be misleading that it returns false for builtins/implicit
code. But I think it makes sense for now.
Fixes issue #1162.
Clang 854c10f8d185286d941307e1033eb492e085c203 changed
PPCallbacks::InclusionDirective and Preprocessor::LookupFile to use
clang::OptionalFileEntryRef instead of llvm::Optional<FileEntryRef>.
Update our overrides and calls to match.
Non-sugared template specialization type template arguments are already
analyzed due to 'TraverseType' call inside
'InstantiatedTemplateVisitor::TraverseSubstTemplateTypeParmTypeHelper'
and subsequent 'TraverseTemplateSpecializationTypeHelper' call.
Components of sugared template specialization types are added into
resugar_map, otherwise their template arguments would not be reported.
Test case has been extended.
Full template specialization type use requires full information about
its template-argument-dependent fields and nested typedefs. But earlier,
it wasn't reported in many cases when template specialization type isn't
written explicitly in non-fwd-decl context.
Only attempt to report decl uses of template names that have an
underlying template decl. This ignores uses of dependent templates and
potentially other such variations.
Even if that causes us to miss reporting for some valid constructs, it
allows further analysis of files that would otherwise crash IWYU.
Fixes issue #1140
Add using-declaration for clang::TemplateDecl, and remove explicit
namespace qualifiers for use of RecordDecl and CXXRecordDecl, which are
already pulled in with using-declarations.
No functional change intended.
this was a proposed issue in #1095 where one could
make a block of keeps as opposed to marking every
keep with a pragma: keep instruction.
added test to verify and updated documentation
where appropriate.
This change provides better location info when using macros such as:
BOOST_CHECK_THROW(..., Exc);
The use of type Exc would be attributed to the file defining the macro
before this patch, but is now correctly attributed to the expansion
location.
Co-authored-by: Kim Gräsman <kim.grasman@gmail.com>
A function may just transmit passed-by-reference parameter somewhere.
Requirement to explicitly write forward declaration in the same file
(.cpp-file) to avoid '#include' suggestion is impractical when that type
is already fwd-declared in the corresponding header.
'Autocast' may still make sense for header-defined functions, due to
unlimited number of possible callers, so analysis of those fuctions
is kept.
Both function declaration site handling and call site handling
are changed.
There is no need in reporting enum opaque declaration or full use if
enumeration type name or enumerator name isn't explicitly written,
because it should already be present elsewhere in the translation unit,
probably through included headers.
These changes have no effect on the current test suite, but there are
obvious weaknesses when sugared types are taken into account.
The way CanForwardDeclareType asks if the parent is-a particular AST
node type is generally unsound in the presence of sugar, such as
ElaboratedType, which may show up above any Type in the tree.
An example:
class C {};
typedef C FooBar;
produces:
`-TypedefDecl 0x559e505c6e30
`-ElaboratedType 0x559e505c6df0 'C' sugar
`-RecordType 0x559e505c6cc0 'C'
`-CXXRecord 0x559e505c6c30 'C'
If we were to call CanForwardDeclare for the type C, it would not
classify as part of a TypedefDecl, because its parent is-a
ElaboratedType.
Therefore, delay any parent type checking until we've rewinded the
current AST node using MostElaboratedAncestor (there's a case to be made
that we should add a more general MostSugaredAncestor to walk _up_ the
tree until a desugared node is reached, much the same way as Desugar
will walk _down_ the tree. But that's good idea for a separate project.)
This was inspired by Clang 15f3cd6bfc670ba6106184a903eb04be059e5977,
which wraps the majority of Type nodes in an additional ElaboratedType
node.
This is kind-of the inverse of desugaring -- walk up the ancestor chain until
we're at the most elaborated (should really be "sugared") ancestor. Then we can
make assumptions about parent types again.
Clang 15f3cd6bfc670ba6106184a903eb04be059e5977 made this necessary, because many
more type nodes are now wrapped in ElaboratedType, which breaks naive parent
type checking.
We already did desugaring (or, historically, RemoveElaboration) before
insertion or lookup in responsibility maps -- e.g.
GetCallerResponsibleTypesForAutocast and
GetCallerResponsibleTypesForFnReturn -- in most cases.
Clang after 15f3cd6bfc670ba6106184a903eb04be059e5977 wraps many more
type nodes in ElaboratedType, so we need to be even more diligent about
this.
No visible functional change, but we now desugar more aggressively.
This patch is the result of:
git grep -l "RemoveSubstTemplateTypeParm" | xargs sed -i -e 's/RemoveSubstTemplateTypeParam/Desugar/'
Desugar performs the same desugaring as RemoveElaboration, and more.
This patch is a straight:
git grep -l "RemoveElaboration" | xargs sed -i -e 's/RemoveElaboration/Desugar/'
followed by a test cleanup in badinc.cc, where Desugar sees through more sugar
than RemoveElaboration, and actually produces better results.
RemovePointersAndReferencesAsWritten should be allowed to return a
desugared type, so typecheck for SubstTemplateTypeParmType on the type
before pointers/refs (and any sugar) have been removed.
Downcasting an AST Type* to a more specific type is increasingly unsafe in the
AST. In general any Type* picked out of the AST might contain so-called 'sugar',
i.e. visible or invisible structure that does not change the identity of the
underlying type, but adds information.
After Clang 15f3cd6bfc670ba6106184a903eb04be059e5977, almost every Type is
wrapped in an ElaboratedType with kind ETK_None (i.e. "no elaboration").
ElaboratedType counts as sugar, and thus needs to be stripped off before we do
dynamic type checking of Type nodes.
The recommended way to look through sugar downwards in the tree is
'type->getAs<MostDerivedType>()', which will recursively remove sugar until a
node of MostDerivedType is reached. If there is no MostDerivedType node it
returns nullptr, so it's safe to consider getAs a desugaring dynamic downcast.
There are still some places where we use DynCastFrom or isa<> on Types, I have
only modified the ones that had any effect on broken testcases. Planning to look
into the rest separately once the test suite is working.
This fixes 4 broken testcases, and is an improvement towards fixing #1092.
Use the new IsElaboratedTypeSpecifier instead of node type-checking.
Just because an AST node is an ElaboratedType does not mean it's an
actual elaborated type specifier (which is what we care about here).
This fixes 10 broken testcases after Clang 15f3cd6bfc670ba6106184a903eb
wrapped every Type inside an ElaboratedType in the AST, but it is
strictly a bugfix in general: all elaborated type nodes are not type
specifiers.
Part of fix for #1092.
'Elaborated type specifier' is the formal term for 'struct X', 'class Y' or
'union Z'. Use it to avoid potential confusion around what 'elaboration node'
means.
Clean up a comment that claimed 'MostElaboratedAncestor' used
'IsElaborationNode', where it actually doesn't.
No functional change.
The IwyuBaseAstVisitor has a base class, BaseAstVisitor, but it only has a
small number of Visit functions implemented, for logging.
So there is no need to call Base::VisitX in general, only for the few with
an implementation that actually does something, and they are preserved
here.
This covers the common C pattern of declaring a typedef for a struct before the
struct itself, e.g.
typedef struct foo foo_t;
struct foo {
int value;
foo_t *next;
};
Fixes issue #1065.
The reason is that
1) enumerators led to much noise in explanatory comments
(the idea is similar to cedf9d6984323a0a68847d), and
2) without this change, EnumClass::Item is reported separately
as fwd-decl use of EnumClass along with the full use
of EnumClass::Item.
Unnamed enums are an exception.
This is preparation for adding suggestions for enumeration opaque
(forward) declarations. Enums should be treated almost similar
to classes and structs, so clang::RecordDecl should be replaced
by clang::TagDecl, which is a superclass for RecordDecl and EnumDecl,
in many places.
We used to ignore all analysis of AST nodes inside a `typedef`, but that
caused us to miss author-intent analysis of nested typedefs.
Remove the special casing for member-of-typedef now that reporting as a
whole is less granular (we now report only the parent type instead of
all nested components).
Add test cases to demonstrate that nested typedefs observe
the author-intent rules.
Non-template code only allows the null decl pointer to be ignored.
For instantiated templates, CanIgnoreDecl is quite sophisticated and uses a
set to ignore subtrees we've visited before, but it doesn't explicitly ignore
the null decl.
There are cases in templated code where the decl can be null as well -- most
prominently for builtin types.
With this test input:
$ cat t.cc
#include <algorithm>
#include <cassert>
#include <vector>
void f() {
std::vector<int> v;
assert(std::none_of(v.cbegin(), v.cend(),
[](auto value) { return value == 100; }));
}
$ include-what-you-use -stdlib=libc++ t.cc
(note the explicit use of libc++)
std::vector<int>::const_iterator collapses to a 'const int *', which has no
declaration, and so yields a null decl. Prior to this fix, that would lead to a
cast assertion or segfault attempting to cast the resulting null pointer to a
FunctionDecl in GetCanonicalUseLocation.
Add an assertion for non-null to GetCanonicalUseLocation to clarify that
null filtering must happen before canonicalizing the use-location.
I have not found a way to capture this in a reduced testcase.
Fixes#989.
Before, this would segfault attempting to print e.g. null decls.
These printers are predominantly used for logging, and it seems like a shame for
debug logging to crash on a null pointer if the code around it does not.
Remove now-unnecessary null-checks in logging.
Rather than recomputing use-flags for decl and using-decl (if one exists),
compute it once into a temporary.
Remove gratuitous const to fit other use-flags expression on one line.
When a consteval function is called, its call-expression is wrapped in a
ConstantExpr in the Clang AST. There's an upstream bug [1] where Clang
doesn't account for that when searching the AST for a cast-expression's
potential user-defined conversion function, leading to an assertion
failure or a crash.
Reimplement GetConversionFunction so it works in this case. This can
safely be reverted when the upstream bug is fixed.
[1] https://github.com/llvm/llvm-project/issues/53044
Two new options:
--error: sets the exit code to use if there are IWYU violations
--error_always: sets the exit code to use whether there are IWYU
violations or not
Add tests to capture all relevant combinations of
(no warnings, warnings) x
(--error, --error_always) x
(explicit arguments, default values)
and make sure --error_always overrides --error.
Exit with EXIT_FAILURE if the command-line arguments are invalid or
Clang fails to parse the input. Otherwise exit with EXIT_SUCCESS.
Add tests to capture this new policy.
CodeAuthorWantsJustAForwardDeclare uses location information to see whether the
there's a forward-declare hint earlier in the same file.
Implicit decls do not have locations, so assume the author (actually the
compiler itself) does not intend to provide hints to IWYU.
I've found no way to test this in a self-contained example. It was originally
reported in issue #1005 as an assertion triggered by the AArch64 target's
implementation of va_list:
$ cat /tmp/test.c
#include <stdarg.h>
void test(void) {
va_list args;
}
$ include-what-you-use --target=aarch64-linux-gnu -c /tmp/test.c
include-what-you-use: ...SourceLocation.h:433: ...
Assertion `Loc.isValid()' failed.
Aborted (core dumped)
Since I don't want the test suite to depend on environment (in this case,
particular targets being available), I'll leave it untested.
The change was introduced in the llvm-project repo here:
105c913156
The goal is to be able to pass a nullptr directory
to the LookupFile function if we are not interested
in retrieving the current directory, instead of
creating a dummy variable that is not used after
the function is called.
Fixes#996
Clang added a new AST node in
https://github.com/llvm/llvm-project/commit/e1600db19d6303f84b995acb93, which
shows up in the AST instead of a desugared type.
The presence of a using declaration would be omitted in the AST in favor of the
target type.
Outside of the test suite, we noticed this for <cstdint>, where the integer
types would show up as TypedefTypes in the AST, despite being pulled in via
using-declarations. They now show as UsingTypes. UsingType represents a new
so-called sugar type introduced by way of a using declaration, and links to its
UsingShadowDecl, which in turn makes it possible to chase through the
using-chain.
Report a use of the UsingShadowDecl, which is resolved centrally to the
underlying target decl (desugared typedef) as well as reporting the using-decl
itself. This is consistent with the prior behavior, but should provide better
results for tracking using declarations.
Kim Gräsman