From 18c027f63eec08cf0eeb53adce8f2d344c5f172b Mon Sep 17 00:00:00 2001 From: Melody Ma Date: Wed, 30 Sep 2026 16:10:06 +0800 Subject: [PATCH] [Cpp Parser] Support function param adjustments and update entity merge logic --- .../parameter_adjustment_dedup/BUILD | 26 +++ .../parameter_adjustment_dedup/expected.json | 203 ++++++++++++++++++ .../parameter_adjustment_dedup/functions.cpp | 22 ++ .../parameter_adjustment_dedup/functions.hpp | 25 +++ .../parameter_adjustment_dedup/run_test.rs | 19 ++ .../src/semantics/src/resolved_type.rs | 19 +- cpp/libclang/src/visitor/BUILD | 1 + .../src/visitor/src/callable_declaration.rs | 43 ++-- .../clang_adapter/exception_specification.rs | 21 ++ .../src/visitor/src/clang_adapter/mod.rs | 1 + cpp/libclang/src/visitor/src/context_ext.rs | 92 +++++++- .../src/visitor/src/function_visitor.rs | 10 +- .../src/visitor/src/types/resolver.rs | 11 + 13 files changed, 467 insertions(+), 26 deletions(-) create mode 100644 cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/BUILD create mode 100644 cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/expected.json create mode 100644 cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.cpp create mode 100644 cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp create mode 100644 cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/run_test.rs create mode 100644 cpp/libclang/src/visitor/src/clang_adapter/exception_specification.rs diff --git a/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/BUILD b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/BUILD new file mode 100644 index 00000000..0349a025 --- /dev/null +++ b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/BUILD @@ -0,0 +1,26 @@ +# ******************************************************************************* +# Copyright (c) 2026 Contributors to the Eclipse Foundation +# +# See the NOTICE file(s) distributed with this work for additional +# information regarding copyright ownership. +# +# This program and the accompanying materials are made available under the +# terms of the Apache License Version 2.0 which is available at +# https://www.apache.org/licenses/LICENSE-2.0 +# +# SPDX-License-Identifier: Apache-2.0 +# ******************************************************************************* +load("//cpp/libclang/integration_test:test_rules.bzl", "cpp_parser_integration_test") + +cc_library( + name = "parameter_adjustment_dedup", + srcs = ["functions.cpp"], + hdrs = ["functions.hpp"], + visibility = ["//cpp/libclang:__subpackages__"], +) + +cpp_parser_integration_test( + name = "test_parameter_adjustment_dedup", + expected_output = ["expected.json"], + target = ":parameter_adjustment_dedup", +) diff --git a/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/expected.json b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/expected.json new file mode 100644 index 00000000..1831f8a2 --- /dev/null +++ b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/expected.json @@ -0,0 +1,203 @@ +{ + "free_function_declarations": [ + { + "name": "alias_fn", + "enclosing_namespace_id": null, + "return_type": "void", + "parameters": [ + { + "name": "", + "param_type": "MyInt", + "is_variadic": false + } + ], + "template_parameters": null, + "source_location": { + "file": "cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp", + "line": 19 + } + }, + { + "name": "typedef_fn", + "enclosing_namespace_id": null, + "return_type": "void", + "parameters": [ + { + "name": "", + "param_type": "OldInt", + "is_variadic": false + } + ], + "template_parameters": null, + "source_location": { + "file": "cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp", + "line": 20 + } + }, + { + "name": "arr_fn", + "enclosing_namespace_id": null, + "return_type": "void", + "parameters": [ + { + "name": "", + "param_type": "int[5]", + "is_variadic": false + } + ], + "template_parameters": null, + "source_location": { + "file": "cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp", + "line": 21 + } + }, + { + "name": "incomplete_arr_fn", + "enclosing_namespace_id": null, + "return_type": "void", + "parameters": [ + { + "name": "", + "param_type": "int[]", + "is_variadic": false + } + ], + "template_parameters": null, + "source_location": { + "file": "cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp", + "line": 22 + } + }, + { + "name": "callback_fn", + "enclosing_namespace_id": null, + "return_type": "void", + "parameters": [ + { + "name": "callback", + "param_type": "void (int)", + "is_variadic": false + } + ], + "template_parameters": null, + "source_location": { + "file": "cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp", + "line": 23 + } + }, + { + "name": "callback_noexcept_identity", + "enclosing_namespace_id": null, + "return_type": "void", + "parameters": [ + { + "name": "", + "param_type": "void (*)()", + "is_variadic": false + } + ], + "template_parameters": null, + "source_location": { + "file": "cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp", + "line": 24 + } + }, + { + "name": "callback_noexcept_identity", + "enclosing_namespace_id": null, + "return_type": "void", + "parameters": [ + { + "name": "", + "param_type": "void (*)() noexcept", + "is_variadic": false + } + ], + "template_parameters": null, + "source_location": { + "file": "cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp", + "line": 25 + } + } + ], + "functions": [ + { + "id": { + "name": "alias_fn", + "scope": "Global" + }, + "kind": "Free", + "return_type": { + "Builtin": "void" + }, + "body": [] + }, + { + "id": { + "name": "typedef_fn", + "scope": "Global" + }, + "kind": "Free", + "return_type": { + "Builtin": "void" + }, + "body": [] + }, + { + "id": { + "name": "arr_fn", + "scope": "Global" + }, + "kind": "Free", + "return_type": { + "Builtin": "void" + }, + "body": [] + }, + { + "id": { + "name": "incomplete_arr_fn", + "scope": "Global" + }, + "kind": "Free", + "return_type": { + "Builtin": "void" + }, + "body": [] + }, + { + "id": { + "name": "callback_fn", + "scope": "Global" + }, + "kind": "Free", + "return_type": { + "Builtin": "void" + }, + "body": [] + }, + { + "id": { + "name": "callback_noexcept_identity", + "scope": "Global" + }, + "kind": "Free", + "return_type": { + "Builtin": "void" + }, + "body": [] + }, + { + "id": { + "name": "callback_noexcept_identity", + "scope": "Global" + }, + "kind": "Free", + "return_type": { + "Builtin": "void" + }, + "body": [] + } + ], + "types": {} +} diff --git a/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.cpp b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.cpp new file mode 100644 index 00000000..bb4e4dd4 --- /dev/null +++ b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.cpp @@ -0,0 +1,22 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ + +#include "functions.hpp" + +void alias_fn(int value) {} +void typedef_fn(int value) {} +void arr_fn(int* values) {} +void incomplete_arr_fn(int* values) {} +void callback_fn(void (*callback)(int)) {} +void callback_noexcept_identity(void (*)()) {} +void callback_noexcept_identity(void (*)() noexcept) {} diff --git a/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp new file mode 100644 index 00000000..861ef660 --- /dev/null +++ b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/functions.hpp @@ -0,0 +1,25 @@ +/******************************************************************************** + * Copyright (c) 2026 Contributors to the Eclipse Foundation + * + * See the NOTICE file(s) distributed with this work for additional + * information regarding copyright ownership. + * + * This program and the accompanying materials are made available under the + * terms of the Apache License Version 2.0 which is available at + * https://www.apache.org/licenses/LICENSE-2.0 + * + * SPDX-License-Identifier: Apache-2.0 + ********************************************************************************/ + +#pragma once + +using MyInt = int; +typedef int OldInt; + +void alias_fn(MyInt); +void typedef_fn(OldInt); +void arr_fn(int[5]); +void incomplete_arr_fn(int[]); +void callback_fn(void callback(int)); +void callback_noexcept_identity(void (*)()); +void callback_noexcept_identity(void (*)() noexcept); diff --git a/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/run_test.rs b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/run_test.rs new file mode 100644 index 00000000..025a0469 --- /dev/null +++ b/cpp/libclang/integration_test/function_cases/parameter_adjustment_dedup/run_test.rs @@ -0,0 +1,19 @@ +// ******************************************************************************* +// Copyright (c) 2026 Contributors to the Eclipse Foundation +// +// See the NOTICE file(s) distributed with this work for additional +// information regarding copyright ownership. +// +// This program and the accompanying materials are made available under the +// terms of the Apache License Version 2.0 which is available at +// +// +// SPDX-License-Identifier: Apache-2.0 +// ******************************************************************************* + +use test_framework::run_parser_case; + +#[test] +fn test_parameter_adjustment_dedup() { + run_parser_case(); +} diff --git a/cpp/libclang/src/semantics/src/resolved_type.rs b/cpp/libclang/src/semantics/src/resolved_type.rs index a8d055ef..a10d1577 100644 --- a/cpp/libclang/src/semantics/src/resolved_type.rs +++ b/cpp/libclang/src/semantics/src/resolved_type.rs @@ -28,6 +28,8 @@ pub enum ResolvedType { return_type: Box, parameter_types: Vec, is_variadic: bool, + #[serde(default, skip_serializing_if = "is_false")] + is_noexcept: bool, }, FunctionPointer(Box), FunctionReference(Box), @@ -173,12 +175,18 @@ impl ResolvedType { return_type, parameter_types, is_variadic, + is_noexcept, } => { let mut parameters = parameter_types.iter().map(Self::render).collect::>(); if *is_variadic { parameters.push("...".to_string()); } - format!("{}({})", return_type.render(), parameters.join(", ")) + let noexcept = if *is_noexcept { " noexcept" } else { "" }; + format!( + "{}({}){noexcept}", + return_type.render(), + parameters.join(", ") + ) } Self::FunctionPointer(inner) => render_function_wrapper(inner, "*"), Self::FunctionReference(inner) => render_function_wrapper(inner, "&"), @@ -198,11 +206,16 @@ impl ResolvedType { } } +fn is_false(value: &bool) -> bool { + !value +} + fn render_function_wrapper(inner: &ResolvedType, marker: &str) -> String { if let ResolvedType::Function { return_type, parameter_types, is_variadic, + is_noexcept, } = inner { let mut parameters = parameter_types @@ -212,8 +225,9 @@ fn render_function_wrapper(inner: &ResolvedType, marker: &str) -> String { if *is_variadic { parameters.push("...".to_string()); } + let noexcept = if *is_noexcept { " noexcept" } else { "" }; format!( - "{} ({marker})({})", + "{} ({marker})({}){noexcept}", return_type.render(), parameters.join(", ") ) @@ -335,6 +349,7 @@ mod tests { return_type: Box::new(ResolvedType::Builtin("void".to_string())), parameter_types: vec![ResolvedType::UserDefined("Engine".to_string())], is_variadic: false, + is_noexcept: false, }; assert_eq!( ResolvedType::FunctionPointer(Box::new(function)).render_for_display(), diff --git a/cpp/libclang/src/visitor/BUILD b/cpp/libclang/src/visitor/BUILD index 6af50ef6..8ace0653 100644 --- a/cpp/libclang/src/visitor/BUILD +++ b/cpp/libclang/src/visitor/BUILD @@ -16,6 +16,7 @@ rust_library( name = "visit_tu", srcs = [ "src/callable_declaration.rs", + "src/clang_adapter/exception_specification.rs", "src/clang_adapter/mod.rs", "src/clang_adapter/scope.rs", "src/clang_adapter/source_filter.rs", diff --git a/cpp/libclang/src/visitor/src/callable_declaration.rs b/cpp/libclang/src/visitor/src/callable_declaration.rs index 7df91c00..3aafc2e9 100644 --- a/cpp/libclang/src/visitor/src/callable_declaration.rs +++ b/cpp/libclang/src/visitor/src/callable_declaration.rs @@ -54,33 +54,35 @@ pub(crate) fn parse_callable_parameters(entity: &Entity) -> ParsedCallableParame let mut parameter_keys = Vec::new(); for argument in callable_arguments(entity) { - let raw_param_type = argument - .get_type() + let argument_type = argument.get_type(); + let raw_param_type = argument_type + .as_ref() .map(|ty| ty.get_display_name()) .unwrap_or_default(); - let resolved_type = argument.get_type().map(|ty| resolve_type(&ty)); + let resolved_type = argument_type.as_ref().map(resolve_type); + let signature_type = argument_type + .as_ref() + .map(|ty| resolve_type(&ty.get_canonical_type())); + let is_pack_expansion = raw_param_type.contains("..."); parameters.push(FunctionArgument { name: argument.get_name().unwrap_or_default(), param_type: Some(normalize_pack_expansion_type(&raw_param_type)), is_variadic: false, - is_pack_expansion: raw_param_type.contains("..."), + is_pack_expansion, }); if let Some(resolved_type) = resolved_type { - parameter_keys.push(CallableArgumentKey { - param_type: Some(render_resolved_type_for_signature_identity(&resolved_type)), - is_variadic: false, - is_pack_expansion: raw_param_type.contains("..."), - }); parameter_types.push(resolved_type); - } else { - parameter_keys.push(CallableArgumentKey { - param_type: None, - is_variadic: false, - is_pack_expansion: raw_param_type.contains("..."), - }); } + + parameter_keys.push(CallableArgumentKey { + param_type: signature_type + .as_ref() + .map(render_resolved_type_for_signature_identity), + is_variadic: false, + is_pack_expansion, + }); } if entity.get_type().is_some_and(|ty| ty.is_variadic()) { @@ -151,7 +153,16 @@ fn normalize_pack_expansion_type(param_type: &str) -> String { } fn render_resolved_type_for_signature_identity(resolved: &ResolvedType) -> String { - strip_top_level_cv_qualifiers_ref(resolved).render_for_display() + let resolved = strip_top_level_cv_qualifiers_ref(resolved); + match resolved { + ResolvedType::Array { element, .. } => { + ResolvedType::Pointer(element.clone()).render_for_display() + } + ResolvedType::Function { .. } => { + ResolvedType::FunctionPointer(Box::new(resolved.clone())).render_for_display() + } + _ => resolved.render_for_display(), + } } fn strip_top_level_cv_qualifiers_ref(resolved: &ResolvedType) -> &ResolvedType { diff --git a/cpp/libclang/src/visitor/src/clang_adapter/exception_specification.rs b/cpp/libclang/src/visitor/src/clang_adapter/exception_specification.rs new file mode 100644 index 00000000..7738900f --- /dev/null +++ b/cpp/libclang/src/visitor/src/clang_adapter/exception_specification.rs @@ -0,0 +1,21 @@ +// ******************************************************************************* +// Copyright (c) 2026 Contributors to the Eclipse Foundation +// +// See the NOTICE file(s) distributed with this work for additional +// information regarding copyright ownership. +// +// This program and the accompanying materials are made available under the +// terms of the Apache License Version 2.0 which is available at +// +// +// SPDX-License-Identifier: Apache-2.0 +// ******************************************************************************* + +use clang::ExceptionSpecification; + +pub(crate) fn has_plain_noexcept(exception_specification: Option) -> bool { + matches!( + exception_specification, + Some(ExceptionSpecification::BasicNoexcept) + ) +} diff --git a/cpp/libclang/src/visitor/src/clang_adapter/mod.rs b/cpp/libclang/src/visitor/src/clang_adapter/mod.rs index aef877d8..0290186f 100644 --- a/cpp/libclang/src/visitor/src/clang_adapter/mod.rs +++ b/cpp/libclang/src/visitor/src/clang_adapter/mod.rs @@ -13,6 +13,7 @@ //! Adapters from libclang entities and types to visitor-local concepts. +pub(crate) mod exception_specification; pub(crate) mod scope; pub(crate) mod source_filter; pub(crate) mod source_location; diff --git a/cpp/libclang/src/visitor/src/context_ext.rs b/cpp/libclang/src/visitor/src/context_ext.rs index 0bb714a7..8b13a591 100644 --- a/cpp/libclang/src/visitor/src/context_ext.rs +++ b/cpp/libclang/src/visitor/src/context_ext.rs @@ -14,7 +14,7 @@ use log::warn; use std::collections::BTreeMap; -use class_diagram::SimpleEntity; +use class_diagram::{EntityType, SimpleEntity}; pub trait EntityMapExt { fn insert_or_merge_type(&mut self, type_name: String, entity: SimpleEntity); @@ -51,7 +51,20 @@ fn merge_simple_entity(existing: &mut SimpleEntity, incoming: SimpleEntity) { existing.source_location = incoming.source_location.clone(); } - if existing.entity_type != incoming.entity_type { + // Replace the default Class classification only when the incoming entity + // exposes abstract semantics. + if matches!( + (existing.entity_type, incoming.entity_type), + ( + EntityType::Class, + EntityType::AbstractClass | EntityType::Interface + ) + ) { + existing.entity_type = incoming.entity_type; + existing.source_location = incoming.source_location.clone(); + } + + if !entity_types_are_compatible(existing.entity_type, incoming.entity_type) { warn!( "conflicting entity types while merging '{}': keeping {:?}, dropping {:?}", existing.id, existing.entity_type, incoming.entity_type @@ -66,6 +79,17 @@ fn merge_simple_entity(existing: &mut SimpleEntity, incoming: SimpleEntity) { extend_unique(&mut existing.relationships, incoming.relationships); } +fn entity_types_are_compatible(existing: EntityType, incoming: EntityType) -> bool { + existing == incoming || (is_class_entity_type(existing) && is_class_entity_type(incoming)) +} + +fn is_class_entity_type(entity_type: EntityType) -> bool { + matches!( + entity_type, + EntityType::Class | EntityType::Struct | EntityType::Interface | EntityType::AbstractClass + ) +} + fn extend_unique(existing: &mut Vec, incoming: Vec) { for item in incoming { if !existing.contains(&item) { @@ -174,4 +198,68 @@ mod tests { assert_eq!(widget.source_location, SourceLocation::new("second.h", 1)); assert_eq!(widget.enclosing_namespace_id.as_deref(), Some("util")); } + + #[test] + fn insert_or_merge_type_upgrades_a_class_to_an_abstract_class() { + let mut types = BTreeMap::new(); + types.insert_or_merge_type( + "svc::IThing".to_string(), + SimpleEntity { + id: "svc::IThing".to_string(), + name: "IThing".to_string(), + enclosing_namespace_id: Some("svc".to_string()), + entity_type: EntityType::Class, + source_location: SourceLocation::new("fwd.hpp", 2), + ..Default::default() + }, + ); + + types.insert_or_merge_type( + "svc::IThing".to_string(), + SimpleEntity { + id: "svc::IThing".to_string(), + name: "IThing".to_string(), + enclosing_namespace_id: Some("svc".to_string()), + entity_type: EntityType::AbstractClass, + source_location: SourceLocation::new("definition.hpp", 4), + ..Default::default() + }, + ); + + let thing = types.get("svc::IThing").expect("merged type should exist"); + assert_eq!(thing.entity_type, EntityType::AbstractClass); + assert_eq!( + thing.source_location, + SourceLocation::new("definition.hpp", 4) + ); + } + + #[test] + fn insert_or_merge_type_does_not_downgrade_an_interface() { + let mut types = BTreeMap::new(); + types.insert_or_merge_type( + "svc::IThing".to_string(), + SimpleEntity { + id: "svc::IThing".to_string(), + name: "IThing".to_string(), + enclosing_namespace_id: Some("svc".to_string()), + entity_type: EntityType::Interface, + ..Default::default() + }, + ); + + types.insert_or_merge_type( + "svc::IThing".to_string(), + SimpleEntity { + id: "svc::IThing".to_string(), + name: "IThing".to_string(), + enclosing_namespace_id: Some("svc".to_string()), + entity_type: EntityType::AbstractClass, + ..Default::default() + }, + ); + + let thing = types.get("svc::IThing").expect("merged type should exist"); + assert_eq!(thing.entity_type, EntityType::Interface); + } } diff --git a/cpp/libclang/src/visitor/src/function_visitor.rs b/cpp/libclang/src/visitor/src/function_visitor.rs index a3312100..9a6b29fc 100644 --- a/cpp/libclang/src/visitor/src/function_visitor.rs +++ b/cpp/libclang/src/visitor/src/function_visitor.rs @@ -15,7 +15,7 @@ //! Preserves structured calls, branches, and loops for supported AST shapes, //! and falls back to conservative traversal for unsupported control-flow forms. -use clang::{Entity, EntityKind, ExceptionSpecification}; +use clang::{Entity, EntityKind}; use class_diagram::{FreeFunctionDecl, Method, MethodModifier}; use cpp_semantics::{ BodyItem, BranchCase, FunctionDef, FunctionId, FunctionKind, GuardExpression, LoopKind, @@ -26,6 +26,7 @@ use std::collections::HashSet; use crate::callable_declaration::{ parse_callable_parameters, parse_callable_return_type, parse_template_parameters, }; +use crate::clang_adapter::exception_specification::has_plain_noexcept; use crate::clang_adapter::scope::{ callable_scope, has_translation_unit_local_linkage, namespace_id, }; @@ -160,11 +161,8 @@ impl FunctionVisitor { .any(|token| token.get_spelling() == "noexcept") }); - let is_noexcept_method = has_noexcept_token - && matches!( - entity.get_exception_specification(), - Some(ExceptionSpecification::BasicNoexcept) - ); + let is_noexcept_method = + has_noexcept_token && has_plain_noexcept(entity.get_exception_specification()); let return_type = if matches!(kind, FunctionKind::Constructor | FunctionKind::Destructor) { None diff --git a/cpp/libclang/src/visitor/src/types/resolver.rs b/cpp/libclang/src/visitor/src/types/resolver.rs index f63969f3..55ca84d3 100644 --- a/cpp/libclang/src/visitor/src/types/resolver.rs +++ b/cpp/libclang/src/visitor/src/types/resolver.rs @@ -18,6 +18,7 @@ use clang::{Entity, EntityKind, Type, TypeKind}; use cpp_semantics::ResolvedType; +use crate::clang_adapter::exception_specification::has_plain_noexcept; use crate::clang_adapter::source_filter; pub(crate) fn resolve_type(original: &Type) -> ResolvedType { @@ -83,6 +84,15 @@ fn resolve_unqualified_type(original: &Type, canonical: &Type) -> ResolvedType { ), size: original.get_size(), }, + TypeKind::IncompleteArray => ResolvedType::Array { + element: Box::new( + original + .get_element_type() + .map(|element| resolve_type(&element)) + .unwrap_or_else(|| unknown(original)), + ), + size: None, + }, // ===== user-defined / template ===== // Named types (including aliases/templates) are resolved through decl-aware fallback. @@ -122,6 +132,7 @@ fn resolve_function_type(original: &Type) -> ResolvedType { return_type: Box::new(return_type), parameter_types, is_variadic: original.is_variadic(), + is_noexcept: has_plain_noexcept(original.get_exception_specification()), } }