[PATCH] D104964: [ms] [llvm-ml] Add support for numeric built-in symbols

Eric Astor via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 21 06:55:25 PDT 2021


epastor added inline comments.


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:3516
+      Sym->isVariable() ? dyn_cast_or_null<MCConstantExpr>(
+                              Sym->getVariableValue(/*SetUsed*/ false))
+                        : nullptr;
----------------
thakis wrote:
> Why is this changing? If it's intentional, nit: Named parameter comment style in LLVM is `/*SetUsed=*/false` (ie with `=` and without space)
We're removing `Var.NumericValue` because it breaks the parallels between Variables and BuiltinSymbols. Instead, we rely on the value stored in the symbol itself as authoritative - and if it's not an MCConstantExpr, then it's definitely not a MASM variable.

Fixed the named parameter comment style, though I believe this was copied from AsmParser.cpp originally...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D104964/new/

https://reviews.llvm.org/D104964



More information about the llvm-commits mailing list