[Lldb-commits] [lldb] [lldb][test] Add test for ASTImporter's name conflict resolution (PR #112566)

[Michael Buch via lldb-commits]

Michael137 wrote:

> I think this is one of those tests that should really be an API test. As far as I can tell, there's nothing darwin-specific about this bug -- except for the fact that building shared libraries in a cross-platform manner is hard. But the API tests have wrappers that let us do that, and they even support windows. (I had a feeling our docs already stated that shared libraries is a reason to make something an API test, but I can't find that now.)

Yea that makes sense. Adding a `system-darwin` requirement did feel off.

> I understand the desire to not skip a test (and that making it an API test will make that even harder, as a crash will bring down the entire test), but I'm not sure if that's so useful as to override this. I also think that making a test which checks that something fails in a particular way can be fragile (as a random refactor could make it fail in a different way).

True, I've seen this be an issue in the past, especially with ASTImporter tests. I was considering adding a warning (or log message at the very least) when we try to overwrite and already mapped decl. We could technically XFAIL on that. But yea that's probably too fragile. Ideally we'd just test that we successfully run the expression.

> From the looks of things, it may be possible to get rid of the dlopen/dlsym thing as well (replace it with hard shared library dependencies) -- it seems the important part is that the two modules have different definitions of one type, and the way that the shared library is loaded is beside the point.

Yup, that should work!

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