Skip to content

Commit

Permalink
[cxxmodules] Prevent RScanner from accessing hidden declarations.
Browse files Browse the repository at this point in the history
RScanner iterates over all decls in our AST, but with modules we
have hidden decl from unimported submodules in our AST. As we
call Sema functions on these decls which use the normal clang
lookup that respects visibility, we suddenly get mysterious
lookup failures from inside Sema when running rootcling.

This patch restricts RScanner to looking at visible decls, which
restores the original behavior where RScanner onlys sees visible decls
from included headers.
  • Loading branch information
Teemperor authored and vgvassilev committed Sep 1, 2017
1 parent 9c2851e commit a785402
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 0 deletions.
2 changes: 2 additions & 0 deletions core/dictgen/res/Scanner.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ class ClassSelectionRule;
class RScanner: public clang::RecursiveASTVisitor<RScanner>
{

bool shouldVisitDecl(clang::NamedDecl *D);

public:
class AnnotatedNamespaceDecl {
public:
Expand Down
39 changes: 39 additions & 0 deletions core/dictgen/src/Scanner.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,27 @@ RScanner::~RScanner ()
{
}

////////////////////////////////////////////////////////////////////////////////
/// Whether we can actually visit this declaration, i.e. if it is reachable
/// via name lookup.
///
/// RScanner shouldn't touch decls for which this method returns false as we
/// call Sema methods on those declarations. Those will fail in strange way as
/// they assume those decls are already visible.
///
/// The main problem this is supposed to prevent is when we use C++ modules and
/// have hidden declarations in our AST. Usually they can't be found as they are
/// hidden from name lookup until their module is actually imported, but as the
/// RecursiveASTVisitor is not supposed to be restricted by lookup limitations,
/// it still reaches those hidden declarations.
bool RScanner::shouldVisitDecl(clang::NamedDecl *D)
{
if (auto M = D->getOwningModule()) {
return fInterpreter.getSema().isModuleVisible(M);
}
return true;
}

////////////////////////////////////////////////////////////////////////////////

inline void* ToDeclProp(clang::Decl* item)
Expand Down Expand Up @@ -455,6 +476,9 @@ bool RScanner::VisitNamespaceDecl(clang::NamespaceDecl* N)
if (fScanType == EScanType::kOnePCM)
return true;

if (!shouldVisitDecl(N))
return true;

// in case it is implicit we don't create a builder
if((N && N->isImplicit()) || !N){
return true;
Expand Down Expand Up @@ -490,6 +514,9 @@ bool RScanner::VisitNamespaceDecl(clang::NamespaceDecl* N)

bool RScanner::VisitRecordDecl(clang::RecordDecl* D)
{
if (!shouldVisitDecl(D))
return true;

// This method visits a class node
return TreatRecordDeclOrTypedefNameDecl(D);

Expand Down Expand Up @@ -806,6 +833,9 @@ bool RScanner::VisitTypedefNameDecl(clang::TypedefNameDecl* D)
if (fScanType == EScanType::kOnePCM)
return true;

if (!shouldVisitDecl(D))
return true;

const clang::DeclContext *ctx = D->getDeclContext();

bool isInStd=false;
Expand All @@ -829,6 +859,9 @@ bool RScanner::VisitEnumDecl(clang::EnumDecl* D)
if (fScanType == EScanType::kOnePCM)
return true;

if (!shouldVisitDecl(D))
return true;

if(fSelectionRules.IsDeclSelected(D) &&
!IsElementPresent(fSelectedEnums, D)){ // Removal of duplicates.
fSelectedEnums.push_back(D);
Expand All @@ -845,6 +878,9 @@ bool RScanner::VisitVarDecl(clang::VarDecl* D)
fScanType == EScanType::kOnePCM)
return true;

if (!shouldVisitDecl(D))
return true;

if(fSelectionRules.IsDeclSelected(D)){
fSelectedVariables.push_back(D);
}
Expand Down Expand Up @@ -882,6 +918,9 @@ bool RScanner::VisitFunctionDecl(clang::FunctionDecl* D)
if (fScanType == EScanType::kOnePCM)
return true;

if (!shouldVisitDecl(D))
return true;

if(clang::FunctionDecl::TemplatedKind::TK_FunctionTemplate == D->getTemplatedKind())
return true;

Expand Down

0 comments on commit a785402

Please sign in to comment.