simon_tatham added a comment.

Hmmm. Two people have pointed out to me that my strategy of having a 32-bit 
`SourceLocations::LowBits` and an 0- or 32-bit 
`SourceLocations::OptionalHighBits` doesn't actually work, because an empty 
struct still takes at least 1 byte. So this version of the patch will still 
increase memory usage in the 32SL configuration, which is just what I was 
trying to avoid. Whoops.

@rsmith, do you have any thoughts on what would be an acceptable replacement 
strategy? You mentioned wanting to avoid actual #ifdefs all over the AST class 
hierarchy. Some possible alternatives:

We could define a family of macros: one for declaring a possible-SourceLocation 
in a branch of the Stmt bitfields union, one for declaring the same 
SourceLocation in a particular Stmt subclass, and a pair for the accessor 
functions that read and write whichever of those exists. Then there'd be 
slightly ugly macro calls all over Stmt.h and friends, but only one outright 
#ifdef, where the macros are defined.

(And then there are two options for how to define the macros in the 64SL case: 
either move the whole SourceLocation into the subclasses for speed, or keep 
half of it in the bitfields as now, for space. But that decision could be 
localized into the macro definitions, and easily changed.)

Alternatively, @miyuki suggests that my SourceLocation::OptionalHighBits could 
become an extra base class of some of the Stmt subclasses, so that empty base 
optimization //would// allow it to take up 0 bytes in the 32SL configuration.

The macro approach is the kind of thing I wouldn't mind doing in my own 
projects, but then, I have a strong stomach for macros :-) and I'd rather check 
before I do all the work to rewrite this patch in one of those styles, only to 
find out that you'd prefer another, or something I haven't even thought of.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97204

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to