Partial fix for 13761 - Fix code which triggers warnings #4174
Conversation
|
Many indentations of your changes are messed up. (A possible solution: set your editor to automatically convert tabs to 4 spaces, and try again). |
|
Thanks, fixed. |
| @@ -2343,7 +2343,10 @@ void FuncDeclaration::buildResultVar(Scope *sc, Type *tret) | |||
| { | |||
| if (!vresult) | |||
| { | |||
| Loc loc = fensure ? fensure->loc : loc; | |||
There was a problem hiding this comment.
If fensure is NULL, the local loc should be initialized by this->loc.
There was a problem hiding this comment.
C++ unlike D accepts code like:
int a = false ? 1 : a;
and a will be uninitialized
Then it was a logic error in the original code.
| se = (StringExp *)e->copy(); | ||
| copied = 1; |
There was a problem hiding this comment.
All copied = 1; are immediately after the copy() call and they're consistent. Removing some assignments would optimize code a little but it would be unkind for humans. keep them as-is.
There was a problem hiding this comment.
After these copied = 1 there is a jump to Lcast which returns and there « copied » are not used.
Are you sure that we must keep these copied ?
There was a problem hiding this comment.
I know those assignments are currently redundant. But if we modify/refactor this function and the code flow would be changed, copied = 1 might be required again. In my thought those redundant lines are necessary for the code robustness.
|
Yes sorry for mistakes now all seems to be right. |
| @@ -425,7 +429,7 @@ int File::exists() | |||
| void File::remove() | |||
| { | |||
| #if POSIX | |||
| int dummy = ::remove(this->name->toChars()); | |||
There was a problem hiding this comment.
No, this needs to stay as-is to work around a limitation of the ddmd conversion tool.
|
I approved testing this PR in the auto-tester. Please rebase commits on git-head master. |
| { | ||
| assert(t1 && t2); | ||
|
|
||
| if(t1->Tty == TYdouble && t2->Tty == TYdouble) |
There was a problem hiding this comment.
What if t2 is null and t1->Tty is not TYdouble? It previously wouldn't crash, but now it will assert.
There was a problem hiding this comment.
Is it possible ?
OK if so I corrected it.
ghost
changed the title
|
Will pull once it is mergable. |
|
Note, in #4239, the |
| @@ -1529,8 +1531,6 @@ elem *toElem(Expression *e, IRState *irs) | |||
| } | |||
| else | |||
| { | |||
| d_uns64 elemsize = sd->size(ne->loc); | |||
There was a problem hiding this comment.
Note, this is needed for the side effect, even though the variable is unused. If memory serves correctly.
|
Outdated and fixed by other prs |
|
For sure now the majority of sources are converted to D, static analysis can no longer be done on the sources. I will ignore most of the asserts here, I don't think any of them are actual bugs. There are still a few unmerged C source changes, and I did spot some logic bugs in the D code too. |
https://issues.dlang.org/show_bug.cgi?id=13761