erichkeane added inline comments.
================
Comment at: clang/lib/Sema/Sema.cpp:1514
+ void visitUsedDecl(SourceLocation Loc, Decl *D) {
+ if (auto *TD = dyn_cast<TranslationUnitDecl>(D)) {
+ for (auto *DD : TD->decls()) {
----------------
rjmccall wrote:
> erichkeane wrote:
> > Note that when recommitting this (if you choose to), this needs to also
> > handle NamespaceDecl. We're a downstream and discovered that this doesn't
> > properly handle functions or records handled in a namespace.
> >
> > It can be implemented identically to TranslationUnitDecl.
> Wait, what? We shouldn't be doing this for TranslationUnitDecl either. I
> don't even know how we're "using" a TranslationUnitDecl, but neither this
> case not the case for `NamespaceDecl` should be recursively using every
> declaration declared inside it. If there's a declaration in a namespace
> that's being used, it should be getting visited as part of the actual use of
> it.
>
> The logic for `RecordDecl` has the same problem.
Despite the name, this seems to be more of a home-written ast walking class.
The entry point is the 'translation unit' which seems to walk through
everything in an attempt to find all the functions (including those that are
'marked' as used by an attribute).
You'll see the FunctionDecl section makes this assumption as well (not
necessarily that we got to a function via a call). IMO, this approach is
strange, and we should register entry points in some manner (functions marked
as emitted to the device in some fashion), then just follow its call-graph (via
the clang::CallGraph?) to emit all of these functions.
It seemed really odd to see this approach here, but it seemed well reviewed by
the time I noticed it (via a downstream bug) so I figured I'd lost my chance to
disagree with the approach.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70172/new/
https://reviews.llvm.org/D70172
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
