[cfe-dev] SourceLocation refactoring review

[Christian Gagneraud via cfe-dev]

On Fri, 24 Apr 2020 at 01:55, Mikhail Maltsev via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>
> Hi folks,
>
> I have series of patches which refactor SourceLocation uses to reduce the number of places where the raw (integer) representation of SourceLocation objects is accessed directly:
> * https://reviews.llvm.org/D69840 - [Basic] Make SourceLocation usable as key in hash maps, NFCI
> * https://reviews.llvm.org/D69844 - [Basic] Integrate SourceLocation and SourceRange with FoldingSet, NFCI
> * https://reviews.llvm.org/D69903 - [Basic] Introduce PODSourceLocation, NFCI
>
> When Adrian Prantl was reviewing the first patch he asked me about my motivation for these patches, and I replied that the goal is to be able to configure the bit width of the SourceLocation representation (32 or 64 bits), this proposal had been discussed on the mailing list: http://lists.llvm.org/pipermail/cfe-dev/2019-October/063515.html
>
> Apparently the statement about my motivation made the reviewers (not just Adrian) very reluctant to continuing the review, probably because changing the SourceLocation representation is a major change in a ubiquitous class.
>
> Nevertheless I would like to emphasize the patch series itself does not introduce any changes to the representation of SourceLocation-s. The change is just a refactoring (which in my opinion makes the code cleaner and better in terms of expressing the intent). We don't actually need consensus on the representation change to apply the patch series because it is an independent change. So I am kindly requesting the Clang code owners to review the patches.

Does this impact clang tools extra? I've noticed an annoying "feature" there:
clang-tidy raises errors for rules violation caused by pre-processor
macros expansion defined in "system headers" expanded in user code.
This is quite annoying to me, it avoids me to deploy clang-tidy checks
(Root errors come from macros in Qt, boost, Google test, ... ).

Chris