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