[2bb4a01] | 1 | # Porting notes for new AST # |
---|
| 2 | |
---|
| 3 | ## Pointer Types ## |
---|
| 4 | * raw pointer `T*` is used for construction, but not storage |
---|
[fdbd4fd] | 5 | * `ast::ptr_base<T,R>` is a pointer to AST node `T` with reference type `R` |
---|
| 6 | * specialization: strong pointer `ast::ptr<T>` is used for an ownership relationship |
---|
| 7 | * specialization: weak pointer `ast::readonly<T>` is used for an observation relationship |
---|
[54e41b3] | 8 | * added `ast::ptr_base<T,R>::as<S>()` with same semantics as `dynamic_cast<S*>(p)` |
---|
| 9 | * added `N * ast::ptr_base<N,R>::set_and_mutate( const N * n )` |
---|
| 10 | * takes ownership of `n`, then returns a mutable version owned by this pointer |
---|
| 11 | * Some debate on whether this is a good approach: |
---|
| 12 | * makes an easy path to cloning, which we were trying to eliminate |
---|
| 13 | * counter-point: these are all mutating clones rather than lifetime-preserving clones, and thus "necessary" (for some definition) |
---|
| 14 | * existing uses: |
---|
| 15 | * `VariableExpr::VariableExpr`, `UntypedExpr::createDeref` |
---|
| 16 | * both involve grabbing a type from elsewhere and making an `lvalue` copy of it |
---|
| 17 | * could potentially be replaced by a view class something like this: |
---|
| 18 | ``` |
---|
| 19 | template<unsigned Quals> |
---|
| 20 | class AddQualifiersType final : public Type { |
---|
| 21 | readonly<Type> base; |
---|
| 22 | // ... |
---|
| 23 | }; |
---|
| 24 | ``` |
---|
| 25 | * requires all `qualifiers` use (and related helpers) to be virtual, non-zero overhead |
---|
| 26 | * also subtle semantic change, where mutations to the source decl now change the viewing expression |
---|
[2bb4a01] | 27 | |
---|
| 28 | ## Visitors ## |
---|
| 29 | * `Visitor` and `Mutator` are combined into a single `ast::Visitor` class |
---|
[6d51bd7] | 30 | * Base nodes now override `const Node * accept( Visitor & v ) const = 0` with, e.g. `const Stmt * accept( Visitor & v ) const override = 0` |
---|
[2bb4a01] | 31 | * `PassVisitor` is replaced with `ast::Pass` |
---|
[3aec25f] | 32 | * Most one shot uses can use `ast::Pass::run` and `ast::Pass::read`. |
---|
| 33 | |
---|
| 34 | `WithConstTypeSubstitution` |
---|
| 35 | * `env` => `typeSubs` |
---|
[2bb4a01] | 36 | |
---|
| 37 | ## Structural Changes ## |
---|
| 38 | `CodeLocation` has been devolved from `BaseSyntaxNode` to `ast::ParseNode` |
---|
| 39 | * excludes `ast::Type` from carrying location information |
---|
| 40 | * `CodeLocation` is a mandatory constructor field for `ast::ParseNode` |
---|
| 41 | * all subclass constructors must fill it; by convention, from their first argument |
---|
| 42 | |
---|
[6f4b7f2] | 43 | `N->print(std::ostream&)` is a visitor now |
---|
| 44 | * `Declaration::printShort` is also integrated |
---|
[2bb4a01] | 45 | |
---|
| 46 | `clone` is private to `Node` now |
---|
| 47 | * still needs to be overriden to return appropriate type |
---|
[6d51bd7] | 48 | * e.g. `private: virtual Stmt * clone() const override = 0;` |
---|
| 49 | * because friendship is not inherited, all implementations of clone need |
---|
| 50 | /// Must be copied in ALL derived classes |
---|
| 51 | template<typename node_t> |
---|
[87701b6] | 52 | friend node_t * mutate(const node_t * node); |
---|
[d1ee9ec] | 53 | template<typename node_t> |
---|
| 54 | friend node_t * shallowCopy(const node_t * node); |
---|
| 55 | or equilant. |
---|
| 56 | * You should use the `mutate` function where possible as it avoids extra copies. |
---|
| 57 | * If you must copy use `shallowCopy` or `deepCopy` as required. |
---|
[2bb4a01] | 58 | |
---|
| 59 | All leaves of the `Node` inheritance tree are now declared `final` |
---|
| 60 | * e.g. `class CompoundStmt final : public Stmt` |
---|
| 61 | * allows compiler to optimize virtual calls to static calls if given static type |
---|
| 62 | |
---|
[9e1d485] | 63 | Pulled `FuncSpecifiers`, `StorageClasses`, `CVQualifiers` out of `Type` into their own headers |
---|
[2bb4a01] | 64 | * Made `BFCommon` a `MakeBitfield` macro in its own header |
---|
| 65 | * added default and field-init constructors to macro |
---|
| 66 | |
---|
| 67 | Prefer move semantics for containers passed to node constructors |
---|
| 68 | |
---|
| 69 | ## Code Style ## |
---|
| 70 | |
---|
| 71 | ### Files ### |
---|
| 72 | * Headers have a `.hpp` suffix |
---|
| 73 | * Source code has a `.cpp` suffix |
---|
| 74 | * All source has the project-standard leading and trailing comments |
---|
| 75 | * prefer `#pragma once` over `#ifdef` guards |
---|
| 76 | * namespaces that cover entire files don't get indented |
---|
[9b4f329] | 77 | * The general node headers only `#include "Fwd.hpp"` if they can get away with it |
---|
| 78 | * Anything that needs definitions goes in the .cpp file |
---|
| 79 | * `Type.hpp` includes `Decl.hpp` so that it knows the `AggregateDecl` subclasses for `ReferenceToType::aggr()` overloads |
---|
[2bb4a01] | 80 | |
---|
| 81 | ### Documentation ### |
---|
| 82 | * class, method, and field comments should use doxygen-style `///` prefix |
---|
| 83 | * should be included on all classes |
---|
| 84 | * should be included on any method declaration that doesn't have an obvious behaviour from either naming convention (e.g. constructor, print operator, implement visitor pattern) or an inline implementation |
---|
| 85 | * use explanatory comments with `//` wherever appropriate |
---|
| 86 | * older comments should be maintained in porting process wherever possible |
---|
| 87 | |
---|
| 88 | ### Naming ### |
---|
| 89 | * Preserve names from previous AST whenever reasonable, and get team consensus on any changes. |
---|
| 90 | * Strong justification required for private fields |
---|
[a300e4a] | 91 | * No `get_` prefix on getters (including for generated fields) |
---|
[fdbd4fd] | 92 | * exception is `DeclWithType::get_type()` |
---|
[2bb4a01] | 93 | * Notable changes: |
---|
| 94 | * for concision and consistency with subclasses: |
---|
| 95 | * `Declaration` => `ast::Decl` |
---|
[14cebb7a] | 96 | * `DeclarationWithType` => `ast::DeclWithType` |
---|
[2bb4a01] | 97 | * `Expression` => `ast::Expr` |
---|
| 98 | * `Initializer` => `ast::Init` |
---|
| 99 | * `Statement` => `ast::Stmt` |
---|
[9e1d485] | 100 | * any field names should follow a similar renaming |
---|
[2bb4a01] | 101 | * because they don't really belong to `Type` (and for consistency with `Linkage::Spec`): |
---|
| 102 | * `Type::StorageClasses` => `ast::Storage::Classes` |
---|
| 103 | * `Type::Extern` etc. => `ast::Storage::Extern` etc. |
---|
[9e1d485] | 104 | * `Type::FuncSpecifiers` => `ast::Function::Specs` |
---|
| 105 | * `Type::Inline` etc. => `ast::Function::Inline` etc. |
---|
| 106 | * `Type::Qualifiers` => `ast::CV::Qualifiers` |
---|
| 107 | * `Type::Const` etc. => `ast::CV::Const` |
---|
| 108 | * couldn't break name-dependency loop without pulling `Qualifiers` out of `Type` |
---|
[2bb4a01] | 109 | * `LinkageSpec::Spec` => `ast::Linkage::Spec` |
---|
| 110 | * `LinkageSpec::Mangle` etc. => `ast::Linkage::Mangle` etc. |
---|
| 111 | * `LinkageSpec::linkageUpdate` => `ast::Linkage::update` |
---|
| 112 | * `LinkageSpec::linkageName` => `ast::Linkage::name` |
---|
| 113 | * `LinkageSpec::isMangled(Spec)` etc. => `Spec.is_mangled` etc. |
---|
| 114 | * `LinkageSpec::Intrinsic` etc. => `ast::Linkage::Intrinsic` etc. |
---|
[d76c588] | 115 | * Boolean flags to `SymTab::Mangler::mangle` are now a `SymTab::Mangle::Mode` struct |
---|
| 116 | * uses `bitfield` |
---|
| 117 | * Because `Indexer` isn't a terribly evocative name: |
---|
| 118 | * `SymTab::Indexer` => `ast::SymbolTable` |
---|
| 119 | * `SymTab/Indexer.{h,cc}` => `AST/SymbolTable.{hpp,cpp}` |
---|
[0e42794] | 120 | * `WithIndexer` => `WithSymbolTable` |
---|
[d76c588] | 121 | * `indexer` => `symTab` |
---|
| 122 | * `IdData::deleteStmt` => `IdData::deleter` |
---|
| 123 | * `lookupId()` now returns a vector rather than an out-param list |
---|
[0e42794] | 124 | * To avoid name collisions: |
---|
| 125 | * `SymTab::Mangler` => `Mangle` |
---|
[d76c588] | 126 | * `ResolvExpr::TypeEnvironment` => `ast::TypeEnvironment` |
---|
| 127 | * in `AST/TypeEnvironment.hpp` |
---|
[9e1d485] | 128 | * Boolean constructor parameters get replaced with a dedicated flag enum: |
---|
| 129 | * e.g. `bool isVarLen;` => `enum LengthFlag { FixedLen, VariableLen };` `LengthFlag isVarLen;` |
---|
| 130 | * field can be *read* in the existing boolean contexts, but requires documentation to write |
---|
| 131 | * suggest naming all flag enums `FooFlag` to hint at boolean nature |
---|
[2bb4a01] | 132 | |
---|
| 133 | ## Specific Nodes ## |
---|
[489bacf] | 134 | `Attribute` |
---|
| 135 | * `parameters` => `params` |
---|
| 136 | |
---|
[2bb4a01] | 137 | `Decl` |
---|
| 138 | * `storageClasses` => `storage` |
---|
| 139 | * `declFromId()` => `fromId()` |
---|
| 140 | * not 100% sure about the return type here... |
---|
| 141 | |
---|
| 142 | `DeclWithType` |
---|
| 143 | * When `SymTab::Validate::Pass2` is rewritten, update comment on `mangleName` with new name of pass |
---|
[a300e4a] | 144 | * `get_scopedMangleName()` => `scopedMangleName()` |
---|
[77a3f41] | 145 | * `get_type()` now returns `const Type*` so can't be inadvertently mutated |
---|
| 146 | * still with `get_` name so it doesn't conflict with subclass field names |
---|
| 147 | |
---|
| 148 | `ObjectDecl` |
---|
| 149 | * changed constructor parameter order for better defaults |
---|
| 150 | * allows `newObject` as just default settings |
---|
[a300e4a] | 151 | |
---|
[3aec25f] | 152 | `FunctionDecl` |
---|
| 153 | * `params` and `returns` added. |
---|
| 154 | * Contain the declarations of the parameters and return variables. |
---|
| 155 | * Types should match (even be shared with) the fields of `type`. |
---|
| 156 | |
---|
[54e41b3] | 157 | `NamedTypeDecl` |
---|
| 158 | * `parameters` => `params` |
---|
| 159 | |
---|
[360b2e13] | 160 | `TypeDecl` |
---|
[9e1d485] | 161 | * moved `TypeDecl::Kind` to `ast::TypeVar::Kind` |
---|
[360b2e13] | 162 | |
---|
[54e41b3] | 163 | `AggregateDecl` |
---|
| 164 | * `parameters` => `params` |
---|
| 165 | |
---|
[3aec25f] | 166 | `StructDecl` |
---|
| 167 | * `makeInst` replaced by better constructor on `StructInstType`. |
---|
| 168 | |
---|
[2bb4a01] | 169 | `Expr` |
---|
[79f7875] | 170 | * Merged `inferParams`/`resnSlots` into union, as suggested by comment in old version |
---|
| 171 | * does imply get_/set_ API, and some care about moving backward |
---|
[54e41b3] | 172 | * added constructor that sets result, for benefit of types that set it directly |
---|
| 173 | |
---|
| 174 | `ApplicationExpr` |
---|
| 175 | * `function` => `func` |
---|
| 176 | |
---|
| 177 | `UntypedExpr` |
---|
| 178 | * `function` => `func` |
---|
| 179 | * removed `begin_args()` in favour of `args.begin()` |
---|
| 180 | |
---|
| 181 | `ConstantExpr` |
---|
| 182 | * inlined features of `Constant`, never used elsewhere, so removed `Constant` |
---|
| 183 | * `Constant Constant::from_int(int)` etc. => `ConstantExpr * ConstantExpr::from_int(CodeLocation, int)` |
---|
| 184 | * allocates new `ConstantExpr`, consistent with all existing uses |
---|
| 185 | |
---|
| 186 | `SizeofExpr`, `AlignofExpr` |
---|
| 187 | * `isType` deprecated in favour of boolean check on `type` |
---|
| 188 | * all existing uses assume `type` set if true and don't use `expr` |
---|
| 189 | |
---|
| 190 | `LogicalExpr` |
---|
| 191 | * un-defaulted constructor parameter determining `&&` or `||` |
---|
[2bb4a01] | 192 | |
---|
[9b4f329] | 193 | `CompoundLiteralExpr` |
---|
| 194 | * `initializer` => `init` |
---|
| 195 | |
---|
| 196 | `RangeExpr` |
---|
| 197 | * removed `set_low`, `set_high` due to disuse |
---|
| 198 | |
---|
| 199 | `TupleIndexExpr` |
---|
| 200 | * removed `set_tuple`, `set_index` due to disuse |
---|
| 201 | |
---|
| 202 | `GenericExpr` |
---|
| 203 | * `Association::isDefault` removed: `! type` is equivalent |
---|
| 204 | |
---|
| 205 | `StmtExpr` |
---|
| 206 | * `statements` => `stmts` |
---|
| 207 | |
---|
[9e1d485] | 208 | `Init` |
---|
| 209 | * `bool maybeConstruct` => `enum ConstructFlag { DoConstruct, MaybeConstruct }` |
---|
| 210 | |
---|
[2bb4a01] | 211 | `Label` |
---|
| 212 | * `get_statement()` exclusively used for code location, replaced with `CodeLocation` field |
---|
| 213 | |
---|
[1e97287] | 214 | `CaseStmt` |
---|
| 215 | * `_isDefault` has been removed |
---|
| 216 | * `isDefault` calculates value from `cond` |
---|
| 217 | * default may not have a condition. I believe case (!default) requires a condition. |
---|
| 218 | |
---|
| 219 | `BranchStmt` |
---|
| 220 | * `Type` -> `Kind` and `type` -> `kind` |
---|
| 221 | * Constructors no longer throw SemanticErrorException: |
---|
| 222 | * `label` constructor claims it is now considered a syntax error, replaced with assert. |
---|
| 223 | * `computedTarget` constructor assumes `Goto`, other check would have SegFaulted. |
---|
| 224 | |
---|
| 225 | `TryStmt` |
---|
| 226 | * `block` -> `body` and `finallyBlock` -> `finally` |
---|
| 227 | |
---|
[6f4b7f2] | 228 | `ThrowStmt` `CatchStmt` |
---|
| 229 | * moved `Kind` enums to shared `ast::ExceptionKind` enum |
---|
| 230 | |
---|
[1e97287] | 231 | `FinallyStmt` |
---|
| 232 | * `block` -> `body` |
---|
| 233 | |
---|
[2bb4a01] | 234 | `CompoundStmt` |
---|
| 235 | * Still a `std::list` for children, rather than `std::vector` |
---|
[fdbd4fd] | 236 | * allows more-efficient splicing for purposes of later code generation |
---|
| 237 | |
---|
| 238 | `Type` |
---|
[9e1d485] | 239 | * `CV::Qualifiers` moved to end of constructor parameter list, defaulted to `{}` |
---|
[54e41b3] | 240 | * removed getter, setter in favour of public `qualifiers` field |
---|
[9e1d485] | 241 | * `ReferenceToType` puts a defaulted list of attributes after qualifiers |
---|
[fdbd4fd] | 242 | * `forall` field split off into `ParameterizedType` subclass |
---|
| 243 | * any type that needs it can inherit from `ParameterizedType` |
---|
[9e1d485] | 244 | * currently `FunctionType`, `ReferenceToType` |
---|
[fdbd4fd] | 245 | * `get_qualifiers()` replaced with accessor `qualifiers()` and mutator `set_qualifiers()` |
---|
[9e1d485] | 246 | * `get_const()` etc. replaced with `is_const()` etc. variants |
---|
| 247 | * `referenceDepth()` now returns `unsigned` rather than `int` |
---|
[fdbd4fd] | 248 | * A number of features only supported on aggregates pushed down to `ReferenceToType`: |
---|
| 249 | * `attributes`: per docs [1] GCC only supports type attributes on aggregates and typedefs |
---|
| 250 | * suggest adding a `TypeWithAttributes` wrapper type if this proves insufficient |
---|
[9e1d485] | 251 | * `getAggr()` => `aggr()` |
---|
| 252 | * also now returns `const AggregateDecl *` |
---|
[d8938622] | 253 | * `genericSubstitution()` moved to own visitor in `AST/GenericSubstitution.hpp` |
---|
[60aaa51d] | 254 | * subsumes old `makeGenericSubstitution()` |
---|
[fdbd4fd] | 255 | |
---|
| 256 | `BasicType` |
---|
| 257 | * **TODO** move `kind`, `typeNames` into code generator |
---|
| 258 | |
---|
[3aec25f] | 259 | `ReferenceToType` => `BaseInstType` |
---|
[9e1d485] | 260 | * deleted `get_baseParameters()` from children |
---|
[54e41b3] | 261 | * replace with `aggr() ? aggr()->params : nullptr` |
---|
| 262 | * `parameters` => `params` |
---|
[9e1d485] | 263 | * hoisted `lookup` implementation into parent, made non-virtual |
---|
| 264 | * also changed to return vector rather than filling; change back if any great win for reuse |
---|
| 265 | * `baseStruct` etc. renamed to `base` |
---|
| 266 | |
---|
| 267 | `PointerType`/`ArrayType` |
---|
| 268 | * `is_array()` => `isArray()` |
---|
| 269 | * `bool isVarLen;` => `enum LengthFlag { FixedLen, VariableLen }; LengthFlag isVarLen;` |
---|
| 270 | * `bool isStatic;` => `enum DimensionFlag { DynamicDim, StaticDim }; DimensionFlag isStatic;` |
---|
| 271 | |
---|
| 272 | `FunctionType` |
---|
[54e41b3] | 273 | * `returnVals` => `returns` |
---|
| 274 | * `parameters` => `params` |
---|
[3aec25f] | 275 | * Both now just point at types. |
---|
[9e1d485] | 276 | * `bool isVarArgs;` => `enum ArgumentFlag { FixedArgs, VariableArgs }; ArgumentFlag isVarArgs;` |
---|
| 277 | |
---|
[3aec25f] | 278 | `SueInstType` |
---|
| 279 | * Template class, with specializations and using to implement some other types: |
---|
| 280 | * `StructInstType`, `UnionInstType` & `EnumInstType` |
---|
| 281 | |
---|
[9e1d485] | 282 | `TypeInstType` |
---|
| 283 | * `bool isFtype` => `TypeVar::Kind kind` |
---|
| 284 | |
---|
| 285 | `TypeofType` |
---|
| 286 | * `bool is_basetypeof` => `enum Kind { Typeof, Basetypeof } kind;` |
---|
| 287 | |
---|
| 288 | `TupleType` |
---|
| 289 | * removed `value_type` typedef due to likely error |
---|
| 290 | * if readded, should be `const Type *` |
---|
| 291 | |
---|
| 292 | `AttrType` |
---|
[69bafd2] | 293 | * did not port due to deprecation of feature |
---|
| 294 | * feature is `type@thing` e.g. `int@MAX` |
---|
[9e1d485] | 295 | |
---|
[d76c588] | 296 | `referenceToRvalueConversion` |
---|
| 297 | * now returns `const Expr *` rather than mutating argument |
---|
| 298 | |
---|
| 299 | `printAssertionSet`, `printOpenVarSet` |
---|
| 300 | * `ostream &` now first argument, for consistency |
---|
| 301 | |
---|
| 302 | `EqvClass` |
---|
| 303 | * `type` => `bound` |
---|
| 304 | |
---|
| 305 | `TypeEnvironment` |
---|
| 306 | * `makeSubstitution()` => `writeToSubstitution()` |
---|
| 307 | * `isEmpty()` => `empty()` |
---|
| 308 | * removed `clone()` in favour of explicit copies |
---|
| 309 | |
---|
| 310 | `occurs` |
---|
| 311 | * moved to be helper function in `TypeEnvironment.cpp` (its only use) |
---|
| 312 | |
---|
[f474e91] | 313 | `WidenMode` |
---|
| 314 | * changed `widenFirst`, `widenSecond` => `first`, `second` |
---|
| 315 | * changed `WidenMode widenMode` => `WidenMode widen` |
---|
| 316 | |
---|
[99d4584] | 317 | `Alternative` => `Candidate` |
---|
| 318 | * `openVars` => `open` |
---|
| 319 | |
---|
[432ce7a] | 320 | `ExplodedActual` => `ExplodedArg` |
---|
| 321 | * `ExplodedActual.h` => `ExplodedArg.hpp` |
---|
| 322 | |
---|
[9d5089e] | 323 | `polyCost` |
---|
| 324 | * switched order of `env`, `symtab` parameters for better consistency |
---|
| 325 | |
---|
[898ae07] | 326 | `findMinCost` |
---|
| 327 | * pulled out conversion cost promotion into separate `promoteCvtCost` function |
---|
| 328 | |
---|
[b69233ac] | 329 | `resolveAssertions` => `satisfyAssertions` |
---|
| 330 | * `ResolveAssertions.h` => `SatisfyAssertions.hpp` |
---|
| 331 | * `Resn*` => `Sat*` |
---|
| 332 | |
---|
[fdbd4fd] | 333 | [1] https://gcc.gnu.org/onlinedocs/gcc-9.1.0/gcc/Type-Attributes.html#Type-Attributes |
---|
| 334 | |
---|