Covers both begin_export/end_export blocks and single-line export
pragmas.
Like with 'IWYU pragma: keep' marks the forward decl as automatically
desired to avoid removing manually exported but unused decls.
Add a simple testcase and update documentation.
Rename ForwardDeclareInKeepRange to ForwardDeclareIsMarkedKeep, take a
NamedDecl and move all location and text wrangling into the function.
This is a layering violation in a sense, because the preprocessor
doesn't know anything about declarations. But the API is much cleaner
that way from the callers' perspective -- they typically just have a
decl and shouldn't be trusted to use the right location info.
No functional change.
Previously, ReportDeclUse was called for TypedefDecl when TypedefType
is passed to ReportTypeUse. This change short-circuits that path. But
an underlying type should still be reported if the typedef doesn't
"provide" it. Hence, corresponding logic is moved into ReportTypeUse.
So ReportDeclUse reports typedef declarations and ReportTypeUse reports
typedef underlying types now.
The core AST nodes (Decl, Stmt, Type, TypeLoc) all have a kind denoting
their specific derived type, and a corresponding function to get a
printable string for the kind.
Implement a polymorphic GetKindName that calls the right function
depending on type, and also adds a suffix to mirror the clang class
name.
Make all uses of the get...Name functions call these wrappers instead.
No visible change intended.
Report fwd-decls only for explicitly written types
There is no point in reporting forward-declaration of a type when just
its pointer is used and the type is not explicitly written
in the source. Explicitly written pointer types are handled, e.g.,
in VisitTemplateSpecializationType and VisitTagType.
Add test cases for redundant fwd-decl absence
This is the unadorned output of:
git grep -En "^\s*=" -- ':!tests/*' | grep-format
There are some examples with comments between the variable name and the
'=' that clang-format left unchanged.
No functional change.
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.
Kim Gräsman