[PATCH] D97204: [RFC] Clang 64-bit source locations

[Simon Tatham via Phabricator via cfe-commits]

simon_tatham updated this revision to Diff 352641.
simon_tatham added a comment.

@miyuki is working on other things at the moment, and I've picked this up.

This updated diff reverts the libclang ABI break completely. In the libclang ABI, there's still only 32 bits of space for a SourceLocation, and this is handled by bounds-checking on the clang side when converting into that format (and we return an invalid location if the bounds check fails). So any use of libclang that previously worked should still work, and ones that would previously have failed will now fail in a different way.

Perhaps in future we might introduce a secondary libclang ABI //coexisting// with the existing one, with different function names, and expanded data structures that can hold a full 64-bit SourceLocation? And then each client could switch over at their own convenience (or not at all).

I've addressed @rsmith's review suggestions by making a new pair of opaque structures called SourceLocation::LowBits and SourceLocation::OptionalHighBits, and a pair of functions to convert between a SourceLocation and a pair of those. So, in all the places where a SourceLocation was stored in one of those 'bits' unions, the union still contains a SourceLocation::LowBits containing 32 of the bits, and the derived class contains an OptionalHighBits which is zero size or another 32 bits depending on what size of SourceLocation we're compiling for.

The AST file format now uses a 64-bit field unconditionally for a source location. I'm not too familiar with this part of clang, but it //looks// as if AST files already have a versioning system based on the clang git commit id, so that incompatibility between this and the previous format will already be detected without me having to manually bump a format version number anywhere?

I haven't yet managed to repeat @miyuki's memory-usage analysis, so I can't say how much of the previous 9% loss is recovered by these changes. But I'll look at doing that next.

Incidentally, in passing I spotted what looks like an obvious bug in the libclang SourceLocation conversion code. Raised D104442 <https://reviews.llvm.org/D104442> separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97204

Files:
  clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
  clang/CMakeLists.txt
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclObjC.h
  clang/include/clang/AST/DeclarationName.h
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/ExprConcepts.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Analysis/ProgramPoint.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Basic/SourceManager.h
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Lex/Token.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/include/clang/Serialization/ASTReader.h
  clang/include/clang/Serialization/ASTWriter.h
  clang/include/clang/Serialization/ModuleFile.h
  clang/lib/ARCMigrate/TransEmptyStatementsAndDealloc.cpp
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ExprConcepts.cpp
  clang/lib/AST/NestedNameSpecifier.cpp
  clang/lib/AST/SelectorLocationsKind.cpp
  clang/lib/AST/Stmt.cpp
  clang/lib/Basic/SourceLocation.cpp
  clang/lib/Basic/SourceManager.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CoverageMappingGen.cpp
  clang/lib/Format/FormatTokenLexer.cpp
  clang/lib/Lex/Lexer.cpp
  clang/lib/Lex/ModuleMap.cpp
  clang/lib/Lex/PPCaching.cpp
  clang/lib/Lex/TokenLexer.cpp
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/test/CMakeLists.txt
  clang/test/Lexer/SourceLocationsOverflow.c
  clang/test/lit.cfg.py
  clang/test/lit.site.cfg.py.in
  clang/tools/libclang/CIndex.cpp
  clang/tools/libclang/CXIndexDataConsumer.cpp
  clang/tools/libclang/CXSourceLocation.cpp
  clang/tools/libclang/CXSourceLocation.h
  clang/tools/libclang/Indexing.cpp
  llvm/include/llvm/IR/DiagnosticInfo.h
  llvm/include/llvm/IR/LLVMContext.h
  llvm/include/llvm/MC/MCParser/MCAsmParser.h
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/CodeGen/MachineInstr.cpp
  llvm/lib/IR/LLVMContext.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/MC/MCParser/MasmParser.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97204.352641.patch
Type: text/x-patch
Size: 145839 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210617/8b66f06c/attachment-0001.bin>