[PATCH] D92503: [ms] [llvm-ml] Fix case-sensitivity for variables and textmacros

[Eric Astor via Phabricator via llvm-commits]

epastor added inline comments.


================
Comment at: llvm/lib/MC/MCParser/MasmParser.cpp:1551
+    if (!Sym) {
+      auto VarIt = Variables.find(SymbolName.lower());
+      if (VarIt != Variables.end())
----------------
thakis wrote:
> This looks up a label, and if that fails, tries to look up a var with the same (lowered) name. Is that intentional? If so, could you mention that in the patch summary? As far as I understand, this change is unrelated to what's currently in the description.
> 
> Same with the Unlex changes below which I'm guessing are related to this change here.
> 
> I understand all the `.lower()` changes in the diff, I think I understand the test, but I don't get why the rest is changing yet.
I've added some comments in a new version of the change. Quick summary: this location doesn't change how things are handled - we already tried looking up a Symbol, now we just canonicalize the casing if it happens to be a Variable name.

Also, the UnLex changes below fix error handling for textmacro expansion, and were required to get the test working cleanly. I've updated the patch summary to mention those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92503