Skip to content

Commit ee04dea

Browse files
committed
PR feedback
1 parent f131b9f commit ee04dea

1 file changed

Lines changed: 15 additions & 12 deletions

File tree

Sources/idt/idt.cc

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,7 @@ struct PPCallbacks : clang::PPCallbacks {
130130
class DeclSet {
131131
std::set<std::uintptr_t> decls_;
132132

133-
// Use the raw address of the canonical declaration object as a unique
134-
// identifier.
133+
// Use pointer identity of the canonical declaration object as a unique ID.
135134
template <typename Decl_>
136135
std::uintptr_t decl_id(const Decl_ *D) const {
137136
return reinterpret_cast<std::uintptr_t>(D->getCanonicalDecl());
@@ -206,7 +205,12 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
206205
}
207206

208207
clang::DiagnosticBuilder
209-
unexported_public_interface(clang::SourceLocation location, const clang::Decl *D) {
208+
unexported_public_interface(const clang::Decl *D) {
209+
return unexported_public_interface(D, get_location(D));
210+
}
211+
212+
clang::DiagnosticBuilder
213+
unexported_public_interface(const clang::Decl *D, clang::SourceLocation location) {
210214
add_missing_include(location);
211215
exported_decls_.insert(D);
212216

@@ -275,8 +279,7 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
275279
if (llvm::isa<clang::RecordDecl>(D))
276280
return false;
277281

278-
// For non-record declarations, check to see if the containing record, if
279-
// any, is exported. This exports the member as well.
282+
// For non-record declarations, the DeclContext is the containing record.
280283
for (const clang::DeclContext *DC = D->getDeclContext(); DC; DC = DC->getParent())
281284
if (const auto *RD = llvm::dyn_cast<clang::RecordDecl>(DC))
282285
return is_symbol_exported(RD);
@@ -340,7 +343,7 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
340343
FD->getTemplatedKind() == clang::FunctionDecl::TK_NonTemplate
341344
? FD->getBeginLoc()
342345
: FD->getInnerLocStart();
343-
unexported_public_interface(get_location(FD), FD)
346+
unexported_public_interface(FD)
344347
<< FD << clang::FixItHint::CreateInsertion(SLoc, export_macro + " ");
345348
}
346349

@@ -355,6 +358,10 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
355358
if (is_in_system_header(VD))
356359
return;
357360

361+
// Skip all variable declarations not in header files.
362+
if (!is_in_header(VD))
363+
return;
364+
358365
// Skip local variables. We are only interested in static fields.
359366
if (VD->getParentFunctionOrMethod())
360367
return;
@@ -363,10 +370,6 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
363370
if (VD->hasInit())
364371
return;
365372

366-
// Skip all variable declarations not in header files.
367-
if (!is_in_header(VD))
368-
return;
369-
370373
// Skip all other local and global variables unless they are extern.
371374
if (!VD->isStaticDataMember() &&
372375
VD->getStorageClass() != clang::StorageClass::SC_Extern)
@@ -392,7 +395,7 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
392395
return;
393396

394397
clang::SourceLocation SLoc = VD->getBeginLoc();
395-
unexported_public_interface(get_location(VD), VD)
398+
unexported_public_interface(VD)
396399
<< VD << clang::FixItHint::CreateInsertion(SLoc, export_macro + " ");
397400
}
398401

@@ -432,7 +435,7 @@ class visitor : public clang::RecursiveASTVisitor<visitor> {
432435
clang::SourceLocation SLoc = RD->getLocation();
433436
const clang::SourceLocation location =
434437
context_.getFullLoc(SLoc).getExpansionLoc();
435-
unexported_public_interface(location, RD)
438+
unexported_public_interface(RD, location)
436439
<< RD << clang::FixItHint::CreateInsertion(SLoc, export_macro + " ");
437440
}
438441

0 commit comments

Comments
 (0)