aaron.ballman added inline comments.
================ Comment at: clang/lib/AST/StmtPrinter.cpp:1159 + case BuiltinType::UInt128: + OS << "Ui128"; + break; ---------------- davidstone wrote: > aaron.ballman wrote: > > riccibruno wrote: > > > riccibruno wrote: > > > > davidstone wrote: > > > > > riccibruno wrote: > > > > > > `i128` and `Ui128` are not valid integer literal suffix. The output > > > > > > of `StmtPrinter` is intended to be valid C++. Unfortunately here I > > > > > > think that your only choice is to print the high and low parts > > > > > > separately. > > > > > I'm confused. i8, Ui8, i16, and Ui16 are also not valid C++ suffixes, > > > > > but just a few lines up we use those. What am I missing here? > > > > The literal suffixes `[u]i8, [u]i16, [u]i32, and [u]i64` are an MSVC > > > > extension (see `NumericLiteralParser::NumericLiteralParser` in > > > > `Lex/LiteralSupport.cpp`). > > > > > > > > This does not explain why they are used even in non-ms compatibility > > > > mode > > > > but at least there is some reason for their existence. > > > > > > > > However I don't think that MSVC supports 128-bits integers (?), and > > > > clang certainly > > > > does not support `[u]i128` so there is no reason to use them. > > > > > > > > @aaron.ballman Do you know why are these suffixes used outside of > > > > ms-compatibility mode? > > > > This does not explain why they are used even in non-ms compatibility > > > > mode > > > > but at least there is some reason for their existence. > > > > > > Let's just ask the author @majnemer > > > @aaron.ballman Do you know why are these suffixes used outside of > > > ms-compatibility mode? > > > > Our pretty printing is *supposed* to generate valid code, but we get it > > wrong fairly often and I don't think we've ever promised (let alone tested) > > that you can use this output to compile code (and get the same results). I > > think that's more of a stretch goal. That said, the pretty printer should > > probably be looking at the language options to decide whether it wants to > > output those suffixes or not. > > > > As for historical context, I think the thread starting here is relevant: > > http://lists.llvm.org/pipermail/cfe-dev/2012-September/024423.html > So what is the next step for this patch? > So what is the next step for this patch? I don't think we should print any suffix for these, similar to how we handle `int`. There is no 128-bit suffix we support. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82880/new/ https://reviews.llvm.org/D82880 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
