Skip to content

c-writer.cc: Eliminate GetGlobalName (NFC)#2041

Merged
keithw merged 2 commits intomainfrom
wasm2c-elim-getglobalname
Nov 5, 2022
Merged

c-writer.cc: Eliminate GetGlobalName (NFC)#2041
keithw merged 2 commits intomainfrom
wasm2c-elim-getglobalname

Conversation

@keithw
Copy link
Member

@keithw keithw commented Nov 5, 2022

Require all resolution of GlobalNames to happen in one place (Write(GlobalName)). Also eliminate AddressOf, Deref, GlobalVar.

This is a step towards fixing #2034 (split out from #2035).

Require all resolution of GlobalNames to happen in one place
(Write(GlobalName)). Also eliminate AddressOf, Deref, GlobalVar.

This is a step towards fixing #2034 (split out from #2035).
src/c-writer.cc Outdated
assert(global_sym_map_.count(name.name) == 1);
auto iter = global_sym_map_.find(name.name);
assert(iter != global_sym_map_.end());
Write(iter->second);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be enough to just do Write(global_sym_map_.at(name.name)) should be enough here.. no need for count or find maybe?

I guess this is existing code though, so might not make sense as part of this change.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that works fine for now. We can worry about how much defensive programming is worthwhile in the next one.

@keithw keithw enabled auto-merge (squash) November 5, 2022 20:13
@keithw keithw merged commit da225fc into main Nov 5, 2022
@keithw keithw deleted the wasm2c-elim-getglobalname branch November 5, 2022 20:32
matthias-blume pushed a commit to matthias-blume/wabt that referenced this pull request Dec 16, 2022
Require all resolution of GlobalNames to happen in one place
(Write(GlobalName)). Also eliminate AddressOf, Deref, GlobalVar.

This is a step towards fixing WebAssembly#2034 (split out from WebAssembly#2035).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants