Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions cpp/libclang/src/visitor/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ rust_library(
"//tools/metamodel/class:class_diagram",
"//tools/metamodel/sequence:sequence_diagram",
"@crates//:clang",
"@crates//:log",
"@crates//:serde",
],
)
Expand All @@ -40,6 +41,7 @@ rust_test(
],
deps = [
"@crates//:clang",
"@crates//:log",
"@crates//:serde",
],
)
Expand Down
12 changes: 10 additions & 2 deletions cpp/libclang/src/visitor/src/class_parser_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,10 +547,18 @@ fn resolve_named_type(original: &Type, canonical: &Type) -> ResolvedType {
}

if is_dependent_expression_type(original) {
return ResolvedType::Dependent(resolve_unknown_name(original, canonical));
let name = resolve_unknown_name(original, canonical);
log::debug!(
"type '{}' is structurally unresolvable before template instantiation \
(dependent/decltype expression)",
name
);
return ResolvedType::Dependent(name);
}

ResolvedType::Unknown(resolve_unknown_name(original, canonical))
let name = resolve_unknown_name(original, canonical);
log::debug!("could not resolve type '{}' to a concrete entity id", name);
ResolvedType::Unknown(name)
}

/// Detects types libclang exposes as `Unexposed` because their meaning depends on
Expand Down
94 changes: 72 additions & 22 deletions cpp/libclang/src/visitor/src/class_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@ impl ClassVisitor {
entity: &Entity,
namespace: Option<&str>,
) -> Option<(ParsedClassInfo, SimpleEntity)> {
let name = entity.get_name()?;
let Some(name) = entity.get_name() else {
log::debug!("skipping class/struct: anonymous type has no name");
return None;
};

let id = match namespace {
Some(ns) if !ns.is_empty() => format!("{ns}::{name}"),
Expand Down Expand Up @@ -154,8 +157,17 @@ pub(crate) fn parse_source_location(entity: &Entity) -> SourceLocation {
}

fn collect_variable_type(entity: &Entity) -> Option<ParsedVariableType> {
let name = entity.get_name()?;
let field_type = entity.get_type()?;
let Some(name) = entity.get_name() else {
log::debug!("skipping field/variable: entity has no name");
return None;
};
let Some(field_type) = entity.get_type() else {
log::debug!(
"skipping field/variable '{}': could not determine its type",
name
);
return None;
};

Some(ParsedVariableType {
name,
Expand Down Expand Up @@ -204,11 +216,21 @@ fn method_arguments<'tu>(entity: &Entity<'tu>) -> Vec<Entity<'tu>> {
}

fn parse_type_alias(entity: &Entity) -> Option<TypeAlias> {
let alias = entity.get_name()?;
let Some(alias) = entity.get_name() else {
log::debug!("skipping type alias: entity has no name");
return None;
};

let original_type = entity
let Some(original_type) = entity
.get_typedef_underlying_type()
.map(|t| render_type_for_display(&t, &resolve_type(&t)))?;
.map(|t| render_type_for_display(&t, &resolve_type(&t)))
else {
log::debug!(
"skipping type alias '{}': could not determine underlying type",
alias
);
return None;
};

Some(TypeAlias {
alias,
Expand Down Expand Up @@ -407,23 +429,41 @@ fn infer_entity_type_from_members(kind: EntityKind, class: &SimpleEntity) -> Ent
fn build_relationships_for_class(ctx: &mut VisitContext, builder: &ParsedClassInfo) {
for base in &builder.base_classes {
let Some(resolved_base) = base.resolved_type.referenced_entity_id() else {
eprintln!(
"Warning: unable to resolve base type '{}' for '{}'; \
skipping inheritance relationship (dependent/decltype expression?)",
base.resolved_type.render_for_display(),
builder.id
);
if matches!(base.resolved_type, ResolvedType::Dependent(_)) {
// Expected, permanent limitation of AST-only analysis (e.g. a
// `decltype`/SFINAE base class that cannot be resolved without
// template instantiation) — never abort, not even in debug/test
// builds.
log::debug!(
"unable to resolve base type '{}' for '{}'; \
skipping inheritance relationship (dependent/decltype expression)",
base.resolved_type.render_for_display(),
builder.id
);
} else {
// Unexpected: a base class resolving to `Unknown`/`Builtin` likely
// indicates a gap in the resolver rather than a known limitation.
// When in doubt, warn and continue rather than abort — a single
// unanticipated input must never crash the parser.
log::warn!(
"unable to resolve base type '{}' for '{}'; \
skipping inheritance relationship (unexpected unresolved type)",
base.resolved_type.render_for_display(),
builder.id
);
}
continue;
};

let Some(target_class) = ctx.types.get(resolved_base) else {
// Base type is not in the type map — it is likely an external dependency
// that was filtered out during the visit phase. Skip the relationship
// rather than panicking so analysis of workspace classes can proceed.
eprintln!(
"Warning: base type '{}' not found in type map for '{}'; \
skipping inheritance relationship (external dependency?)",
resolved_base, builder.id
// that was filtered out during the visit phase. This is expected and
// common, so skip the relationship without ever aborting.
log::debug!(
"base type '{}' not found in type map for '{}'; \
skipping inheritance relationship (external dependency)",
resolved_base,
builder.id
);
continue;
};
Expand All @@ -434,10 +474,20 @@ fn build_relationships_for_class(ctx: &mut VisitContext, builder: &ParsedClassIn
RelationType::Inheritance
};

let class = ctx
.types
.get_mut(&builder.id)
.expect("Source class must exist before building relationships");
let Some(class) = ctx.types.get_mut(&builder.id) else {
// Internal invariant: `builder.id` is derived from `ctx.types` during
// the visit phase, so it should always still be present here. If it
// isn't, that's a bug in the visitor pipeline rather than an expected
// input condition. When in doubt, warn and skip rather than abort —
// a single unanticipated input must never crash the parser.
log::warn!(
"source class '{}' unexpectedly missing from type map; \
skipping inheritance relationship to '{}'",
builder.id,
resolved_base
);
continue;
};

add_relationship(
class,
Expand Down
42 changes: 42 additions & 0 deletions cpp/libclang/src/visitor/src/class_visitor_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,45 @@ fn resolve_relationships_skips_dependent_base_class_without_panicking() {
SourceLocation::new(source_file, 5)
);
}

/// An unresolved base type that is *not* `ResolvedType::Dependent` (e.g.
/// `Unknown`) is unexpected and gets a `log::warn!`, but must still never
/// abort the parser — when in doubt, warn and skip rather than crash.
#[test]
fn resolve_relationships_warns_and_skips_unexpected_unresolved_base() {
let source_file = "unit_source.cpp";

let mut ctx = VisitContext::default();
ctx.types.insert(
"Derived".to_string(),
SimpleEntity {
id: "Derived".to_string(),
name: "Derived".to_string(),
source_location: SourceLocation::new(source_file, 1),
..Default::default()
},
);

ctx.parsed_class_info.push(ParsedClassInfo {
id: "Derived".to_string(),
base_classes: vec![ParsedBaseClass {
// Not `Dependent`: an unexpected, unresolvable base type.
resolved_type: ResolvedType::Unknown("SomeWeirdType".to_string()),
source_location: SourceLocation::new(source_file, 1),
}],
variable_types: vec![],
method_types: vec![],
});

// Must not panic.
ClassVisitor::resolve_relationships(&mut ctx);

let derived = ctx
.types
.get("Derived")
.expect("Derived must still exist after relationship resolution");
assert!(
derived.relationships.is_empty(),
"unresolvable base class must not produce a relationship"
);
}
9 changes: 8 additions & 1 deletion cpp/libclang/src/visitor/src/enum_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ impl AstVisitor for EnumVisitor {

impl EnumVisitor {
fn visit_enum(entity: Entity) -> Option<SimpleEntity> {
let name = entity.get_name()?;
let Some(name) = entity.get_name() else {
log::debug!("skipping enum: anonymous enum has no name");
return None;
};
let namespace_id = Self::get_namespace_id(&entity);
let full_qualified_id = if let Some(namespace_id) = &namespace_id {
format!("{}::{}", namespace_id, name)
Expand Down Expand Up @@ -79,6 +82,10 @@ fn get_literals(entity: Entity) -> Vec<EnumLiteral> {
Some(constant_val_tuple.0 as i128)
}
} else {
log::debug!(
"enum constant '{}' has no value; skipping literal",
c.get_name().unwrap_or_default()
);
return None;
};

Expand Down
21 changes: 19 additions & 2 deletions cpp/libclang/src/visitor/src/function_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,40 @@ impl FunctionVisitor {
// ── Top-level extraction ──────────────────────────────────────────────────

fn extract_function_def(entity: Entity) -> Option<FunctionDef> {
let body_node = Self::get_method_body(entity)?;
let Some(body_node) = Self::get_method_body(entity) else {
log::debug!(
"skipping method '{}': no compound statement body (declaration-only?)",
entity.get_name().unwrap_or_default()
);
return None;
};

if !entity
.get_location()
.map(|loc| loc.is_in_main_file())
.unwrap_or(false)
{
log::debug!(
"skipping method '{}': not located in the main file",
entity.get_name().unwrap_or_default()
);
return None;
}

let method_name = entity.get_name()?;
let Some(method_name) = entity.get_name() else {
log::debug!("skipping method: entity has no name");
return None;
};
let class_name = entity
.get_semantic_parent()
.and_then(|p| p.get_name())
.unwrap_or_default();

if class_name.is_empty() {
log::debug!(
"skipping method '{}': owning class/struct has no name",
method_name
);
return None;
}

Expand Down
Loading