[lld] Ensure NoTrapAfterNoreturn is false for the wasm backend (PR #65876)

Heejin Ahn via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 29 00:17:09 PDT 2023


aheejin wrote:

> > Yeah I get that we haven't been warning about a similar case (--trap-unreachable) before. But I think they are more of what ended up happen, and not the firm intention not to warn for conflicted requests.
> > For example, that --trap-unreachable command line option was added much later ([6d9f8c9](https://github.com/llvm/llvm-project/commit/6d9f8c98172cd4d648e33b21679325227c5cec83)) than when we set it to false ([ffa143c](https://github.com/llvm/llvm-project/commit/ffa143ce814101fb1277ba65b20bdf86775d0b32)). When first we set it to false in 2015, that option didn't exist so we had nothing to warn against. And someone who added that option in 2018 didn't go through all the code to check if there were any conflicting assignments.
> 
> You might be right, but my feeling is that the behaviour of `--trap-unreachable` and `--no-trap-after-noreturn` should be consistent across all backends, and probably with other options too. I think that's a big enough change to warrant discussion and testing separately.
> 
> This change is consistent with the current behaviour and fixes a real issue, so I think it makes sense for this change to go in first, before we start to think about changing any existing behaviour.

OK fair enough. Sorry that this PR is taking so long time. I guess we can land this after we fix some tests / add some comments. I still think the test file needs more comments; I would like that the readers of the test file find it easy enough to figure out what the purpose of the tests in that file is, without referring to `git blame`s and PR discussions.

https://github.com/llvm/llvm-project/pull/65876


More information about the llvm-commits mailing list