Skip to content
This repository has been archived by the owner on Jun 3, 2021. It is now read-only.

[WIP] Parse qualifiers correctly #508

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
82 changes: 63 additions & 19 deletions src/analyze/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,16 @@ impl PureAnalyzer {
let sc = original.storage_class.unwrap_or(StorageClass::Auto);
let mut decls = Vec::new();
for d in declaration.declarators {
// NOTE: this clone is cheap, but making it explicit so it's clear this is mutable
// This needs to be separate because qualifiers are specific to a single declarator,
// e.g. `int *const p, i;`
let mut qualifiers = original.qualifiers;
let mut ctype =
self.parse_declarator(original.ctype.clone(), d.data.declarator.decl, d.location);
self.parse_declarator(original.ctype.clone(), d.data.declarator.decl, &mut qualifiers, d.location);

if !ctype.is_function() && original.qualifiers.func != FunctionQualifiers::default() {
if !ctype.is_function() && qualifiers.func != FunctionQualifiers::default() {
self.err(
SemanticError::FuncQualifiersNotAllowed(original.qualifiers.func),
SemanticError::FuncQualifiersNotAllowed(qualifiers.func),
d.location,
);
}
Expand Down Expand Up @@ -226,7 +230,7 @@ impl PureAnalyzer {
let symbol = Variable {
ctype,
id,
qualifiers: original.qualifiers,
qualifiers,
storage_class: sc,
};
let symbol = self.declare(symbol, init.is_some(), d.location);
Expand Down Expand Up @@ -278,7 +282,7 @@ impl PureAnalyzer {
location: Location,
) -> ParsedType {
let mut specs = self.parse_specifiers(specifiers, location);
specs.ctype = self.parse_declarator(specs.ctype, declarator, location);
specs.ctype = self.parse_declarator(specs.ctype, declarator, &mut specs.qualifiers, location);

if !specs.ctype.is_function() && specs.qualifiers.func != FunctionQualifiers::default() {
self.err(
Expand Down Expand Up @@ -590,12 +594,6 @@ impl PureAnalyzer {
location: Location,
) -> Vec<Variable> {
let parsed_type = self.parse_specifiers(members.specifiers, location);
if parsed_type.qualifiers.has_func_qualifiers() {
self.err(
SemanticError::FuncQualifiersNotAllowed(parsed_type.qualifiers.func),
location,
);
}

let mut parsed_members = Vec::new();
// A member of a structure or union may have any complete object type other than a variably modified type.
Expand All @@ -606,7 +604,9 @@ impl PureAnalyzer {
None => continue,
Some(d) => d,
};
let ctype = match self.parse_declarator(parsed_type.ctype.clone(), decl.decl, location)
// NOTE: need to make a copy in case there are multiple declarators: `int *const p, i;`
let mut qualifiers = parsed_type.qualifiers;
let ctype = match self.parse_declarator(parsed_type.ctype.clone(), decl.decl, &mut qualifiers, location)
{
Type::Void => {
// TODO: catch this error for types besides void?
Expand All @@ -615,9 +615,16 @@ impl PureAnalyzer {
}
other => other,
};
// TODO: does this really belong here`
if qualifiers.has_func_qualifiers() {
self.err(
SemanticError::FuncQualifiersNotAllowed(qualifiers.func),
location,
);
}
let mut symbol = Variable {
storage_class: StorageClass::Auto,
qualifiers: parsed_type.qualifiers,
qualifiers,
ctype,
id: decl.id.expect("struct members should have an id"),
};
Expand Down Expand Up @@ -834,11 +841,17 @@ impl PureAnalyzer {
/// The parser generated a linked list `DeclaratorType`,
/// which we now transform into the recursive `Type`.
///
/// Note the special handling of qualifiers. In the AST, we store
/// `int *const p` with the `const` as part of the pointer declarator.
/// But this code actually means that the variable binding is const and the pointed-to data is mutable.
/// So we need to switch it.
///
/// 6.7.6 Declarators
fn parse_declarator(
&mut self,
current: Type,
decl: ast::DeclaratorType,
ptr_qualifiers: &mut Qualifiers,
location: Location,
) -> Type {
use crate::data::ast::DeclaratorType::*;
Expand All @@ -848,15 +861,17 @@ impl PureAnalyzer {
match decl {
End => current,
Pointer { to, qualifiers } => {
use std::mem;
use UnitSpecifier::*;

let inner = self.parse_declarator(current, *to, location);
// `**const p` still means `p` is const, not `*p` or `**p`
// TODO: is this correct for `*const *p` and `const **p`?
let inner = self.parse_declarator(current, *to, ptr_qualifiers, location);
// we reuse `count_specifiers` even though we really only want the qualifiers
let (counter, compounds) =
count_specifiers(qualifiers, &mut self.error_handler, location);
// *const volatile
// TODO: this shouldn't allow `inline` or `_Noreturn`
let qualifiers = Qualifiers {
let binding_qualifiers = Qualifiers {
c_const: counter.get(&Const).is_some(),
volatile: counter.get(&Volatile).is_some(),
func: FunctionQualifiers {
Expand All @@ -867,14 +882,23 @@ impl PureAnalyzer {
for &q in counter.keys() {
if !q.is_qualifier() {
// *extern
// TODO: this location is wrong, it points to the declarator and not the keyword
self.err(SemanticError::NotAQualifier(q.into()), location);
}
}
for spec in compounds {
// *struct s {}
self.err(SemanticError::NotAQualifier(spec), location);
}
Type::Pointer(Box::new(inner), qualifiers)
// disallow `int (*inline f)();`
if binding_qualifiers.func != FunctionQualifiers::default() {
self.err(SemanticError::FuncQualifiersNotAllowed(binding_qualifiers.func), location);
// disallow `inline int (*f)();`
} else if ptr_qualifiers.func != FunctionQualifiers::default() {
self.err(SemanticError::FuncQualifiersNotAllowed(ptr_qualifiers.func), location);
}
let ptr_quals = mem::replace(ptr_qualifiers, binding_qualifiers);
Type::Pointer(Box::new(inner), ptr_quals)
}
Array { of, size } => {
// int a[5]
Expand All @@ -888,7 +912,7 @@ impl PureAnalyzer {
// int a[]
ArrayType::Unbounded
};
let of = self.parse_declarator(current, *of, location);
let of = self.parse_declarator(current, *of, ptr_qualifiers, location);
// int a[]()
if let Type::Function(_) = &of {
self.err(SemanticError::ArrayStoringFunction(of.clone()), location);
Expand All @@ -897,7 +921,8 @@ impl PureAnalyzer {
}
Function(func) => {
// TODO: give a warning for `const int f();` somewhere
let return_type = self.parse_declarator(current, *func.return_type, location);
// TODO: should we do something else with `ptr_qualifiers`?
let return_type = self.parse_declarator(current, *func.return_type, ptr_qualifiers, location);
match &return_type {
// int a()[]
Type::Array(_, _) => self.err(
Expand Down Expand Up @@ -1951,4 +1976,23 @@ int main() {
"extern int f(void) {\n return (int)(1);\n}\n",
);
}

#[test]
fn qualifiers() {
assert!(decl("int (*inline f)();").is_err());
assert!(decl("inline int (*f)();").is_err());
assert_no_change("const int *p;");
assert_no_change("int *const p;");

let const_binding = decl("int *const p;").unwrap().symbol.get();
assert!(const_binding.qualifiers.c_const);
assert_eq!(const_binding.ctype, Type::Pointer(Box::new(Type::Int(true)), Qualifiers::default()));

let const_ptr = decl("const int *p;").unwrap().symbol.get();
assert!(!const_ptr.qualifiers.c_const); // this is the binding, not the pointer qualifier
assert_eq!(const_binding.ctype, Type::Pointer(Box::new(Type::Int(true)), Qualifiers {
c_const: true,
..Qualifiers::default()
}));
}
}
2 changes: 1 addition & 1 deletion src/data/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ impl Display for FunctionQualifiers {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match (self.inline, self.no_return) {
(true, true) => write!(f, "{} {}", Keyword::Inline, Keyword::NoReturn),
(true, false) => write!(f, "{}", Keyword::NoReturn),
(true, false) => write!(f, "{}", Keyword::Inline),
(false, true) => write!(f, "{}", Keyword::NoReturn),
(false, false) => Ok(()),
}
Expand Down