Building user-manager 5.18.1 with clang++ 10.0 resulted in 2 compiler warnings - a quick look at the code shows they're real bugs.
"case Qt::DisplayRole || AccountModel::FriendlyName:" isn't quite the same as "if(role==Qt::DisplayRole || role==AccountModel::FriendlyName)" -- but that's clearly what is meant here.
Details
Details
- Reviewers
davidedmundson - Group Reviewers
Plasma
Check the output of AccountModel::data() with various role types, including Qt::DisplayRole, AccountModel::FriendlyName, Qt::DecorationRole and AccountModel::Face before and after applying the patch.
Diff Detail
Diff Detail
- Repository
- R128 User Manager
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Comment Actions
Actually the obvious fix doesn't work (thought it was so obvious that I submitted it before testing, sorry) because Qt::DisplayRole and AccountModel::FriendlyName are both 0 (-> duplicate case value) and Qt::DecorationRole and AccountModel::Face are both 1 (-> duplicate case value). Also explains why the bogus variant before "worked" - (Qt::DisplayRole || AccountModel::FriendlyName) = 0 || 0 = 0 = Qt::DisplayRole = AccountModel::FriendlyName and (Qt::DecorationRole || AccountModel::Face) = 1 || 1 = 1 = Qt::DecorationRole = AccountModel::Face
Revising the patch...
Comment Actions
Probably best to just add a comment explaining what needs to be done if AccountModel::* roles ever diverge from DisplayRole/DecorationRole...
Leaving the code the way it is now will result in a harder to trace error if Qt ever changes the values of DisplayRole or DecorationRole (and may give people looking at the code bad ideas), revising it the way I proposed earlier causes duplicate case values.
Comment Actions
Another option would be switching the code to use if instead of switch/case so the || operator would actually do what the code expected (and x || x, unlike duplicate case values, isn't an error)