joerg added a comment.
Please fix the spelling errors in the titel / summary before commit.
I somewhat agree with Hal -- I think this is too aggressive. Common use cases
for local volatile include atomic ops or returns-twice functions like
setjmp/longjmp.
Disabling the warning in those cases ha
thakis added a comment.
In https://reviews.llvm.org/D28543#671953, @hfinkel wrote:
> What's the motivation for this? The placement of a local volatile variable is
> still under the compiler's direction, and unless the address escapes, we
> still assume we can reason about its aliasing (and, thu
hfinkel added a comment.
What's the motivation for this? The placement of a local volatile variable is
still under the compiler's direction, and unless the address escapes, we still
assume we can reason about its aliasing (and, thus, whether or not it is
initialized).
https://reviews.llvm.org
lethalantidote added a comment.
Any updates on this?
https://reviews.llvm.org/D28543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
thakis accepted this revision.
thakis added a comment.
This revision is now accepted and ready to land.
Looks great to me, thanks! Unless someone else shouts, I'll land this for you
soon.
https://reviews.llvm.org/D28543
___
cfe-commits mailing list
lethalantidote updated this revision to Diff 84045.
lethalantidote added a comment.
Moves check in IsTracked().
https://reviews.llvm.org/D28543
Files:
clang/lib/Analysis/UninitializedValues.cpp
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/Sema/uninit-variables.c
Index: clang/test
lethalantidote added a comment.
So I tried the update you suggested (moving it up into IsTracked) and it seems
to work and the reasoning makes sense to me.
https://reviews.llvm.org/D28543
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http
thakis added a comment.
Thanks, this is looking pretty good! From clicking around a bit on cs, do you
think it's better to put the check where you have it, or is maybe
http://llvm-cs.pcc.me.uk/tools/clang/lib/Analysis/UninitializedValues.cpp#36
more appropriate? I think having it where you have
lethalantidote updated this revision to Diff 84026.
lethalantidote marked an inline comment as done.
lethalantidote added a comment.
Addresses moving check further up, during analysis.
Adds test to check for sometimes branch. Please review.
https://reviews.llvm.org/D28543
Files:
clang/lib/Ana
thakis added a comment.
Thanks for the test!
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:907
else
DiagUninitUse(S, VD, Use, true);
}
lethalantidote wrote:
> thakis wrote:
> > Should the check be in DiagUninitUse()? Or is there a reason t
lethalantidote added inline comments.
Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:907
else
DiagUninitUse(S, VD, Use, true);
}
thakis wrote:
> Should the check be in DiagUninitUse()? Or is there a reason this one should
> happen for volati
lethalantidote updated this revision to Diff 84016.
lethalantidote marked an inline comment as done.
lethalantidote added a comment.
Addresses thakis' comments and adds a test.
https://reviews.llvm.org/D28543
Files:
clang/lib/Sema/AnalysisBasedWarnings.cpp
clang/test/Sema/uninit-variables.c
thakis added a comment.
Thanks! Can you add a test somewhere? `grep -R W.*uninitialized test/Sema*`
will probably show you the existing test for this code. (Another technique:
Comment some of the existing tests, run `ninja check-clang`, and see which
tests start failing.)
You can run an indiv
lethalantidote created this revision.
lethalantidote added a reviewer: thakis.
lethalantidote added a subscriber: cfe-commits.
Elliminates uninitialized warning for volitile variables.
https://reviews.llvm.org/D28543
Files:
clang/lib/Sema/AnalysisBasedWarnings.cpp
Index: clang/lib/Sema/Anal
14 matches
Mail list logo
