[PATCH] D82435: [DWARFYAML][debug_gnu_*] Add the missing context `IsGNUStyle`. NFC.

Xing GUO via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 3 03:12:20 PDT 2020


Higuoxing added inline comments.


================
Comment at: llvm/unittests/ObjectYAML/DWARFYAMLTest.cpp:107
+
+  ASSERT_EQ(PubNames.Entries.size(), 2ul);
+  ASSERT_EQ((uint32_t)PubNames.Entries[0].DieOffset, 0x1234u);
----------------
jhenderson wrote:
> Higuoxing wrote:
> > jhenderson wrote:
> > > I might be wrong, but I don't think you need the `ul` suffix here (similar below for `u` suffices).
> > My compiler complains that `comparison of integer expressions of different signedness: ‘const long unsigned int’ and ‘const int’`. So I have to add the suffices to suppress the warning.
> Huh. In this case, I'd argue this is a spurious warning since the `2` is a literal and trivially can be converted to `unsigned` for comparisons. This might be caused by gtest rather than your compiler though.
> 
> Either way, I wouldn't use `ul` - I'd use `u` if you have to use anything. `size()` returns a `size_t` whose size is not necessarily `unsigned long`. `size_t` is 32 bits or 64 bits depending on the system (i.e. a 32 or 64 bit host OS typically). `long` is 32 bits or 64 bits depending on the system too, but with different rules (the standard merely dictates that it is at least as large as an `int` and on Windows it's only 4 bytes).
Thanks! I learned something new today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82435





More information about the llvm-commits mailing list