[PATCH] D139774: [libclang] Add API to set temporary directory location

[Aaron Ballman via Phabricator via llvm-commits]

aaron.ballman added a comment.

In D139774#4098695 <https://reviews.llvm.org/D139774#4098695>, @vedgy wrote:

> Another disadvantage of using `CINDEX_VERSION_MINOR` instead of the `sizeof` is that an older libclang version won't know about compatibility of newer versions with itself. But this reasoning brought me to a different thought: when a program is compiled against a libclang version X, it should not be expected to be runtime-compatible with a libclang version older than X. For example, KDevelop uses `CINDEX_VERSION_MINOR` to decide whether or not certain libclang API is available.
>
> I suspect modifying the exposed struct's size will violate ODR in C++ programs that use libclang. Quote from the cppreference ODR page <https://en.cppreference.com/w/cpp/language/definition#One_Definition_Rule>:
>
>> Note: in C, there is no program-wide ODR for types, and even extern declarations of the same variable in different translation units may have different types as long as they are compatible. In C++, the source-code tokens used in declarations of the same type must be the same as described above: if one .cpp file defines `struct S { int x; };` and the other .cpp file defines `struct S { int y; };`, the behavior of the program that links them together is undefined. This is usually resolved with unnamed namespaces.
>
> I think the answer to this StackOverflow question <https://stackoverflow.com/questions/5140893/struct-defined-differently-for-c-and-c-is-it-safe-pc-lint-warns> confirms my suspicion.

Oh wow, I know the person who asked that question 12 years ago, that's always a fun thing to run into randomly. :-D

Given that this idiom (using size of a structure to version it) is used in practice all over the place, I'm not really worried about violating the ODR. Because the library is using the size information provided to determine what's safe to access, the only actionable UB comes from setting the size field incorrectly because then the library may access out of bounds memory. But an optimizer has no real opportunity for optimization here because we're talking about a structure being passed across a DSO boundary (so not even the linker sees both the library and the application in this scenario, so LTO also doesn't come into play).

(If this idiom wasn't so entrenched in industry, we could use a tagged union instead, but conceptually that's an identical solution to this one because the `Size` field is acting as the tag to determine which union members are accessible. I don't think we need to go to that much trouble though.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139774