control reaches end of non-void function warnings in 1.3

[hadess]

Would be nice to be made quiet:

[12/110] Building CXX object src/gba/CMakeFiles/gba.dir/core.cpp.o
src/gba/core.cpp: In member function ‘virtual u32 AGB_core::ex_get_reg(u8)’:
src/gba/core.cpp:634:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[22/110] Building CXX object src/gba/CMakeFiles/gba.dir/sio.cpp.o
src/gba/sio.cpp: In member function ‘bool AGB_SIO::soul_doll_adapter_save_data()’:
src/gba/sio.cpp:675:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[27/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/mbc1.cpp.o
src/dmg/mbc1.cpp: In member function ‘u8 DMG_MMU::mbc1_read(u16)’:
src/dmg/mbc1.cpp:82:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
src/dmg/mbc1.cpp: In member function ‘u8 DMG_MMU::mbc1_multicart_read(u16)’:
src/dmg/mbc1.cpp:165:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
src/dmg/mbc1.cpp: In member function ‘u8 DMG_MMU::mbc1s_read(u16)’:
src/dmg/mbc1.cpp:250:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[28/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/mbc3.cpp.o
src/dmg/mbc3.cpp: In member function ‘u8 DMG_MMU::mbc3_read(u16)’:
src/dmg/mbc3.cpp:251:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[29/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/mbc5.cpp.o
src/dmg/mbc5.cpp: In member function ‘u8 DMG_MMU::mbc5_read(u16)’:
src/dmg/mbc5.cpp:93:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[31/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/mbc6.cpp.o
src/dmg/mbc6.cpp: In member function ‘u8 DMG_MMU::mbc6_read(u16)’:
src/dmg/mbc6.cpp:268:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[32/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/mbc7.cpp.o
src/dmg/mbc7.cpp: In member function ‘u8 DMG_MMU::mbc7_read(u16)’:
src/dmg/mbc7.cpp:353:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[33/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/huc3.cpp.o
src/dmg/huc3.cpp: In member function ‘u8 DMG_MMU::huc3_read(u16)’:
src/dmg/huc3.cpp:83:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[34/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/huc1.cpp.o
src/dmg/huc1.cpp: In member function ‘u8 DMG_MMU::huc1_read(u16)’:
src/dmg/huc1.cpp:85:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[35/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/mmm01.cpp.o
src/dmg/mmm01.cpp: In member function ‘u8 DMG_MMU::mmm01_read(u16)’:
src/dmg/mmm01.cpp:98:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[38/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/tama5.cpp.o
src/dmg/tama5.cpp: In member function ‘u8 DMG_MMU::tama5_read(u16)’:
src/dmg/tama5.cpp:126:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[39/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/camera.cpp.o
src/dmg/camera.cpp: In member function ‘u8 DMG_MMU::cam_read(u16)’:
src/dmg/camera.cpp:90:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[42/110] Building CXX object src/dmg/CMakeFiles/dmg.dir/mmu.cpp.o
src/dmg/mmu.cpp: In member function ‘u8 DMG_MMU::mbc_read(u16)’:
src/dmg/mmu.cpp:1458:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[53/110] Building CXX object src/sgb/CMakeFiles/sgb.dir/__/dmg/mbc1.cpp.o
src/dmg/mbc1.cpp: In member function ‘u8 DMG_MMU::mbc1_read(u16)’:
src/dmg/mbc1.cpp:82:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
src/dmg/mbc1.cpp: In member function ‘u8 DMG_MMU::mbc1_multicart_read(u16)’:
src/dmg/mbc1.cpp:165:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
src/dmg/mbc1.cpp: In member function ‘u8 DMG_MMU::mbc1s_read(u16)’:
src/dmg/mbc1.cpp:250:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[56/110] Building CXX object src/sgb/CMakeFiles/sgb.dir/__/dmg/mbc3.cpp.o
src/dmg/mbc3.cpp: In member function ‘u8 DMG_MMU::mbc3_read(u16)’:
src/dmg/mbc3.cpp:251:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[57/110] Building CXX object src/sgb/CMakeFiles/sgb.dir/__/dmg/mbc5.cpp.o
src/dmg/mbc5.cpp: In member function ‘u8 DMG_MMU::mbc5_read(u16)’:
src/dmg/mbc5.cpp:93:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[58/110] Building CXX object src/sgb/CMakeFiles/sgb.dir/__/dmg/mbc7.cpp.o
src/dmg/mbc7.cpp: In member function ‘u8 DMG_MMU::mbc7_read(u16)’:
src/dmg/mbc7.cpp:353:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[59/110] Building CXX object src/sgb/CMakeFiles/sgb.dir/__/dmg/huc1.cpp.o
src/dmg/huc1.cpp: In member function ‘u8 DMG_MMU::huc1_read(u16)’:
src/dmg/huc1.cpp:85:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[60/110] Building CXX object src/sgb/CMakeFiles/sgb.dir/__/dmg/mmm01.cpp.o
src/dmg/mmm01.cpp: In member function ‘u8 DMG_MMU::mmm01_read(u16)’:
src/dmg/mmm01.cpp:98:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[61/110] Building CXX object src/sgb/CMakeFiles/sgb.dir/__/dmg/camera.cpp.o
src/dmg/camera.cpp: In member function ‘u8 DMG_MMU::cam_read(u16)’:
src/dmg/camera.cpp:90:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[65/110] Building CXX object src/sgb/CMakeFiles/sgb.dir/__/dmg/mmu.cpp.o
src/dmg/mmu.cpp: In member function ‘u8 DMG_MMU::mbc_read(u16)’:
src/dmg/mmu.cpp:1458:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[68/110] Building CXX object src/nds/CMakeFiles/nds.dir/core.cpp.o
src/nds/core.cpp: In member function ‘virtual bool NTR_core::read_bios(std::__cxx11::string)’:
src/nds/core.cpp:896:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[76/110] Building CXX object src/nds/CMakeFiles/nds.dir/arm9.cpp.o
src/nds/arm9.cpp: In member function ‘u32 NTR_ARM9::get_reg(u8) const’:
src/nds/arm9.cpp:187:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
src/nds/arm9.cpp: In member function ‘u32 NTR_ARM9::get_spsr() const’:
src/nds/arm9.cpp:286:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
[100/110] Building CXX object src/qt/CMakeFiles/gbe_plus_qt.dir/cgfx.cpp.o
src/qt/cgfx.cpp: In member function ‘bool gbe_cgfx::delete_manifest_entry(int)’:
src/qt/cgfx.cpp:2388:28: warning: control reaches end of non-void function [-Wreturn-type]
  std::vector <std::string> manifest;
                            ^~~~~~~~
src/qt/cgfx.cpp: In member function ‘std::__cxx11::string gbe_cgfx::hash_tile(u8, u8)’:
src/qt/cgfx.cpp:3025:1: warning: control reaches end of non-void function [-Wreturn-type]
 }
 ^
src/qt/cgfx.cpp: In member function ‘bool gbe_cgfx::parse_manifest_items()’:
src/qt/cgfx.cpp:3030:28: warning: control reaches end of non-void function [-Wreturn-type]
  std::vector <std::string> manifest;
                            ^~~~~~~~

[shonumi]

There are a bunch of other warnings in GBE+ too at the moment. You can turn off all warnings by adding -w to this line in CMakeLists.txt

Eventually I'll get around to looking at all of them, but for the short-term it should be okay to suppress all warnings.

[hadess]

The problem isn't so much that the warnings appear, it's that they might end up hiding bugs. What happens if nothing is returned when a string, or a portion of memory is expected?

I'd protect those returns with assert() calls, so that you know that those branches without a return value will never be hit, and that the program would crash before hitting the undefined behaviour.

[shonumi]

A vast majority of those wouldn't hit undefined behavior due to the way GBE+ parses input for those functions before they're called. Specifically for the majority of those warnings, they're from the DMG core's MBC reads. Those reads are only ever called when some other code has already validated the memory ranges. GBE+ would never give input that falls into out of range memory (and cause nothing to be returned).

The NDS ones (except for read_bios()) are basically in the same category. Every possible value of the cpu_modes enum is covered by the switch statement, but it appears the compiler is essentially complaining because there's no default statement, despite it being redundant. GBE+ should never set an invalid enum value.

I know it seems like a very "rubber and bubble gum" approach but in those cases, it should never encounter a situation where nothing is returned. I don't say that to defend it, only to illustrate that in those instances it's not a huge issue. Still an issue (I have known about it for years), just one that hasn't warranted much attention.

However, I do acknowledge that are a few areas that appear new to me, and those are actually a bit concerning. read_bios() and the other CGFX related stuff would definitely hit undefined behavior since those aren't checked beforehand. I'll take a look soon. I could use assert() but IMO the better solution would be to return values of some kind, even if only to indicate an error.

[hadess]

assert() statements would make it clear that the section of code are not supposed to be reached, and quiet the compiler, and would catch eventual mistakes earlier.

[shonumi]

Asserts here aren't ideal at all though. Program execution should not be terminated under those conditions. Those situations would typically bring up warnings in GBE+ (via std::cout), but they shouldn't halt the emulator altogether.

A better solution is to return something, anything in non-void functions, be it a default value or an error code.

[shonumi]

Should be fixed as of 3fe61a77c413179c22674278f9018fff29e9b35b