SourceLocation refactoring review

Clang Frontend
1

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:
* ⚙ D69840 [clang] Use SourceLocation as key in hash maps, NFCI - [Basic] Make SourceLocation usable as key in hash maps, NFCI
* ⚙ D69844 [clang][Basic] Integrate SourceLocation with FoldingSet, NFCI - [Basic] Integrate SourceLocation and SourceRange with FoldingSet, NFCI
* ⚙ D69903 [Basic] Introduce PODSourceLocation, NFCI - [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: [cfe-dev] [RFC] Clang SourceLocation overflow

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.

2

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

3

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, ... ).

The refactoring does affect clang tools extra, but just as any refactoring it is a
non-functional change, i.e. it is not expected to change the behavior.