r205667 - [analyzer] Add an ErrnoChecker (PR18701) to the Potential Checkers list.
Author: jrose Date: Sat Apr 5 01:10:28 2014 New Revision: 205667 URL: http://llvm.org/viewvc/llvm-project?rev=205667&view=rev Log: [analyzer] Add an ErrnoChecker (PR18701) to the Potential Checkers list. Modified: cfe/trunk/www/analyzer/potential_checkers.html Modified: cfe/trunk/www/analyzer/potential_checkers.html URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/potential_checkers.html?rev=205667&r1=205666&r2=205667&view=diff == --- cfe/trunk/www/analyzer/potential_checkers.html (original) +++ cfe/trunk/www/analyzer/potential_checkers.html Sat Apr 5 01:10:28 2014 @@ -314,6 +314,34 @@ int foo(bool cond) { + +POSIX + + +Name, DescriptionExampleProgress + +posix.Errno +Record that errno is non-zero when certain functions fail. + +#include+ +int readWrapper(int fd, int *count) { + int lcount = read(fd, globalBuf, sizeof(globalBuf)); + if (lcount < 0) +return errno; + *count = lcount; + return 0; +} + +void use(int fd) { + int count; + if (!readWrapper(fd)) +print("%d", count); // should not warn +} +http://llvm.org/bugs/show_bug.cgi?id=18701";>PR18701 + + + undefined behavior ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205666 - [analyzer] Update Open Projects page with BitwiseConstraintManager.
Author: jrose Date: Sat Apr 5 01:10:22 2014 New Revision: 205666 URL: http://llvm.org/viewvc/llvm-project?rev=205666&view=rev Log: [analyzer] Update Open Projects page with BitwiseConstraintManager. Also, add the names of people most recently working on particular projects, and remove "relate bugs and checkers" (thanks, Alex!). Modified: cfe/trunk/www/analyzer/open_projects.html Modified: cfe/trunk/www/analyzer/open_projects.html URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/analyzer/open_projects.html?rev=205666&r1=205665&r2=205666&view=diff == --- cfe/trunk/www/analyzer/open_projects.html (original) +++ cfe/trunk/www/analyzer/open_projects.html Sat Apr 5 01:10:22 2014 @@ -56,7 +56,7 @@ mailing list to notify other members Enhance CFG to model C++ temporaries properly. There is an existing implementation of this, but it's not complete and is disabled in the analyzer. -(Difficulty: Medium) +(Difficulty: Medium; current contact: Alex McCarthy) Enhance CFG to model exception-handling properly. Currently exceptions are treated as "black holes", and exception-handling @@ -70,7 +70,7 @@ mailing list to notify other members (operator new), then initialize the result with a constructor call. The problem is discussed at length in http://llvm.org/bugs/show_bug.cgi?id=12014";>PR12014. -(Difficulty: Easy) +(Difficulty: Easy; current contact: Karthik Bhat) Enhance CFG to model C++ delete more precisely. Similarly, the representation of delete does not include @@ -78,7 +78,14 @@ mailing list to notify other members function (operator delete). One particular issue (noreturn destructors) is discussed in http://llvm.org/bugs/show_bug.cgi?id=15599";>PR15599 -(Difficulty: Easy) +(Difficulty: Easy; current contact: Karthik Bhat) + +Implement a BitwiseConstraintManager to handle http://llvm.org/bugs/show_bug.cgi?id=3098";>PR3098. +Constraints on the bits of an integer are not easily representable as +ranges. A bitwise constraint manager would model constraints such as "bit 32 +is known to be 1". This would help code that made use of bitmasks. +(Difficulty: Medium) + Track type info through casts more precisely. The DynamicTypePropagation checker is in charge of inferring a region's @@ -107,14 +114,6 @@ mailing list to notify other members display such paths in HTML output. (Difficulty: Medium) -Relate bugs to checkers / "bug types" -We need to come up with an API which will relate bug reports -to the checkers that produce them and refactor the existing code to use the -new API. This would allow us to identify the checker from the bug report, -which paves the way for selective control of certain checks. -(Difficulty: Easy-Medium) - - Refactor path diagnostic generation in http://clang.llvm.org/doxygen/BugReporter_8cpp_source.html";>BugReporter.cpp. It would be great to have more code reuse between "Minimal" and "Extensive" PathDiagnostic generation algorithms. One idea is to create an @@ -182,7 +181,7 @@ mailing list to notify other members Take a look at the http://pages.cs.wisc.edu/~shanlu/paper/TSE-CPMiner.pdf";>CP-Miner paper for inspiration. -(Difficulty: Medium-Hard) +(Difficulty: Medium-Hard; current contacts: Per Viberg and Daniel Fahlgren) ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205665 - Add a new subgroup to -Wtautological-compare, -Wtautological-overlap-compare,
Author: rtrieu Date: Sat Apr 5 00:17:01 2014 New Revision: 205665 URL: http://llvm.org/viewvc/llvm-project?rev=205665&view=rev Log: Add a new subgroup to -Wtautological-compare, -Wtautological-overlap-compare, which warns on compound conditionals that always evaluate to the same value. For instance, (x > 5 && x < 3) will always be false since no value for x can satisfy both conditions. This patch also changes the CFG to use these tautological values for better branch analysis. The test for -Wunreachable-code shows how this change catches additional dead code. Patch by Anders Rönnholm. Added: cfe/trunk/test/Sema/warn-overlap.c Modified: cfe/trunk/include/clang/Analysis/CFG.h cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td cfe/trunk/lib/Analysis/CFG.cpp cfe/trunk/lib/Sema/AnalysisBasedWarnings.cpp cfe/trunk/test/SemaCXX/warn-unreachable.cpp Modified: cfe/trunk/include/clang/Analysis/CFG.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/CFG.h?rev=205665&r1=205664&r2=205665&view=diff == --- cfe/trunk/include/clang/Analysis/CFG.h (original) +++ cfe/trunk/include/clang/Analysis/CFG.h Sat Apr 5 00:17:01 2014 @@ -47,6 +47,7 @@ namespace clang { class CXXRecordDecl; class CXXDeleteExpr; class CXXNewExpr; + class BinaryOperator; /// CFGElement - Represents a top-level expression in a basic block. class CFGElement { @@ -698,6 +699,15 @@ public: } }; +/// \brief CFGCallback defines methods that should be called when a logical +/// operator error is found when building the CFG. +class CFGCallback { +public: + CFGCallback() {} + virtual void compareAlwaysTrue(const BinaryOperator *B, bool isAlwaysTrue) {} + virtual ~CFGCallback() {} +}; + /// CFG - Represents a source-level, intra-procedural CFG that represents the /// control-flow of a Stmt. The Stmt can represent an entire function body, /// or a single expression. A CFG will always contain one empty block that @@ -716,7 +726,7 @@ public: public: typedef llvm::DenseMap ForcedBlkExprs; ForcedBlkExprs **forcedBlkExprs; - +CFGCallback *Observer; bool PruneTriviallyFalseEdges; bool AddEHEdges; bool AddInitializers; @@ -740,7 +750,7 @@ public: } BuildOptions() -: forcedBlkExprs(0), PruneTriviallyFalseEdges(true) +: forcedBlkExprs(0), Observer(0), PruneTriviallyFalseEdges(true) ,AddEHEdges(false) ,AddInitializers(false) ,AddImplicitDtors(false) Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=205665&r1=205664&r2=205665&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Sat Apr 5 00:17:01 2014 @@ -291,9 +291,11 @@ def StringPlusChar : DiagGroup<"string-p def StrncatSize : DiagGroup<"strncat-size">; def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">; def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">; +def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">; def TautologicalCompare : DiagGroup<"tautological-compare", [TautologicalOutOfRangeCompare, - TautologicalPointerCompare]>; + TautologicalPointerCompare, + TautologicalOverlapCompare]>; def HeaderHygiene : DiagGroup<"header-hygiene">; def DuplicateDeclSpecifier : DiagGroup<"duplicate-decl-specifier">; def CompareDistinctPointerType : DiagGroup<"compare-distinct-pointer-types">; Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=205665&r1=205664&r2=205665&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Sat Apr 5 00:17:01 2014 @@ -6383,6 +6383,9 @@ def note_ref_subobject_of_member_declare def warn_comparison_always : Warning< "%select{self-|array }0comparison always evaluates to %select{false|true|a constant}1">, InGroup; +def warn_tautological_overlap_comparison : Warning< + "overlapping comparisons always evaluate to %select{false|true}0">, + InGroup, DefaultIgnore; def warn_stringcompare : Warning< "result of comparison against %select{a string literal|@encode}0 is " Modified: cfe/trunk/lib/Analysis/CFG.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Analysis/CFG.cpp?rev=205665&r1=205664&r2=205665&view=diff ==
r205664 - Revert "DebugInfo: Place global constants in their appropriate context."
Author: dblaikie Date: Fri Apr 4 22:39:29 2014 New Revision: 205664 URL: http://llvm.org/viewvc/llvm-project?rev=205664&view=rev Log: Revert "DebugInfo: Place global constants in their appropriate context." This reverts commit r205655. Breaks the compiler-rt build with an assertion failure in LLVM... reverting while I investigate. Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp cfe/trunk/test/CodeGenCXX/debug-info.cpp Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=205664&r1=205663&r2=205664&view=diff == --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Apr 4 22:39:29 2014 @@ -3230,11 +3230,8 @@ void CGDebugInfo::EmitGlobalVariable(con // Do not emit separate definitions for function local const/statics. if (isa(VD->getDeclContext())) return; - llvm::DIDescriptor DContext = - getContextDescriptor(dyn_cast(VD->getDeclContext())); llvm::DIGlobalVariable GV = DBuilder.createStaticVariable( - DContext, Name, StringRef(), Unit, getLineNumber(VD->getLocation()), Ty, - true, Init, + Unit, Name, Name, Unit, getLineNumber(VD->getLocation()), Ty, true, Init, getOrCreateStaticDataMemberDeclarationOrNull(cast(VD))); DeclCache.insert(std::make_pair(VD->getCanonicalDecl(), llvm::WeakVH(GV))); } Modified: cfe/trunk/test/CodeGenCXX/debug-info.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info.cpp?rev=205664&r1=205663&r2=205664&view=diff == --- cfe/trunk/test/CodeGenCXX/debug-info.cpp (original) +++ cfe/trunk/test/CodeGenCXX/debug-info.cpp Fri Apr 4 22:39:29 2014 @@ -83,16 +83,9 @@ foo func(foo f) { // CHECK: [[FUNC:![0-9]*]] = {{.*}} metadata !"_ZN7pr147634funcENS_3fooE", i32 {{[0-9]*}}, metadata [[FUNC_TYPE:![0-9]*]], {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [def] [func] } -namespace local_const { -const wchar_t lc_c = L'x'; -} - -// CHECK: metadata [[LOCAL_CONST:![0-9]*]], metadata !"lc_c", {{.*}}; [ DW_TAG_variable ] [lc_c] -// CHECK: [[LOCAL_CONST]] = {{.*}}; [ DW_TAG_namespace ] [local_const] - void foo() { const wchar_t c = L'x'; - wchar_t d = c + local_const::lc_c; + wchar_t d = c; } // CHECK-NOT: ; [ DW_TAG_variable ] [c] ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205661 - [analyzer] Look through temporary destructors when finding a region to construct.
Author: jrose Date: Fri Apr 4 21:01:41 2014 New Revision: 205661 URL: http://llvm.org/viewvc/llvm-project?rev=205661&view=rev Log: [analyzer] Look through temporary destructors when finding a region to construct. Fixes a false positive when temporary destructors are enabled where a temporary is destroyed after a variable is constructed but before the VarDecl itself is processed, which occurs when the variable is in the condition of an if or while. Patch by Alex McCarthy, with an extra test from me. Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp cfe/trunk/test/Analysis/dtor.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp?rev=205661&r1=205660&r2=205661&view=diff == --- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Fri Apr 4 21:01:41 2014 @@ -113,8 +113,16 @@ static const MemRegion *getRegionForCons // See if we're constructing an existing region by looking at the next // element in the CFG. const CFGBlock *B = CurrBldrCtx.getBlock(); - if (CurrStmtIdx + 1 < B->size()) { -CFGElement Next = (*B)[CurrStmtIdx+1]; + unsigned int NextStmtIdx = CurrStmtIdx + 1; + if (NextStmtIdx < B->size()) { +CFGElement Next = (*B)[NextStmtIdx]; + +// Is this a destructor? If so, we might be in the middle of an assignment +// to a local or member: look ahead one more element to see what we find. +while (Next.getAs() && NextStmtIdx + 1 < B->size()) { + ++NextStmtIdx; + Next = (*B)[NextStmtIdx]; +} // Is this a constructor for a local variable? if (Optional StmtElem = Next.getAs()) { Modified: cfe/trunk/test/Analysis/dtor.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/dtor.cpp?rev=205661&r1=205660&r2=205661&view=diff == --- cfe/trunk/test/Analysis/dtor.cpp (original) +++ cfe/trunk/test/Analysis/dtor.cpp Fri Apr 4 21:01:41 2014 @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=destructors -Wno-null-dereference -verify %s +// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=destructors,cfg-temporary-dtors=true -Wno-null-dereference -verify %s void clang_analyzer_eval(bool); void clang_analyzer_checkInlined(bool); @@ -426,8 +426,14 @@ namespace LifetimeExtension { // This case used to cause an unexpected "Undefined or garbage value returned // to caller" warning - bool testNamedCustomDestructor() { -if (CheckCustomDestructor c = CheckCustomDestructor()) +// bool testNamedCustomDestructor() { +//if (CheckCustomDestructor c = CheckCustomDestructor()) +// return true; +//return false; +// } + + bool testMultipleTemporariesCustomDestructor() { +if (CheckCustomDestructor c = (CheckCustomDestructor(), CheckCustomDestructor())) return true; return false; } @@ -477,8 +483,7 @@ namespace NoReturn { void g2(int *x) { if (! x) NR(); -// FIXME: this shouldn't cause a warning. -*x = 47; // expected-warning{{Dereference of null pointer}} +*x = 47; // no warning } } ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Division by zero
On Apr 3, 2014, at 7:18 , Anders Rönnholm wrote: > Hi Jordan, see below for comments and questions. > > < Hi, Anders. I have some high-level concerns about this checker as it's > structured now. First (and most mundanely), the checker is conflating values > and locations. A particular symbol in the analyzer can there's no reason to ever have to remove something from the DivZeroMap > because of invalidation. > < > < int x = foo(); // The value stored in 'x' is some symbol '$foo' > < int y = 5 / x; // use $foo > < x = bar(); // The value stored in 'x' is now some symbol '$bar', but '$foo' > hasn't changed. > > < On the flip side, I'm concerned that values never leave the map. You could > clean up some of them with a checkDeadSymbols callback, but that's still a > large amount of state for values that may have been < checked quite a while > ago. In particular, this will result in false positives if a value is used in > one function and then checked in another. > > Why don't we have to remove it? When x changes as you say we need to remove > it or flag it so we don't check x against zero anymore since x has changed > after the division. > > I guess we could remove all items in DivZeroMap when we leave the function, > do you think that would work? "When 'x' changes" is the interesting part. '$foo' can't change, and that's what you're inserting into the map. The value stored in 'x' can change, and that's why 'x' is being removed from the map. But 'x' should never be in the map in the first place; it's '$foo', the value, that's important. Here's another example: int x = foo(); int y = x; use(5 / x); if (y == 0) // warn! If we track the variable 'x', then we won't catch this case, even though it's pretty much equivalent to what you had before. Here's another problem with doing things this way: doing this as a path-sensitive check says that there's a spurious comparison along one path, but you really need to know if it happens along all paths: void foo() { bool isPositive; int x = getValue(&isPositive); if (isPositive) { use(5 / x); } if (x == 0) { log("x is zero"); } } In this case, there exists a path where 'x' checked against 0 after being used as a denominator, but it's not actually a problem. You can argue that the test should be moved up before the use, or that an assertion should be added, but I don't know if we want to tackle that. OTOH, you would have to add an assertion if you did move the test before the use. I like the idea of clearing the map/set on function exit, because it's somewhat reasonable to add asserts within a function...but you'd basically also have to clear it on function entrance too. Which means you have one set per LocationContext. I haven't fully thought that through yet. > > < > < (The opposite order hit us so much when dealing with C++ support that we > coined a term for it: "inlined defensive checks". This occurs when one > function accepts null pointers and so does a check, thevalue is never null but doesn't actually assert it, and then the value is > used. We ended up having to silence all bugs where the first null check came > from an inlined function, which with the false ones.) > < > < What do you think? > < Jordan > < > < P.S. If your map entries always map to "true", you might as well use a set. > ;-) > > Is it possible to get a specific entry using set or do you have to iterate > through all items to find the one your looking for? > > With map you can do this,: > const bool D = State->get(MR); > > but with set i'm only able to get them all. > DivZeroMapTy DivZeroes = State->get(); For future reference, set traits get a State->contains(). Jordan___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r205646 - Vector [Sema]. Vector "splats" which are truncated should have a warning
I added a warning with r205521, for this and some other scalar->vector conversions. -- Steve Sent from my iPhone > On Apr 4, 2014, at 8:14 PM, jahanian wrote: > > This is strange. Before my patch, there was no warning for my test. This was > the reason behind the patch. > If my patch is no longer needed due to other changes which took place while I > was fixing this, I can take it out. > > - Fariborz > > >> On Apr 4, 2014, at 3:05 PM, Stephen Canon wrote: >> >> Hi Fariborz -- >> >> Before this patch, we were generating the following warning for your test >> case: >> >>implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned >> int') to 'ushort16' >> >> with your patch, we get the following instead: >> >>implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned >> int') to 'unsigned short' >> >> I have a slight preference for the warning we used to issue, as it refers to >> the conversion that actually takes place conceptually (scalar -> vector), >> rather than the "subconversion" that changes the value (scalar -> vector >> element). Reasonable people may disagree, of course. >> >> – Steve >> >>> On Apr 4, 2014, at 3:33 PM, Fariborz Jahanian wrote: >>> >>> Author: fjahanian >>> Date: Fri Apr 4 14:33:39 2014 >>> New Revision: 205646 >>> >>> URL: http://llvm.org/viewvc/llvm-project?rev=205646&view=rev >>> Log: >>> Vector [Sema]. Vector "splats" which are truncated should have a warning >>> with -Wconversion. // rdar://16502418 >>> >>> Modified: >>> cfe/trunk/lib/Sema/SemaChecking.cpp >>> cfe/trunk/test/Sema/conversion.c >>> >>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=205646&r1=205645&r2=205646&view=diff >>> == >>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) >>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Apr 4 14:33:39 2014 >>> @@ -6113,11 +6113,20 @@ void CheckConditionalOperator(Sema &S, C >>> /// implicit conversions in the given expression. There are a couple >>> /// of competing diagnostics here, -Wconversion and -Wsign-compare. >>> void AnalyzeImplicitConversions(Sema &S, Expr *OrigE, SourceLocation CC) { >>> - QualType T = OrigE->getType(); >>> Expr *E = OrigE->IgnoreParenImpCasts(); >>> >>> if (E->isTypeDependent() || E->isValueDependent()) >>> return; >>> + >>> + QualType T = OrigE->getType(); >>> + // Check for conversion from an arithmetic type to a vector of arithmetic >>> + // type elements. Warn if this results in truncation of each vector >>> element. >>> + if (isa(E)) { >>> +if (ImplicitCastExpr *ICE = dyn_cast(OrigE)) >>> + if ((ICE->getCastKind() == CK_VectorSplat) && >>> + !T.isNull() && T->isExtVectorType()) >>> +T = cast(T.getCanonicalType())->getElementType(); >>> + } >>> >>> // For conditional operators, we analyze the arguments as if they >>> // were being fed directly into the output. >>> >>> Modified: cfe/trunk/test/Sema/conversion.c >>> URL: >>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/conversion.c?rev=205646&r1=205645&r2=205646&view=diff >>> == >>> --- cfe/trunk/test/Sema/conversion.c (original) >>> +++ cfe/trunk/test/Sema/conversion.c Fri Apr 4 14:33:39 2014 >>> @@ -417,3 +417,15 @@ void test26(int si, long sl) { >>> si = si / sl; >>> si = sl / si; // expected-warning {{implicit conversion loses integer >>> precision: 'long' to 'int'}} >>> } >>> + >>> +// rdar://16502418 >>> +typedef unsigned short uint16_t; >>> +typedef unsigned int uint32_t; >>> +typedef __attribute__ ((ext_vector_type(16),__aligned__(32))) uint16_t >>> ushort16; >>> +typedef __attribute__ ((ext_vector_type( 8),__aligned__( 32))) uint32_t >>> uint8; >>> + >>> +void test27(ushort16 constants) { >>> +uint8 pairedConstants = (uint8) constants; >>> +ushort16 crCbScale = pairedConstants.s4; // expected-warning >>> {{implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned >>> int') to 'unsigned short'}} >>> +ushort16 brBias = pairedConstants.s6; // expected-warning {{implicit >>> conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to >>> 'unsigned short'}} >>> +} >>> >>> >>> ___ >>> cfe-commits mailing list >>> cfe-commits@cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r205646 - Vector [Sema]. Vector "splats" which are truncated should have a warning
This is strange. Before my patch, there was no warning for my test. This was the reason behind the patch. If my patch is no longer needed due to other changes which took place while I was fixing this, I can take it out. - Fariborz On Apr 4, 2014, at 3:05 PM, Stephen Canon wrote: > Hi Fariborz -- > > Before this patch, we were generating the following warning for your test > case: > > implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned > int') to 'ushort16' > > with your patch, we get the following instead: > > implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned > int') to 'unsigned short' > > I have a slight preference for the warning we used to issue, as it refers to > the conversion that actually takes place conceptually (scalar -> vector), > rather than the "subconversion" that changes the value (scalar -> vector > element). Reasonable people may disagree, of course. > > – Steve > > On Apr 4, 2014, at 3:33 PM, Fariborz Jahanian wrote: > >> Author: fjahanian >> Date: Fri Apr 4 14:33:39 2014 >> New Revision: 205646 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=205646&view=rev >> Log: >> Vector [Sema]. Vector "splats" which are truncated should have a warning >> with -Wconversion. // rdar://16502418 >> >> Modified: >> cfe/trunk/lib/Sema/SemaChecking.cpp >> cfe/trunk/test/Sema/conversion.c >> >> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=205646&r1=205645&r2=205646&view=diff >> == >> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Apr 4 14:33:39 2014 >> @@ -6113,11 +6113,20 @@ void CheckConditionalOperator(Sema &S, C >> /// implicit conversions in the given expression. There are a couple >> /// of competing diagnostics here, -Wconversion and -Wsign-compare. >> void AnalyzeImplicitConversions(Sema &S, Expr *OrigE, SourceLocation CC) { >> - QualType T = OrigE->getType(); >> Expr *E = OrigE->IgnoreParenImpCasts(); >> >> if (E->isTypeDependent() || E->isValueDependent()) >>return; >> + >> + QualType T = OrigE->getType(); >> + // Check for conversion from an arithmetic type to a vector of arithmetic >> + // type elements. Warn if this results in truncation of each vector >> element. >> + if (isa(E)) { >> +if (ImplicitCastExpr *ICE = dyn_cast(OrigE)) >> + if ((ICE->getCastKind() == CK_VectorSplat) && >> + !T.isNull() && T->isExtVectorType()) >> +T = cast(T.getCanonicalType())->getElementType(); >> + } >> >> // For conditional operators, we analyze the arguments as if they >> // were being fed directly into the output. >> >> Modified: cfe/trunk/test/Sema/conversion.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/conversion.c?rev=205646&r1=205645&r2=205646&view=diff >> == >> --- cfe/trunk/test/Sema/conversion.c (original) >> +++ cfe/trunk/test/Sema/conversion.c Fri Apr 4 14:33:39 2014 >> @@ -417,3 +417,15 @@ void test26(int si, long sl) { >> si = si / sl; >> si = sl / si; // expected-warning {{implicit conversion loses integer >> precision: 'long' to 'int'}} >> } >> + >> +// rdar://16502418 >> +typedef unsigned short uint16_t; >> +typedef unsigned int uint32_t; >> +typedef __attribute__ ((ext_vector_type(16),__aligned__(32))) uint16_t >> ushort16; >> +typedef __attribute__ ((ext_vector_type( 8),__aligned__( 32))) uint32_t >> uint8; >> + >> +void test27(ushort16 constants) { >> +uint8 pairedConstants = (uint8) constants; >> +ushort16 crCbScale = pairedConstants.s4; // expected-warning {{implicit >> conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to >> 'unsigned short'}} >> +ushort16 brBias = pairedConstants.s6; // expected-warning {{implicit >> conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to >> 'unsigned short'}} >> +} >> >> >> ___ >> cfe-commits mailing list >> cfe-commits@cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205659 - Objective-C arc [Sema]. Allow bridge cast of a qualified-id expression
Author: fjahanian Date: Fri Apr 4 18:53:45 2014 New Revision: 205659 URL: http://llvm.org/viewvc/llvm-project?rev=205659&view=rev Log: Objective-C arc [Sema]. Allow bridge cast of a qualified-id expression when bridged Objective-C type conforms to the protocols in CF types's Objective-C class. // rdar://16393330 Modified: cfe/trunk/lib/AST/ASTContext.cpp cfe/trunk/test/SemaObjC/objcbridge-attribute-arc.m cfe/trunk/test/SemaObjC/objcbridge-attribute.m cfe/trunk/test/SemaObjCXX/objcbridge-attribute-arc.mm cfe/trunk/test/SemaObjCXX/objcbridge-attribute.mm Modified: cfe/trunk/lib/AST/ASTContext.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=205659&r1=205658&r2=205659&view=diff == --- cfe/trunk/lib/AST/ASTContext.cpp (original) +++ cfe/trunk/lib/AST/ASTContext.cpp Fri Apr 4 18:53:45 2014 @@ -3559,12 +3559,28 @@ bool ASTContext::QIdProtocolsAdoptObjCOb CollectInheritedProtocols(IDecl, InheritedProtocols); if (InheritedProtocols.empty()) return false; - + // Check that if every protocol in list of id conforms to a protcol + // of IDecl's, then bridge casting is ok. + bool Conforms = false; + for (auto *Proto : OPT->quals()) { +Conforms = false; +for (auto *PI : InheritedProtocols) { + if (ProtocolCompatibleWithProtocol(Proto, PI)) { +Conforms = true; +break; + } +} +if (!Conforms) + break; + } + if (Conforms) +return true; + for (auto *PI : InheritedProtocols) { // If both the right and left sides have qualifiers. bool Adopts = false; for (auto *Proto : OPT->quals()) { - // return 'true' if '*PI' is in the inheritance hierarchy of Proto + // return 'true' if 'PI' is in the inheritance hierarchy of Proto if ((Adopts = ProtocolCompatibleWithProtocol(PI, Proto))) break; } Modified: cfe/trunk/test/SemaObjC/objcbridge-attribute-arc.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objcbridge-attribute-arc.m?rev=205659&r1=205658&r2=205659&view=diff == --- cfe/trunk/test/SemaObjC/objcbridge-attribute-arc.m (original) +++ cfe/trunk/test/SemaObjC/objcbridge-attribute-arc.m Fri Apr 4 18:53:45 2014 @@ -1,9 +1,9 @@ // RUN: %clang_cc1 -fsyntax-only -x objective-c -fobjc-arc -verify -Wno-objc-root-class %s // rdar://15454846 -typedef struct __attribute__ ((objc_bridge(NSError))) __CFErrorRef * CFErrorRef; // expected-note 7 {{declared here}} +typedef struct __attribute__ ((objc_bridge(NSError))) __CFErrorRef * CFErrorRef; // expected-note 5 {{declared here}} -typedef struct __attribute__ ((objc_bridge(MyError))) __CFMyErrorRef * CFMyErrorRef; // expected-note 3 {{declared here}} +typedef struct __attribute__ ((objc_bridge(MyError))) __CFMyErrorRef * CFMyErrorRef; // expected-note 1 {{declared here}} typedef struct __attribute__((objc_bridge(12))) __CFMyColor *CFMyColorRef; // expected-error {{parameter of 'objc_bridge' attribute must be a single name of an Objective-C class}} @@ -53,9 +53,9 @@ typedef CFErrorRef1 CFErrorRef2; // expe @protocol P4 @end @protocol P5 @end -@interface NSError @end // expected-note 7 {{declared here}} +@interface NSError @end // expected-note 5 {{declared here}} -@interface MyError : NSError // expected-note 3 {{declared here}} +@interface MyError : NSError // expected-note 1 {{declared here}} @end @interface NSUColor @end @@ -92,8 +92,8 @@ void Test5(id P123, id ID, i (void)(CFErrorRef)ID; // ok (void)(CFErrorRef)P123; // ok (void)(CFErrorRef)P1234; // ok - (void)(CFErrorRef)P12; // expected-warning {{'id' cannot bridge to 'CFErrorRef' (aka 'struct __CFErrorRef *')}} - (void)(CFErrorRef)P23; // expected-warning {{'id' cannot bridge to 'CFErrorRef' (aka 'struct __CFErrorRef *')}} + (void)(CFErrorRef)P12; + (void)(CFErrorRef)P23; } void Test6(id P123, id ID, id P1234, id P12, id P23) { @@ -101,21 +101,21 @@ void Test6(id P123, id ID, i (void)(CFMyErrorRef)ID; // ok (void)(CFMyErrorRef)P123; // ok (void)(CFMyErrorRef)P1234; // ok - (void)(CFMyErrorRef)P12; // expected-warning {{'id' cannot bridge to 'CFMyErrorRef' (aka 'struct __CFMyErrorRef *')}} - (void)(CFMyErrorRef)P23; // expected-warning {{'id' cannot bridge to 'CFMyErrorRef' (aka 'struct __CFMyErrorRef *')}} + (void)(CFMyErrorRef)P12; // ok + (void)(CFMyErrorRef)P23; // ok } -typedef struct __attribute__ ((objc_bridge(MyPersonalError))) __CFMyPersonalErrorRef * CFMyPersonalErrorRef; // expected-note 4 {{declared here}} +typedef struct __attribute__ ((objc_bridge(MyPersonalError))) __CFMyPersonalErrorRef * CFMyPersonalErrorRef; // expected-note 1 {{declared here}} -@interface MyPersonalError : NSError // expected-note 4 {{declared here}} +@interface MyPersonalError : NSError // expected-note 1 {{declared here}} @end void Test7(
Re: [PATCH] Add support for named values in the parser.
Comment at: lib/ASTMatchers/Dynamic/Parser.cpp:304 @@ +303,3 @@ + + // Parse as a matcher expressoin. + return parseMatcherExpressionImpl(NameToken, Value); Typo: "expression". Comment at: lib/ASTMatchers/Dynamic/Registry.cpp:334 @@ -333,2 +333,3 @@ if (it == RegistryData->constructors().end()) { -Error->addError(NameRange, Error->ET_RegistryNotFound) << MatcherName; +if (Error) { + Error->addError(NameRange, Error->ET_RegistryMatcherNotFound) I wonder if it would be better to drop the Error parameter from this function (and Sema::lookupMatcherCtor) and move diagnostic emission to the parser. That would be more consistent with how Sema::getNamedValue works, and would also allow you to do both lookups once, in parseIdentifierPrefixImpl. http://llvm-reviews.chandlerc.com/D3276 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205655 - DebugInfo: Place global constants in their appropriate context.
Author: dblaikie Date: Fri Apr 4 18:16:44 2014 New Revision: 205655 URL: http://llvm.org/viewvc/llvm-project?rev=205655&view=rev Log: DebugInfo: Place global constants in their appropriate context. We also don't need to duplicate the name in the LinkageName field. Just leave it empty. Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp cfe/trunk/test/CodeGenCXX/debug-info.cpp Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=205655&r1=205654&r2=205655&view=diff == --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Apr 4 18:16:44 2014 @@ -3230,8 +3230,11 @@ void CGDebugInfo::EmitGlobalVariable(con // Do not emit separate definitions for function local const/statics. if (isa(VD->getDeclContext())) return; + llvm::DIDescriptor DContext = + getContextDescriptor(dyn_cast(VD->getDeclContext())); llvm::DIGlobalVariable GV = DBuilder.createStaticVariable( - Unit, Name, Name, Unit, getLineNumber(VD->getLocation()), Ty, true, Init, + DContext, Name, StringRef(), Unit, getLineNumber(VD->getLocation()), Ty, + true, Init, getOrCreateStaticDataMemberDeclarationOrNull(cast(VD))); DeclCache.insert(std::make_pair(VD->getCanonicalDecl(), llvm::WeakVH(GV))); } Modified: cfe/trunk/test/CodeGenCXX/debug-info.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info.cpp?rev=205655&r1=205654&r2=205655&view=diff == --- cfe/trunk/test/CodeGenCXX/debug-info.cpp (original) +++ cfe/trunk/test/CodeGenCXX/debug-info.cpp Fri Apr 4 18:16:44 2014 @@ -83,9 +83,16 @@ foo func(foo f) { // CHECK: [[FUNC:![0-9]*]] = {{.*}} metadata !"_ZN7pr147634funcENS_3fooE", i32 {{[0-9]*}}, metadata [[FUNC_TYPE:![0-9]*]], {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [def] [func] } +namespace local_const { +const wchar_t lc_c = L'x'; +} + +// CHECK: metadata [[LOCAL_CONST:![0-9]*]], metadata !"lc_c", {{.*}}; [ DW_TAG_variable ] [lc_c] +// CHECK: [[LOCAL_CONST]] = {{.*}}; [ DW_TAG_namespace ] [local_const] + void foo() { const wchar_t c = L'x'; - wchar_t d = c; + wchar_t d = c + local_const::lc_c; } // CHECK-NOT: ; [ DW_TAG_variable ] [c] ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r205651 - DebugInfo: PR19298: function local const variables duplicated in the root scope
Hmm - this might not be the right long-term fix. We still have basically the same bug for a global const int: const int i = 3; void func(int*); int main() { func(&i); // references the global variable return i; // references the Constant } from this Clang describes two global variables and LLVM produces debug info describing both... I suppose what we want is to emit the constant location description when the variable itself is not emitted (such as when the address doesn't escape) and otherwise emit the global memory location perhaps a simple frontend lookup-and-replacement would suffice. Except in cases where the global variable is emitted initially and then optimized away by LLVM... oh, and we don't put the constant in the right namespace - but I'll probably just fix that immediately, even if the broader issue still stands :/ On Fri, Apr 4, 2014 at 1:56 PM, David Blaikie wrote: > Author: dblaikie > Date: Fri Apr 4 15:56:17 2014 > New Revision: 205651 > > URL: http://llvm.org/viewvc/llvm-project?rev=205651&view=rev > Log: > DebugInfo: PR19298: function local const variables duplicated in the root > scope > > See the comment for CodeGenFunction::tryEmitAsConstant that describes > how in some contexts (lambdas) we must not emit references to the > variable, but instead use the constant directly - because of this we end > up emitting a constant for the variable, as well as emitting the > variable itself. > > Should we just skip putting the variable on the stack at all and omit > the debug info for the constant? It's not clear to me - what if the > address of the local is taken? > > Modified: > cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > cfe/trunk/test/CodeGenCXX/debug-info.cpp > > Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=205651&r1=205650&r2=205651&view=diff > == > --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) > +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Apr 4 15:56:17 2014 > @@ -3227,6 +3227,9 @@ void CGDebugInfo::EmitGlobalVariable(con >// Do not use DIGlobalVariable for enums. >if (Ty.getTag() == llvm::dwarf::DW_TAG_enumeration_type) > return; > + // Do not emit separate definitions for function local const/statics. > + if (isa(VD->getDeclContext())) > +return; >llvm::DIGlobalVariable GV = DBuilder.createStaticVariable( >Unit, Name, Name, Unit, getLineNumber(VD->getLocation()), Ty, true, > Init, >getOrCreateStaticDataMemberDeclarationOrNull(cast(VD))); > > Modified: cfe/trunk/test/CodeGenCXX/debug-info.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info.cpp?rev=205651&r1=205650&r2=205651&view=diff > == > --- cfe/trunk/test/CodeGenCXX/debug-info.cpp (original) > +++ cfe/trunk/test/CodeGenCXX/debug-info.cpp Fri Apr 4 15:56:17 2014 > @@ -51,11 +51,6 @@ namespace VirtualBase { >} > } > > -void foo() { > - const wchar_t c = L'x'; > - wchar_t d = c; > -} > - > namespace b5249287 { > template class A { >struct B; > @@ -88,6 +83,13 @@ foo func(foo f) { > // CHECK: [[FUNC:![0-9]*]] = {{.*}} metadata !"_ZN7pr147634funcENS_3fooE", > i32 {{[0-9]*}}, metadata [[FUNC_TYPE:![0-9]*]], {{.*}} ; [ DW_TAG_subprogram > ] {{.*}} [def] [func] > } > > +void foo() { > + const wchar_t c = L'x'; > + wchar_t d = c; > +} > + > +// CHECK-NOT: ; [ DW_TAG_variable ] [c] > + > namespace pr9608 { // also pr9600 > struct incomplete; > incomplete (*x)[3]; > @@ -96,9 +98,11 @@ incomplete (*x)[3]; > // CHECK: [[INCARRAY]] = {{.*}}metadata !"_ZTSN6pr960810incompleteE", > metadata {{![0-9]*}}, i32 0, null, null, null} ; [ DW_TAG_array_type ] [line > 0, size 0, align 0, offset 0] [from _ZTSN6pr960810incompleteE] > } > > -// For some reason the argument for PR14763 ended up all the way down here > +// For some reason function arguments ended up down here > // CHECK: = metadata !{i32 {{[0-9]*}}, metadata [[FUNC]], {{.*}}, metadata > !"[[FOO]]", i32 8192, i32 0} ; [ DW_TAG_arg_variable ] [f] > > +// CHECK: ; [ DW_TAG_auto_variable ] [c] > + > namespace pr16214 { > struct a { >int i; > > > ___ > cfe-commits mailing list > cfe-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205653 - Try harder about not suggesting methods as corrections when they
Author: rikka Date: Fri Apr 4 17:16:30 2014 New Revision: 205653 URL: http://llvm.org/viewvc/llvm-project?rev=205653&view=rev Log: Try harder about not suggesting methods as corrections when they obviously won't work. Specifically, don't suggest methods (static or not) from unrelated classes when the expression is a method call through a specific object. Modified: cfe/trunk/include/clang/Sema/TypoCorrection.h cfe/trunk/lib/Sema/SemaExpr.cpp cfe/trunk/lib/Sema/SemaLookup.cpp cfe/trunk/lib/Sema/SemaOverload.cpp cfe/trunk/test/SemaCXX/typo-correction-pt2.cpp Modified: cfe/trunk/include/clang/Sema/TypoCorrection.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/TypoCorrection.h?rev=205653&r1=205652&r2=205653&view=diff == --- cfe/trunk/include/clang/Sema/TypoCorrection.h (original) +++ cfe/trunk/include/clang/Sema/TypoCorrection.h Fri Apr 4 17:16:30 2014 @@ -306,15 +306,15 @@ class FunctionCallFilterCCC : public Cor public: FunctionCallFilterCCC(Sema &SemaRef, unsigned NumArgs, bool HasExplicitTemplateArgs, -bool AllowNonStaticMethods = true); +MemberExpr *ME = 0); bool ValidateCandidate(const TypoCorrection &candidate) override; private: unsigned NumArgs; bool HasExplicitTemplateArgs; - bool AllowNonStaticMethods; DeclContext *CurContext; + MemberExpr *MemberFn; }; // @brief Callback class that effectively disabled typo correction Modified: cfe/trunk/lib/Sema/SemaExpr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=205653&r1=205652&r2=205653&view=diff == --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) +++ cfe/trunk/lib/Sema/SemaExpr.cpp Fri Apr 4 17:16:30 2014 @@ -3974,8 +3974,8 @@ namespace { class FunctionCallCCC : public FunctionCallFilterCCC { public: FunctionCallCCC(Sema &SemaRef, const IdentifierInfo *FuncName, - unsigned NumArgs, bool HasExplicitTemplateArgs) - : FunctionCallFilterCCC(SemaRef, NumArgs, HasExplicitTemplateArgs), + unsigned NumArgs, MemberExpr *ME) + : FunctionCallFilterCCC(SemaRef, NumArgs, false, ME), FunctionName(FuncName) {} bool ValidateCandidate(const TypoCorrection &candidate) override { @@ -3992,17 +3992,20 @@ private: }; } -static TypoCorrection TryTypoCorrectionForCall(Sema &S, - DeclarationNameInfo FuncName, +static TypoCorrection TryTypoCorrectionForCall(Sema &S, Expr *Fn, + FunctionDecl *FDecl, ArrayRef Args) { - FunctionCallCCC CCC(S, FuncName.getName().getAsIdentifierInfo(), - Args.size(), false); - if (TypoCorrection Corrected = - S.CorrectTypo(FuncName, Sema::LookupOrdinaryName, -S.getScopeForContext(S.CurContext), NULL, CCC)) { + MemberExpr *ME = dyn_cast(Fn); + DeclarationName FuncName = FDecl->getDeclName(); + SourceLocation NameLoc = ME ? ME->getMemberLoc() : Fn->getLocStart(); + FunctionCallCCC CCC(S, FuncName.getAsIdentifierInfo(), Args.size(), ME); + + if (TypoCorrection Corrected = S.CorrectTypo( + DeclarationNameInfo(FuncName, NameLoc), Sema::LookupOrdinaryName, + S.getScopeForContext(S.CurContext), NULL, CCC)) { if (NamedDecl *ND = Corrected.getCorrectionDecl()) { if (Corrected.isOverloaded()) { -OverloadCandidateSet OCS(FuncName.getLoc()); +OverloadCandidateSet OCS(NameLoc); OverloadCandidateSet::iterator Best; for (TypoCorrection::decl_iterator CD = Corrected.begin(), CDEnd = Corrected.end(); @@ -4011,7 +4014,7 @@ static TypoCorrection TryTypoCorrectionF S.AddOverloadCandidate(FD, DeclAccessPair::make(FD, AS_none), Args, OCS); } -switch (OCS.BestViableFunction(S, FuncName.getLoc(), Best)) { +switch (OCS.BestViableFunction(S, NameLoc, Best)) { case OR_Success: ND = Best->Function; Corrected.setCorrectionDecl(ND); @@ -4062,13 +4065,8 @@ Sema::ConvertArgumentsForCall(CallExpr * // arguments for the remaining parameters), don't make the call. if (Args.size() < NumParams) { if (Args.size() < MinArgs) { - MemberExpr *ME = dyn_cast(Fn); TypoCorrection TC; - if (FDecl && (TC = TryTypoCorrectionForCall( -*this, DeclarationNameInfo(FDecl->getDeclName(), - (ME ? ME->getMemberLoc() - : Fn->getLocStart())), -Args))) { + if (FDecl && (TC = TryTypoCorrectionForCall(*this, Fn, FDecl, Args))) {
Re: r205646 - Vector [Sema]. Vector "splats" which are truncated should have a warning
Hi Fariborz -- Before this patch, we were generating the following warning for your test case: implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to 'ushort16' with your patch, we get the following instead: implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to 'unsigned short' I have a slight preference for the warning we used to issue, as it refers to the conversion that actually takes place conceptually (scalar -> vector), rather than the "subconversion" that changes the value (scalar -> vector element). Reasonable people may disagree, of course. – Steve On Apr 4, 2014, at 3:33 PM, Fariborz Jahanian wrote: > Author: fjahanian > Date: Fri Apr 4 14:33:39 2014 > New Revision: 205646 > > URL: http://llvm.org/viewvc/llvm-project?rev=205646&view=rev > Log: > Vector [Sema]. Vector "splats" which are truncated should have a warning > with -Wconversion. // rdar://16502418 > > Modified: >cfe/trunk/lib/Sema/SemaChecking.cpp >cfe/trunk/test/Sema/conversion.c > > Modified: cfe/trunk/lib/Sema/SemaChecking.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=205646&r1=205645&r2=205646&view=diff > == > --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) > +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Apr 4 14:33:39 2014 > @@ -6113,11 +6113,20 @@ void CheckConditionalOperator(Sema &S, C > /// implicit conversions in the given expression. There are a couple > /// of competing diagnostics here, -Wconversion and -Wsign-compare. > void AnalyzeImplicitConversions(Sema &S, Expr *OrigE, SourceLocation CC) { > - QualType T = OrigE->getType(); > Expr *E = OrigE->IgnoreParenImpCasts(); > > if (E->isTypeDependent() || E->isValueDependent()) > return; > + > + QualType T = OrigE->getType(); > + // Check for conversion from an arithmetic type to a vector of arithmetic > + // type elements. Warn if this results in truncation of each vector > element. > + if (isa(E)) { > +if (ImplicitCastExpr *ICE = dyn_cast(OrigE)) > + if ((ICE->getCastKind() == CK_VectorSplat) && > + !T.isNull() && T->isExtVectorType()) > +T = cast(T.getCanonicalType())->getElementType(); > + } > > // For conditional operators, we analyze the arguments as if they > // were being fed directly into the output. > > Modified: cfe/trunk/test/Sema/conversion.c > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/conversion.c?rev=205646&r1=205645&r2=205646&view=diff > == > --- cfe/trunk/test/Sema/conversion.c (original) > +++ cfe/trunk/test/Sema/conversion.c Fri Apr 4 14:33:39 2014 > @@ -417,3 +417,15 @@ void test26(int si, long sl) { > si = si / sl; > si = sl / si; // expected-warning {{implicit conversion loses integer > precision: 'long' to 'int'}} > } > + > +// rdar://16502418 > +typedef unsigned short uint16_t; > +typedef unsigned int uint32_t; > +typedef __attribute__ ((ext_vector_type(16),__aligned__(32))) uint16_t > ushort16; > +typedef __attribute__ ((ext_vector_type( 8),__aligned__( 32))) uint32_t > uint8; > + > +void test27(ushort16 constants) { > +uint8 pairedConstants = (uint8) constants; > +ushort16 crCbScale = pairedConstants.s4; // expected-warning {{implicit > conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to > 'unsigned short'}} > +ushort16 brBias = pairedConstants.s6; // expected-warning {{implicit > conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to > 'unsigned short'}} > +} > > > ___ > cfe-commits mailing list > cfe-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow multiple modules with the same name to coexist in the module cache
On Fri, Apr 4, 2014 at 10:44 AM, Ben Langmuir wrote: > > On Apr 4, 2014, at 10:35 AM, Richard Smith wrote: > > > On 4 Apr 2014 09:09, "Ben Langmuir" wrote: > > > > > > On Apr 3, 2014, at 7:49 PM, Richard Smith wrote: > > > >> On Thu, Apr 3, 2014 at 3:40 PM, Ben Langmuir > wrote: > >>> > >>> > >>> On Mar 28, 2014, at 2:45 PM, Richard Smith > wrote: > >>> > On Fri, Mar 28, 2014 at 1:12 PM, Ben Langmuir > wrote: > > > > This patch allows multiple modules that have the same name to > coexist in the module cache. To differentiate between two modules with the > same name, we will consider the path the module map file that they are > defined by* part of the 'key' for looking up the precompiled module (pcm > file). Specifically, this patch renames the precompiled module (pcm) files > from > > > > cache-path//Foo.pcm > > > > to > > > > cache-path//Foo-.pcm > > > From a high level, I don't really see why we need a second hash here. > Shouldn't the -I options be included in the ? If I build the > same module with different -I flags, that should resolve to different .pcm > files, regardless of whether it makes the module name resolve to a > different module.map file. > > Are you trying to cope with the case where the -I path finds multiple > module.map files defining the same module (where it's basically chance > which one will get built and used)? I don't really feel like this is the > right solution to that problem either -- we should remove the 'luck' aspect > and use some sane mechanism to determine which module.map files are loaded, > and in what order. > > Is this addressing some other case? > > > > > > In addition, I've taught the ASTReader to re-resolve the names of > imported modules during module loading so that if the header search context > changes between when a module was originally built and when it is loaded we > can rebuild it if necessary. For example, if module A imports module B > > > > first time: > > clang -I /path/to/A -I /path/to/B ... > > > > second time: > > clang -I /path/to/A -I /different/path/to/B ... > > > > will now rebuild A as expected. > > > > > > * in the case of inferred modules, we use the module map file that > *allowed* the inference, not the __inferred_module.map file, since the > inferred file path is the same for every inferred module. > > > > Review comments on the patch itself: > > + /// the inferrence (e.g. contained 'module *') rather than the > virtual > > Typo 'inference', 'Module *'. > > + /// For an explanaition of \p ModuleMap, see Module::ModuleMap. > > Typo 'explanation'. > > + // safe becuase the FileManager is shared between the compiler > instances. > > Typo 'because' > > + // the inferred module. If this->ModuleMap is nullptr, then we are > using > + // -emit-module directly, and we cannot have an inferred module. > > I don't understand what this comment is trying to say. If we're using > -emit-module, then we were given a module map on the command-line; should > that not be referred to by this->ModuleMap? (Also, why 'this->'?) How can a > top-level module be inferred? Is that a framework-specific thing? > > +StringRef ModuleMap = this->ModuleMap ? > this->ModuleMap->getName() : InFile; > > Please pick a different variable name rather than shadowing a member > of '*this' here. > > +// Construct the name -.pcm > which should > +// be globally unique to this particular module. > +llvm::APInt Code(64, llvm::hash_value(ModuleMapPath)); > +SmallString<128> HashStr; > +Code.toStringUnsigned(HashStr); > > Use base 36, like the module hash. > >>> > >>> > >>> I've attached an updated patch. Changes since the previous one: > >>> 1. Fixed the typos and other issues Richard pointed out > >>> 2. I've added code to canonicalize the module map path (using > realpath); I was getting spurious failures on case-intensitive filesystems. > >> > >> > >> This part is probably not OK, because it'll do the wrong thing on some > build farms (where the canonical path is not predictable, but the path that > make_absolute returns is, by careful control of $PWD). I'll look into this > more, but will be traveling for the next couple of days. > > > > > > Okay, I have a better idea: I already store the actual module map path > in MODULE_MAP_FILE, so when that is read, we can just get a FileEntry for > it and compare to what header search finds for the current module (since > FileEntries are uniqued by inode). That also means I can give a better > diagnostic with the module map paths rather than the pcm filenames. > > Sounds great, thanks! > > > Nuts, it turns out I still need a canonical path - when we hash the path > into the pcm name we don't want to get a
[PATCH] Extend the check to detect patterns like 'ptr.get() == nullptr'
Hi djasper, Extend the check to detect patterns like 'ptr.get() == nullptr' It detects == and != when any argument is a ptr.get() and the other is a nullptr. Only supports standard smart pointer types std::unique_ptr and std::shared_ptr. Does not support the case 'ptr.get() == other.get()' yet. http://llvm-reviews.chandlerc.com/D3294 Files: clang-tidy/misc/RedundantSmartptrGet.cpp test/clang-tidy/make_compile_commands_json.sh test/clang-tidy/redundant-smartptr-get-fix.cpp test/clang-tidy/redundant-smartptr-get.cpp Index: clang-tidy/misc/RedundantSmartptrGet.cpp === --- clang-tidy/misc/RedundantSmartptrGet.cpp +++ clang-tidy/misc/RedundantSmartptrGet.cpp @@ -16,44 +16,69 @@ namespace clang { namespace tidy { -void RedundantSmartptrGet::registerMatchers(MatchFinder *Finder) { +namespace { +internal::Matcher callToGet(internal::Matcher OnClass) { + return memberCallExpr( + on(expr(anyOf(hasType(OnClass), + hasType(qualType(pointsTo(decl(OnClass).bind( + "ptr_to_ptr")).bind("smart_pointer")), + callee(methodDecl(hasName("get".bind("redundant_get"); +} + +void registerMatchersForGetArrowStart(MatchFinder *Finder, + MatchFinder::MatchCallback *Callback) { const auto QuacksLikeASmartptr = recordDecl( + recordDecl().bind("duck_typing"), has(methodDecl(hasName("operator->"), returns(qualType(pointsTo(type().bind("op->Type")), has(methodDecl(hasName("operator*"), returns(qualType(references(type().bind("op*Type")), has(methodDecl(hasName("get"), returns(qualType(pointsTo(type().bind("getType"))); - const auto CallToGet = - memberCallExpr(on(expr(hasType(recordDecl(QuacksLikeASmartptr))) -.bind("smart_pointer")), - callee(methodDecl(hasName("get".bind("redundant_get"); - - const auto ArrowCallToGet = - memberCallExpr( - on(expr(hasType(qualType(pointsTo(recordDecl(QuacksLikeASmartptr) - .bind("smart_pointer")), - callee(methodDecl(hasName("get".bind("redundant_get"); - // Catch 'ptr.get()->Foo()' - Finder->addMatcher( - memberExpr(isArrow(), hasObjectExpression(ignoringImpCasts(CallToGet))), - this); + Finder->addMatcher(memberExpr(expr().bind("memberExpr"), isArrow(), +hasObjectExpression(ignoringImpCasts( +callToGet(QuacksLikeASmartptr, + Callback); - // Catch '*ptr.get()' + // Catch '*ptr.get()' or '*ptr->get()' Finder->addMatcher( - unaryOperator(hasOperatorName("*"), hasUnaryOperand(CallToGet)), this); + unaryOperator(hasOperatorName("*"), +hasUnaryOperand(callToGet(QuacksLikeASmartptr))), + Callback); +} + +void registerMatchersForGetEquals(MatchFinder *Finder, + MatchFinder::MatchCallback *Callback) { + // This one is harder to do with duck typing. + // The operator==/!= that we are looking for might be member or non-member, + // might be on global namespace or found by ADL, might be a template, etc. + // For now, lets keep a list of known standard types. - // Catch '*ptr->get()' + const auto IsAKnownSmartptr = recordDecl( + anyOf(hasName("::std::unique_ptr"), hasName("::std::shard_ptr"))); + + // Matches against nullptr. Finder->addMatcher( - unaryOperator(hasOperatorName("*"), hasUnaryOperand(ArrowCallToGet)) - .bind("ptr_to_ptr"), - this); + binaryOperator(anyOf(hasOperatorName("=="), hasOperatorName("!=")), + hasEitherOperand(ignoringImpCasts(nullPtrLiteralExpr())), + hasEitherOperand(callToGet(IsAKnownSmartptr))), + Callback); + // TODO: Catch ptr.get() == other_ptr.get() +} + + +} // namespace + +void RedundantSmartptrGet::registerMatchers(MatchFinder *Finder) { + registerMatchersForGetArrowStart(Finder, this); + registerMatchersForGetEquals(Finder, this); } namespace { bool allReturnTypesMatch(const MatchFinder::MatchResult &Result) { + if (Result.Nodes.getNodeAs("duck_typing") == nullptr) return true; // Verify that the types match. // We can't do this on the matcher because the type nodes can be different, // even though they represent the same type. This difference comes from how @@ -71,14 +96,20 @@ void RedundantSmartptrGet::check(const MatchFinder::MatchResult &Result) { if (!allReturnTypesMatch(Result)) return; - bool IsPtrToPtr = Result.Nodes.getNodeAs("ptr_to_ptr") != nullptr; + bool IsPtrToPtr = Result.Nodes.getNodeAs("ptr_to_ptr") != nullptr; + bool IsMemberExpr = Result.Nodes.getNodeAs("memberExpr") != nullptr; const Expr *GetCall = Result.Nodes.getNodeAs("redundant_get"); const Expr
Re: [PATCH] Driver: add target definition for Windows on ARM
Committed with suggested changes. http://llvm-reviews.chandlerc.com/D3241 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205651 - DebugInfo: PR19298: function local const variables duplicated in the root scope
Author: dblaikie Date: Fri Apr 4 15:56:17 2014 New Revision: 205651 URL: http://llvm.org/viewvc/llvm-project?rev=205651&view=rev Log: DebugInfo: PR19298: function local const variables duplicated in the root scope See the comment for CodeGenFunction::tryEmitAsConstant that describes how in some contexts (lambdas) we must not emit references to the variable, but instead use the constant directly - because of this we end up emitting a constant for the variable, as well as emitting the variable itself. Should we just skip putting the variable on the stack at all and omit the debug info for the constant? It's not clear to me - what if the address of the local is taken? Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp cfe/trunk/test/CodeGenCXX/debug-info.cpp Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=205651&r1=205650&r2=205651&view=diff == --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Apr 4 15:56:17 2014 @@ -3227,6 +3227,9 @@ void CGDebugInfo::EmitGlobalVariable(con // Do not use DIGlobalVariable for enums. if (Ty.getTag() == llvm::dwarf::DW_TAG_enumeration_type) return; + // Do not emit separate definitions for function local const/statics. + if (isa(VD->getDeclContext())) +return; llvm::DIGlobalVariable GV = DBuilder.createStaticVariable( Unit, Name, Name, Unit, getLineNumber(VD->getLocation()), Ty, true, Init, getOrCreateStaticDataMemberDeclarationOrNull(cast(VD))); Modified: cfe/trunk/test/CodeGenCXX/debug-info.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info.cpp?rev=205651&r1=205650&r2=205651&view=diff == --- cfe/trunk/test/CodeGenCXX/debug-info.cpp (original) +++ cfe/trunk/test/CodeGenCXX/debug-info.cpp Fri Apr 4 15:56:17 2014 @@ -51,11 +51,6 @@ namespace VirtualBase { } } -void foo() { - const wchar_t c = L'x'; - wchar_t d = c; -} - namespace b5249287 { template class A { struct B; @@ -88,6 +83,13 @@ foo func(foo f) { // CHECK: [[FUNC:![0-9]*]] = {{.*}} metadata !"_ZN7pr147634funcENS_3fooE", i32 {{[0-9]*}}, metadata [[FUNC_TYPE:![0-9]*]], {{.*}} ; [ DW_TAG_subprogram ] {{.*}} [def] [func] } +void foo() { + const wchar_t c = L'x'; + wchar_t d = c; +} + +// CHECK-NOT: ; [ DW_TAG_variable ] [c] + namespace pr9608 { // also pr9600 struct incomplete; incomplete (*x)[3]; @@ -96,9 +98,11 @@ incomplete (*x)[3]; // CHECK: [[INCARRAY]] = {{.*}}metadata !"_ZTSN6pr960810incompleteE", metadata {{![0-9]*}}, i32 0, null, null, null} ; [ DW_TAG_array_type ] [line 0, size 0, align 0, offset 0] [from _ZTSN6pr960810incompleteE] } -// For some reason the argument for PR14763 ended up all the way down here +// For some reason function arguments ended up down here // CHECK: = metadata !{i32 {{[0-9]*}}, metadata [[FUNC]], {{.*}}, metadata !"[[FOO]]", i32 8192, i32 0} ; [ DW_TAG_arg_variable ] [f] +// CHECK: ; [ DW_TAG_auto_variable ] [c] + namespace pr16214 { struct a { int i; ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Add support for named values in the parser.
Added the check you asked for. It will say "Value not found" only when it think you really meant a to use a value. http://llvm-reviews.chandlerc.com/D3276 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Add support for named values in the parser.
Added "Value not found" error. Added explicit conversion to bool on VariantValue. Replaced isNothing() with hasValue(). Hi pcc, http://llvm-reviews.chandlerc.com/D3276 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D3276?vs=8338&id=8383#toc Files: include/clang/ASTMatchers/Dynamic/Diagnostics.h include/clang/ASTMatchers/Dynamic/Parser.h include/clang/ASTMatchers/Dynamic/VariantValue.h lib/ASTMatchers/Dynamic/Diagnostics.cpp lib/ASTMatchers/Dynamic/Parser.cpp lib/ASTMatchers/Dynamic/Registry.cpp unittests/ASTMatchers/Dynamic/ParserTest.cpp unittests/ASTMatchers/Dynamic/VariantValueTest.cpp Index: include/clang/ASTMatchers/Dynamic/Diagnostics.h === --- include/clang/ASTMatchers/Dynamic/Diagnostics.h +++ include/clang/ASTMatchers/Dynamic/Diagnostics.h @@ -60,11 +60,12 @@ enum ErrorType { ET_None = 0, -ET_RegistryNotFound = 1, +ET_RegistryMatcherNotFound = 1, ET_RegistryWrongArgCount = 2, ET_RegistryWrongArgType = 3, ET_RegistryNotBindable = 4, ET_RegistryAmbiguousOverload = 5, +ET_RegistryValueNotFound = 6, ET_ParserStringError = 100, ET_ParserNoOpenParen = 101, Index: include/clang/ASTMatchers/Dynamic/Parser.h === --- include/clang/ASTMatchers/Dynamic/Parser.h +++ include/clang/ASTMatchers/Dynamic/Parser.h @@ -18,13 +18,14 @@ /// /// \code /// Grammar for the expressions supported: -/// := | +/// := | | ///:= | /// := "quoted string" /// := [0-9]+ -/// := () | -///().bind() -///:= [a-zA-Z]+ +/// := +/// := () | +///().bind() +/// := [a-zA-Z]+ /// := | , /// \endcode /// @@ -62,6 +63,19 @@ public: virtual ~Sema(); +/// \brief Lookup a value by name. +/// +/// This can be used in the Sema layer to declare known constants or to +/// allow to split an expression in pieces. +/// +/// \param Name The name of the value to lookup. +/// +/// \return The named value. It could be any type that VariantValue +/// supports. A 'nothing' value means that the name is not recognized. +virtual VariantValue getNamedValue(StringRef Name) { + return VariantValue(); +} + /// \brief Process a matcher expression. /// /// All the arguments passed here have already been processed. @@ -100,6 +114,21 @@ Diagnostics *Error) = 0; }; + /// \brief Sema implementation that uses the matcher registry to process the + /// tokens. + class RegistrySema : public Parser::Sema { + public: +virtual ~RegistrySema(); +llvm::Optional lookupMatcherCtor(StringRef MatcherName, + const SourceRange &NameRange, + Diagnostics *Error) override; +VariantMatcher actOnMatcherExpression(MatcherCtor Ctor, + const SourceRange &NameRange, + StringRef BindID, + ArrayRef Args, + Diagnostics *Error) override; + }; + /// \brief Parse a matcher expression, creating matchers from the registry. /// /// This overload creates matchers calling directly into the registry. If the @@ -160,7 +189,9 @@ Diagnostics *Error); bool parseExpressionImpl(VariantValue *Value); - bool parseMatcherExpressionImpl(VariantValue *Value); + bool parseMatcherExpressionImpl(const TokenInfo &NameToken, + VariantValue *Value); + bool parseIdentifierPrefixImpl(VariantValue *Value); void addCompletion(const TokenInfo &CompToken, StringRef TypedText, StringRef Decl); Index: include/clang/ASTMatchers/Dynamic/VariantValue.h === --- include/clang/ASTMatchers/Dynamic/VariantValue.h +++ include/clang/ASTMatchers/Dynamic/VariantValue.h @@ -78,7 +78,8 @@ /// \brief Clones the provided matchers. /// /// They should be the result of a polymorphic matcher. - static VariantMatcher PolymorphicMatcher(std::vector Matchers); + static VariantMatcher + PolymorphicMatcher(std::vector Matchers); /// \brief Creates a 'variadic' operator matcher. /// @@ -208,6 +209,10 @@ VariantValue(const std::string &String); VariantValue(const VariantMatcher &Matchers); + /// \brief Returns true iff this is not an empty value. + LLVM_EXPLICIT operator bool() const { return hasValue(); } + bool hasValue() const { return Type != VT_Nothing; } + /// \brief Unsigned value functions. bool isUnsigned() const; unsigned getUnsigned() const; Index: lib/ASTMatchers/Dynamic/Diagnostics.cpp
r205650 - Driver: add target definition for Windows on ARM
Author: compnerd Date: Fri Apr 4 15:31:19 2014 New Revision: 205650 URL: http://llvm.org/viewvc/llvm-project?rev=205650&view=rev Log: Driver: add target definition for Windows on ARM This introduces the definitions needed for the Windows on ARM target. Add target definitions for both the MSVC environment and the MSVC + Itanium C++ ABI environment. The Visual Studio definitions correspond to the definitions provided by Visual Studio 2012. Added: cfe/trunk/test/Driver/windows-arm-minimal-arch.c cfe/trunk/test/Parser/arm-windows-calling-convention-handling.c cfe/trunk/test/Preprocessor/woa-defaults.c Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td cfe/trunk/lib/Basic/Targets.cpp cfe/trunk/lib/Driver/ToolChain.cpp cfe/trunk/lib/Driver/Tools.cpp Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=205650&r1=205649&r2=205650&view=diff == --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Fri Apr 4 15:31:19 2014 @@ -82,6 +82,9 @@ def err_drv_invalid_libcxx_deployment : def err_drv_malformed_sanitizer_blacklist : Error< "malformed sanitizer blacklist: '%0'">; +def err_target_unsupported_arch + : Error<"the target architecture '%0' is not supported by the target '%1'">; + def err_drv_I_dash_not_supported : Error< "'%0' not supported, please use -iquote instead">; def err_drv_unknown_argument : Error<"unknown argument: '%0'">; Modified: cfe/trunk/lib/Basic/Targets.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=205650&r1=205649&r2=205650&view=diff == --- cfe/trunk/lib/Basic/Targets.cpp (original) +++ cfe/trunk/lib/Basic/Targets.cpp Fri Apr 4 15:31:19 2014 @@ -3708,6 +3708,9 @@ class ARMTargetInfo : public TargetInfo static const Builtin::Info BuiltinInfo[]; static bool shouldUseInlineAtomic(const llvm::Triple &T) { +if (T.isOSWindows()) + return true; + // On linux, binaries targeting old cpus call functions in libgcc to // perform atomic operations. The implementation in libgcc then calls into // the kernel which on armv6 and newer uses ldrex and strex. The net result @@ -3774,19 +3777,30 @@ class ARMTargetInfo : public TargetInfo if (IsThumb) { // Thumb1 add sp, #imm requires the immediate value be multiple of 4, // so set preferred for small types to 32. - if (T.isOSBinFormatMachO()) + if (T.isOSBinFormatMachO()) { DescriptionString = BigEndian ? "E-m:o-p:32:32-i1:8:32-i8:8:32-i16:16:32-i64:64-" "v128:64:128-a:0:32-n32-S64" : "e-m:o-p:32:32-i1:8:32-i8:8:32-i16:16:32-i64:64-" "v128:64:128-a:0:32-n32-S64"; - else + } else if (T.isOSWindows()) { +// FIXME: this is invalid for WindowsCE +assert(!BigEndian && "Windows on ARM does not support big endian"); +DescriptionString = "e" +"-m:e" +"-p:32:32" +"-i1:8:32-i8:8:32-i16:16:32-i64:64" +"-v128:64:128" +"-a:0:32" +"-n32" +"-S64"; + } else { DescriptionString = BigEndian ? "E-m:e-p:32:32-i1:8:32-i8:8:32-i16:16:32-i64:64-" "v128:64:128-a:0:32-n32-S64" : "e-m:e-p:32:32-i1:8:32-i8:8:32-i16:16:32-i64:64-" "v128:64:128-a:0:32-n32-S64"; - + } } else { if (T.isOSBinFormatMachO()) DescriptionString = BigEndian ? @@ -4093,12 +4107,14 @@ public: // FIXME: It's more complicated than this and we don't really support // interworking. -if (5 <= CPUArchVer && CPUArchVer <= 8) +// Windows on ARM does not "support" interworking +if (5 <= CPUArchVer && CPUArchVer <= 8 && !getTriple().isOSWindows()) Builder.defineMacro("__THUMB_INTERWORK__"); if (ABI == "aapcs" || ABI == "aapcs-linux" || ABI == "aapcs-vfp") { // Embedded targets on Darwin follow AAPCS, but not EABI. - if (!getTriple().isOSDarwin()) + // Windows on ARM follows AAPCS VFP, but does not conform to EABI. + if (!getTriple().isOSDarwin() && !getTriple().isOSWindows()) Builder.defineMacro("__ARM_EABI__"); Builder.defineMacro("__ARM_PCS", "1"); @@ -4371,6 +4387,72 @@ public: } // end anonymous namespace. namespace { +class WindowsARMTargetInfo : public WindowsTargetInfo { + const llvm::Triple Triple; +public: + WindowsARMTargetInfo(con
Re: clang-format: better detection for multiplication operator
On Sun, Mar 30, 2014 at 07:45:51PM +0200, Daniel Jasper wrote: > All in all, I think a much easier solution for the mentioned bug would be > to treat "<<" similar to how we treat assignments, i.e. if we find a <<, we > assume the RHS to be an expression (that's the very first "if" in > determineTokenType()). > Not sure if that's the right place. The "if" branch you are talking about will set all preceding stars (on the same level) to TT_PointerOrReference: Before: cout << a *a << b *b << c *c; After: cout << a *a << b *b << c * b; My Fix: cout << a * a << b * b << c * c; So we need the special care for operator<< at least in a new "else if". I've added a "light" version of this patch below, note that it only handles operator<< and doesn't work with other operators. Doing it the way I did in the last mail has the additional benefit that it works for all operators and not just "<<" (in particular operator+ is hard to get otherwise). But we probably won't need this. -- >From 16d81e7e3a0dedd5d51f963a134b26eb9c63b777 Mon Sep 17 00:00:00 2001 From: Kapfhammer Date: Sun, 30 Mar 2014 12:04:17 +0200 Subject: [PATCH] Use binary operators as an indicator of for an expression and extend the test suite. This solves the issue where the star was too often interpreted as a pointer, e.g "coutisOneOf(tok::kw_template, tok::kw_using) && + (!Current.Previous || +Current.Previous->isNot(tok::kw_operator))) { + Contexts.back().IsExpression = true; } else if (Current.isOneOf(tok::kw_return, tok::kw_throw)) { Contexts.back().IsExpression = true; } else if (Current.is(tok::l_paren) && !Line.MustBeDeclaration && diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 5395fd9..7478a23 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -4625,6 +4625,21 @@ TEST_F(FormatTest, UnderstandsRvalueReferences) { verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))"); } +TEST_F(FormatTest, FormatsMultiplicationOperator) { + verifyFormat("operator*(type *a)"); + verifyFormat("operator<<(type *a)"); + verifyFormat("{ cout << (a * b); }"); + verifyFormat("{ cout << a * b; }"); + verifyFormat("{ cout << a * b << c * d; }"); + verifyFormat("type (*f)(type *a)"); + verifyFormat("type (&f)(type *a)"); + verifyFormat("void f(type *a)"); + verifyFormat("using f = void (*)(a *b)"); + verifyFormat("template "); +} + TEST_F(FormatTest, FormatsBinaryOperatorsPrecedingEquals) { verifyFormat("void f() {\n" " x[a -\n" -- 1.9.1 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205646 - Vector [Sema]. Vector "splats" which are truncated should have a warning
Author: fjahanian Date: Fri Apr 4 14:33:39 2014 New Revision: 205646 URL: http://llvm.org/viewvc/llvm-project?rev=205646&view=rev Log: Vector [Sema]. Vector "splats" which are truncated should have a warning with -Wconversion. // rdar://16502418 Modified: cfe/trunk/lib/Sema/SemaChecking.cpp cfe/trunk/test/Sema/conversion.c Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=205646&r1=205645&r2=205646&view=diff == --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Apr 4 14:33:39 2014 @@ -6113,11 +6113,20 @@ void CheckConditionalOperator(Sema &S, C /// implicit conversions in the given expression. There are a couple /// of competing diagnostics here, -Wconversion and -Wsign-compare. void AnalyzeImplicitConversions(Sema &S, Expr *OrigE, SourceLocation CC) { - QualType T = OrigE->getType(); Expr *E = OrigE->IgnoreParenImpCasts(); if (E->isTypeDependent() || E->isValueDependent()) return; + + QualType T = OrigE->getType(); + // Check for conversion from an arithmetic type to a vector of arithmetic + // type elements. Warn if this results in truncation of each vector element. + if (isa(E)) { +if (ImplicitCastExpr *ICE = dyn_cast(OrigE)) + if ((ICE->getCastKind() == CK_VectorSplat) && + !T.isNull() && T->isExtVectorType()) +T = cast(T.getCanonicalType())->getElementType(); + } // For conditional operators, we analyze the arguments as if they // were being fed directly into the output. Modified: cfe/trunk/test/Sema/conversion.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/conversion.c?rev=205646&r1=205645&r2=205646&view=diff == --- cfe/trunk/test/Sema/conversion.c (original) +++ cfe/trunk/test/Sema/conversion.c Fri Apr 4 14:33:39 2014 @@ -417,3 +417,15 @@ void test26(int si, long sl) { si = si / sl; si = sl / si; // expected-warning {{implicit conversion loses integer precision: 'long' to 'int'}} } + +// rdar://16502418 +typedef unsigned short uint16_t; +typedef unsigned int uint32_t; +typedef __attribute__ ((ext_vector_type(16),__aligned__(32))) uint16_t ushort16; +typedef __attribute__ ((ext_vector_type( 8),__aligned__( 32))) uint32_t uint8; + +void test27(ushort16 constants) { +uint8 pairedConstants = (uint8) constants; +ushort16 crCbScale = pairedConstants.s4; // expected-warning {{implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to 'unsigned short'}} +ushort16 brBias = pairedConstants.s6; // expected-warning {{implicit conversion loses integer precision: 'uint32_t' (aka 'unsigned int') to 'unsigned short'}} +} ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Driver: add target definition for Windows on ARM
LGTM Comment at: lib/Basic/Targets.cpp:4425-4426 @@ +4424,4 @@ +// TODO base this on the actual triple +Builder.defineMacro("_M_ARM", "7"); +// TODO map the complete set of values +// 31: VFPv3 40: VFPv4 Saleem Abdulrasool wrote: > Reid Kleckner wrote: > > Any reason not to just do that? > I couldnt figure out how to get MSVC to provide the values for _M_ARM_FP that > it supports. Oops, I was referring to the _M_ARM macro. It seems like just a matter of extracting the version number as a string. Comment at: lib/Basic/Targets.cpp:4437 @@ +4436,3 @@ +: WindowsARMTargetInfo(Triple) { +TheCXXABI.set(TargetCXXABI::GenericItanium); + } Saleem Abdulrasool wrote: > Reid Kleckner wrote: > > Any reason not to use GenericARM? GenericItanium uses a bit to implement > > member pointers that conflicts with the thumb bit of the PC. > Ah, I thought GenericARM was the ARM C++ ABI rather than IA-64 on ARM, fine > by me to switch to that. The ARM C++ ABI is derived from the Itanium C++ ABI. It's mostly a bunch of bugfixes (virtual inline functions cannot be key functions) and member pointer tweaks for thumb. Comment at: lib/Basic/Targets.cpp:4415 @@ +4414,3 @@ + +// Windows ARM + IA-64 C++ ABI Target +class ItaniumWindowsARMleTargetInfo : public WindowsARMTargetInfo { Just call it the "Itanium C++ ABI". That's the title of the actual document: http://mentorembedded.github.io/cxx-abi/abi.html http://llvm-reviews.chandlerc.com/D3241 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Warn when a virtual function tries to override a non-virtual one
Did you evaluate this on some large codebase for true and false positives? The current behavior of -Woverloaded-virtual was carefully chosen to be less noisy than gcc's version of the warning (clang's version is low enough on noise to be useful, gcc's isn't). On Fri, Apr 4, 2014 at 10:54 AM, Lubos Lunak wrote: > > Testcase: > > struct B5 { > void func(); > }; > struct S5 : public B5 { > virtual void func(); > }; > > Here most likely S5::func() was meant to override B5::func() but doesn't > because of missing 'virtual' in the base class. The attached patch warns > about this case. I added the warning to -Woverloaded-virtual, because > although technically it is not an overloaded virtual, it is exactly the > same > kind of a developer mistake, but if somebody insists I can update the patch > to make it -Woverridden-non-virtual (which I think is a misnomer too :) ) > or > whatever you name it. > > -- > Lubos Lunak > > ___ > cfe-commits mailing list > cfe-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[PATCH] Warn when a virtual function tries to override a non-virtual one
Testcase: struct B5 { void func(); }; struct S5 : public B5 { virtual void func(); }; Here most likely S5::func() was meant to override B5::func() but doesn't because of missing 'virtual' in the base class. The attached patch warns about this case. I added the warning to -Woverloaded-virtual, because although technically it is not an overloaded virtual, it is exactly the same kind of a developer mistake, but if somebody insists I can update the patch to make it -Woverridden-non-virtual (which I think is a misnomer too :) ) or whatever you name it. -- Lubos Lunak From 5da1b420e4b0fd7c828876aaff31cb915687a1ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Fri, 4 Apr 2014 19:43:48 +0200 Subject: [PATCH] warn when a virtual function tries to override a non-virtual one --- include/clang/Basic/DiagnosticSemaKinds.td | 5 lib/Sema/SemaDecl.cpp | 37 ++ test/SemaCXX/warn-overloaded-virtual.cpp | 8 +++ 3 files changed, 41 insertions(+), 9 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index fad654f..90a8aeb 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -5176,6 +5176,11 @@ def note_hidden_overloaded_virtual_declared_here : Note< "volatile and restrict|const, volatile, and restrict}2 vs " "%select{none|const|restrict|const and restrict|volatile|const and volatile|" "volatile and restrict|const, volatile, and restrict}3)}1">; +def warn_virtual_function_overriding_non_virtual : Warning< + "virtual %q0 does not override non-virtual function in a base class">, + InGroup, DefaultIgnore; +def note_overridden_non_virtual_declared_here : Note< + "non-virtual function %q0 declared here">; def warn_using_directive_in_header : Warning< "using namespace directive in global context in header">, InGroup, DefaultIgnore; diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 43d855c..f447692 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -5973,6 +5973,8 @@ struct FindOverriddenMethodData { /// \brief Member lookup function that determines whether a given C++ /// method overrides a method in a base class, to be used with /// CXXRecordDecl::lookupInBases(). +/// Note that this returns a method in a base class even if that method +/// is not virtual and thus not actually overridden (to be used for a warning). static bool FindOverriddenMethod(const CXXBaseSpecifier *Specifier, CXXBasePath &Path, void *UserData) { @@ -5997,7 +5999,7 @@ static bool FindOverriddenMethod(const CXXBaseSpecifier *Specifier, Path.Decls = Path.Decls.slice(1)) { NamedDecl *D = Path.Decls.front(); if (CXXMethodDecl *MD = dyn_cast(D)) { - if (MD->isVirtual() && !Data->S->IsOverload(Data->Method, MD, false)) + if (!Data->S->IsOverload(Data->Method, MD, false)) return true; } } @@ -6041,17 +6043,34 @@ bool Sema::AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD) { bool hasDeletedOverridenMethods = false; bool hasNonDeletedOverridenMethods = false; bool AddedAny = false; + bool ignoreOverloadedVirtual = + (Diags.getDiagnosticLevel(diag::warn_overloaded_virtual, +MD->getLocation()) == + DiagnosticsEngine::Ignored); if (DC->lookupInBases(&FindOverriddenMethod, &Data, Paths)) { for (auto *I : Paths.found_decls()) { if (CXXMethodDecl *OldMD = dyn_cast(I)) { -MD->addOverriddenMethod(OldMD->getCanonicalDecl()); -if (!CheckOverridingFunctionReturnType(MD, OldMD) && -!CheckOverridingFunctionAttributes(MD, OldMD) && -!CheckOverridingFunctionExceptionSpec(MD, OldMD) && -!CheckIfOverriddenFunctionIsMarkedFinal(MD, OldMD)) { - hasDeletedOverridenMethods |= OldMD->isDeleted(); - hasNonDeletedOverridenMethods |= !OldMD->isDeleted(); - AddedAny = true; +if (OldMD->isVirtual()) { + MD->addOverriddenMethod(OldMD->getCanonicalDecl()); + if (!CheckOverridingFunctionReturnType(MD, OldMD) && + !CheckOverridingFunctionAttributes(MD, OldMD) && + !CheckOverridingFunctionExceptionSpec(MD, OldMD) && + !CheckIfOverriddenFunctionIsMarkedFinal(MD, OldMD)) { +hasDeletedOverridenMethods |= OldMD->isDeleted(); +hasNonDeletedOverridenMethods |= !OldMD->isDeleted(); +AddedAny = true; + } +} else { + // Warn if this method has virtual explicitly written but the one + // in a base class is not virtual. + if (!ignoreOverloadedVirtual && MD->isVirtualAsWritten()) { +Diag(MD->getLocation(), + diag::warn_virtual_function_overriding_non_virtual) +<< MD; +
Re: [PATCH] Add support for optimization reports.
Hi Diego, For the part I can comment on (OptimizationRemarkHandler), this LGTM. For the rest, I leave to Richard the final OK. Thanks, Quentin http://llvm-reviews.chandlerc.com/D3226 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow multiple modules with the same name to coexist in the module cache
On Apr 4, 2014, at 10:35 AM, Richard Smith wrote: > > On 4 Apr 2014 09:09, "Ben Langmuir" wrote: > > > > > > On Apr 3, 2014, at 7:49 PM, Richard Smith wrote: > > > >> On Thu, Apr 3, 2014 at 3:40 PM, Ben Langmuir wrote: > >>> > >>> > >>> On Mar 28, 2014, at 2:45 PM, Richard Smith wrote: > >>> > On Fri, Mar 28, 2014 at 1:12 PM, Ben Langmuir > wrote: > > > > This patch allows multiple modules that have the same name to coexist > > in the module cache. To differentiate between two modules with the > > same name, we will consider the path the module map file that they are > > defined by* part of the ‘key’ for looking up the precompiled module > > (pcm file). Specifically, this patch renames the precompiled module > > (pcm) files from > > > > cache-path//Foo.pcm > > > > to > > > > cache-path//Foo-.pcm > > > From a high level, I don't really see why we need a second hash here. > Shouldn't the -I options be included in the ? If I build > the same module with different -I flags, that should resolve to > different .pcm files, regardless of whether it makes the module name > resolve to a different module.map file. > > Are you trying to cope with the case where the -I path finds multiple > module.map files defining the same module (where it's basically chance > which one will get built and used)? I don't really feel like this is the > right solution to that problem either -- we should remove the 'luck' > aspect and use some sane mechanism to determine which module.map files > are loaded, and in what order. > > Is this addressing some other case? > > > > > > In addition, I’ve taught the ASTReader to re-resolve the names of > > imported modules during module loading so that if the header search > > context changes between when a module was originally built and when it > > is loaded we can rebuild it if necessary. For example, if module A > > imports module B > > > > first time: > > clang -I /path/to/A -I /path/to/B … > > > > second time: > > clang -I /path/to/A -I /different/path/to/B … > > > > will now rebuild A as expected. > > > > > > * in the case of inferred modules, we use the module map file that > > *allowed* the inference, not the __inferred_module.map file, since the > > inferred file path is the same for every inferred module. > > > > Review comments on the patch itself: > > + /// the inferrence (e.g. contained 'module *') rather than the > virtual > > Typo 'inference', 'Module *'. > > + /// For an explanaition of \p ModuleMap, see Module::ModuleMap. > > Typo 'explanation'. > > + // safe becuase the FileManager is shared between the compiler > instances. > > Typo 'because' > > + // the inferred module. If this->ModuleMap is nullptr, then we are > using > + // -emit-module directly, and we cannot have an inferred module. > > I don't understand what this comment is trying to say. If we're using > -emit-module, then we were given a module map on the command-line; > should that not be referred to by this->ModuleMap? (Also, why 'this->'?) > How can a top-level module be inferred? Is that a framework-specific > thing? > > +StringRef ModuleMap = this->ModuleMap ? this->ModuleMap->getName() > : InFile; > > Please pick a different variable name rather than shadowing a member of > '*this' here. > > +// Construct the name -.pcm > which should > +// be globally unique to this particular module. > +llvm::APInt Code(64, llvm::hash_value(ModuleMapPath)); > +SmallString<128> HashStr; > +Code.toStringUnsigned(HashStr); > > Use base 36, like the module hash. > >>> > >>> > >>> I’ve attached an updated patch. Changes since the previous one: > >>> 1. Fixed the typos and other issues Richard pointed out > >>> 2. I’ve added code to canonicalize the module map path (using realpath); > >>> I was getting spurious failures on case-intensitive filesystems. > >> > >> > >> This part is probably not OK, because it'll do the wrong thing on some > >> build farms (where the canonical path is not predictable, but the path > >> that make_absolute returns is, by careful control of $PWD). I'll look into > >> this more, but will be traveling for the next couple of days. > > > > > > Okay, I have a better idea: I already store the actual module map path in > > MODULE_MAP_FILE, so when that is read, we can just get a FileEntry for it > > and compare to what header search finds for the current module (since > > FileEntries are uniqued by inode). That also means I can give a better > > diagnostic with the module map paths rather than
Re: [PATCH] Allow multiple modules with the same name to coexist in the module cache
On 4 Apr 2014 09:09, "Ben Langmuir" wrote: > > > On Apr 3, 2014, at 7:49 PM, Richard Smith wrote: > >> On Thu, Apr 3, 2014 at 3:40 PM, Ben Langmuir wrote: >>> >>> >>> On Mar 28, 2014, at 2:45 PM, Richard Smith wrote: >>> On Fri, Mar 28, 2014 at 1:12 PM, Ben Langmuir wrote: > > This patch allows multiple modules that have the same name to coexist in the module cache. To differentiate between two modules with the same name, we will consider the path the module map file that they are defined by* part of the 'key' for looking up the precompiled module (pcm file). Specifically, this patch renames the precompiled module (pcm) files from > > cache-path//Foo.pcm > > to > > cache-path//Foo-.pcm From a high level, I don't really see why we need a second hash here. Shouldn't the -I options be included in the ? If I build the same module with different -I flags, that should resolve to different .pcm files, regardless of whether it makes the module name resolve to a different module.map file. Are you trying to cope with the case where the -I path finds multiple module.map files defining the same module (where it's basically chance which one will get built and used)? I don't really feel like this is the right solution to that problem either -- we should remove the 'luck' aspect and use some sane mechanism to determine which module.map files are loaded, and in what order. Is this addressing some other case? > > In addition, I've taught the ASTReader to re-resolve the names of imported modules during module loading so that if the header search context changes between when a module was originally built and when it is loaded we can rebuild it if necessary. For example, if module A imports module B > > first time: > clang -I /path/to/A -I /path/to/B ... > > second time: > clang -I /path/to/A -I /different/path/to/B ... > > will now rebuild A as expected. > > > * in the case of inferred modules, we use the module map file that *allowed* the inference, not the __inferred_module.map file, since the inferred file path is the same for every inferred module. Review comments on the patch itself: + /// the inferrence (e.g. contained 'module *') rather than the virtual Typo 'inference', 'Module *'. + /// For an explanaition of \p ModuleMap, see Module::ModuleMap. Typo 'explanation'. + // safe becuase the FileManager is shared between the compiler instances. Typo 'because' + // the inferred module. If this->ModuleMap is nullptr, then we are using + // -emit-module directly, and we cannot have an inferred module. I don't understand what this comment is trying to say. If we're using -emit-module, then we were given a module map on the command-line; should that not be referred to by this->ModuleMap? (Also, why 'this->'?) How can a top-level module be inferred? Is that a framework-specific thing? +StringRef ModuleMap = this->ModuleMap ? this->ModuleMap->getName() : InFile; Please pick a different variable name rather than shadowing a member of '*this' here. +// Construct the name -.pcm which should +// be globally unique to this particular module. +llvm::APInt Code(64, llvm::hash_value(ModuleMapPath)); +SmallString<128> HashStr; +Code.toStringUnsigned(HashStr); Use base 36, like the module hash. >>> >>> >>> I've attached an updated patch. Changes since the previous one: >>> 1. Fixed the typos and other issues Richard pointed out >>> 2. I've added code to canonicalize the module map path (using realpath); I was getting spurious failures on case-intensitive filesystems. >> >> >> This part is probably not OK, because it'll do the wrong thing on some build farms (where the canonical path is not predictable, but the path that make_absolute returns is, by careful control of $PWD). I'll look into this more, but will be traveling for the next couple of days. > > > Okay, I have a better idea: I already store the actual module map path in MODULE_MAP_FILE, so when that is read, we can just get a FileEntry for it and compare to what header search finds for the current module (since FileEntries are uniqued by inode). That also means I can give a better diagnostic with the module map paths rather than the pcm filenames. Sounds great, thanks! >> >>> 3. I've moved the initialization of the MainFileID (in source manager) from Execute() to BeginSourceFile(), since we are now potentially creating file ids for module map files during pch loading and need to be able to find the main file reliably to construct a correct include stack. > > ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Do not use "1" in linemarker for main file in -frewrite-includes
I don't know what else this would interact with, so this is not an approval, but I think it's a good idea. We've gotten at least one bug report about this already, PR18948. Jordan On Apr 4, 2014, at 3:09 , Lubos Lunak wrote: > > The -frewrite-includes flag incorrectly uses at the beginning a linemarker > for the main file with the "1" flag which suggests that the contents have > been in fact included from another file. This for example > disables -Wunused-macros, which warns only for macros from the main file: > > $ cat a.cpp > #define FOO 1 > $ clang++ -E -frewrite-includes a.cpp | clang++ -Wall -Wunused-macros -x > c++ -c - > $ clang++ -Wall -Wunused-macros -c a.cpp > a.cpp:1:9: warning: macro is not used [-Wunused-macros] > #define FOO 1 > ^ > 1 warning generated. > > The attached patch fixes the code to start the resulting file with the > correct linemarker. > > -- > Lubos Lunak > <0001-do-not-use-1-for-line-marker-for-the-main-file.patch>___ > cfe-commits mailing list > cfe-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205632 - Add a test where the module map is overriden in the vfs
Author: benlangmuir Date: Fri Apr 4 11:42:53 2014 New Revision: 205632 URL: http://llvm.org/viewvc/llvm-project?rev=205632&view=rev Log: Add a test where the module map is overriden in the vfs Specifically, we pass two -ivfsoverlay yaml files, and the topmost one remaps the module map file. Added: cfe/trunk/test/VFS/Inputs/actual_module2.map cfe/trunk/test/VFS/Inputs/vfsoverlay2.yaml Modified: cfe/trunk/test/VFS/module-import.m Added: cfe/trunk/test/VFS/Inputs/actual_module2.map URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/actual_module2.map?rev=205632&view=auto == --- cfe/trunk/test/VFS/Inputs/actual_module2.map (added) +++ cfe/trunk/test/VFS/Inputs/actual_module2.map Fri Apr 4 11:42:53 2014 @@ -0,0 +1,5 @@ +module not_real { + header "not_real.h" + export * + explicit module from_second_vfs { } +} Added: cfe/trunk/test/VFS/Inputs/vfsoverlay2.yaml URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/Inputs/vfsoverlay2.yaml?rev=205632&view=auto == --- cfe/trunk/test/VFS/Inputs/vfsoverlay2.yaml (added) +++ cfe/trunk/test/VFS/Inputs/vfsoverlay2.yaml Fri Apr 4 11:42:53 2014 @@ -0,0 +1,12 @@ +{ + 'version': 0, + 'roots': [ +{ 'name': 'OUT_DIR', 'type': 'directory', + 'contents': [ +{ 'name': 'module.map', 'type': 'file', + 'external-contents': 'INPUT_DIR/actual_module2.map' +} + ] +} + ] +} Modified: cfe/trunk/test/VFS/module-import.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/VFS/module-import.m?rev=205632&r1=205631&r2=205632&view=diff == --- cfe/trunk/test/VFS/module-import.m (original) +++ cfe/trunk/test/VFS/module-import.m Fri Apr 4 11:42:53 2014 @@ -8,3 +8,20 @@ void foo() { bar(); } + +// Import a submodule that is defined in actual_module2.map, which is only +// mapped in vfsoverlay2.yaml. +#ifdef IMPORT2 +@import not_real.from_second_module; +// CHECK-VFS2: error: no submodule +#endif + +// Override the module map (vfsoverlay2 on top) +// RUN: sed -e "s:INPUT_DIR:%S/Inputs:g" -e "s:OUT_DIR:%t:g" %S/Inputs/vfsoverlay2.yaml > %t2.yaml +// RUN: %clang_cc1 -Werror -fmodules -fmodules-cache-path=%t -ivfsoverlay %t.yaml -ivfsoverlay %t2.yaml -I %t -fsyntax-only %s + +// vfsoverlay2 not present +// RUN: not %clang_cc1 -Werror -fmodules -fmodules-cache-path=%t -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s + +// vfsoverlay2 on the bottom +// RUN: not %clang_cc1 -Werror -fmodules -fmodules-cache-path=%t -ivfsoverlay %t2.yaml -ivfsoverlay %t.yaml -I %t -fsyntax-only %s -DIMPORT2 2>&1 | FileCheck -check-prefix=CHECK-VFS2 %s ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow multiple modules with the same name to coexist in the module cache
On Apr 4, 2014, at 9:09 AM, Ben Langmuir wrote: > > On Apr 3, 2014, at 7:49 PM, Richard Smith wrote: > >> On Thu, Apr 3, 2014 at 3:40 PM, Ben Langmuir wrote: >> >> On Mar 28, 2014, at 2:45 PM, Richard Smith wrote: >> >>> On Fri, Mar 28, 2014 at 1:12 PM, Ben Langmuir wrote: >>> This patch allows multiple modules that have the same name to coexist in >>> the module cache. To differentiate between two modules with the same name, >>> we will consider the path the module map file that they are defined by* >>> part of the ‘key’ for looking up the precompiled module (pcm file). >>> Specifically, this patch renames the precompiled module (pcm) files from >>> >>> cache-path//Foo.pcm >>> >>> to >>> >>> cache-path//Foo-.pcm >>> >>> From a high level, I don't really see why we need a second hash here. >>> Shouldn't the -I options be included in the ? If I build the >>> same module with different -I flags, that should resolve to different .pcm >>> files, regardless of whether it makes the module name resolve to a >>> different module.map file. >>> >>> Are you trying to cope with the case where the -I path finds multiple >>> module.map files defining the same module (where it's basically chance >>> which one will get built and used)? I don't really feel like this is the >>> right solution to that problem either -- we should remove the 'luck' aspect >>> and use some sane mechanism to determine which module.map files are loaded, >>> and in what order. >>> >>> Is this addressing some other case? >>> >>> >>> In addition, I’ve taught the ASTReader to re-resolve the names of imported >>> modules during module loading so that if the header search context changes >>> between when a module was originally built and when it is loaded we can >>> rebuild it if necessary. For example, if module A imports module B >>> >>> first time: >>> clang -I /path/to/A -I /path/to/B … >>> >>> second time: >>> clang -I /path/to/A -I /different/path/to/B … >>> >>> will now rebuild A as expected. >>> >>> >>> * in the case of inferred modules, we use the module map file that >>> *allowed* the inference, not the __inferred_module.map file, since the >>> inferred file path is the same for every inferred module. >>> >>> >>> Review comments on the patch itself: >>> >>> + /// the inferrence (e.g. contained 'module *') rather than the virtual >>> >>> Typo 'inference', 'Module *'. >>> >>> + /// For an explanaition of \p ModuleMap, see Module::ModuleMap. >>> >>> Typo 'explanation'. >>> >>> + // safe becuase the FileManager is shared between the compiler instances. >>> >>> Typo 'because' >>> >>> + // the inferred module. If this->ModuleMap is nullptr, then we are using >>> + // -emit-module directly, and we cannot have an inferred module. >>> >>> I don't understand what this comment is trying to say. If we're using >>> -emit-module, then we were given a module map on the command-line; should >>> that not be referred to by this->ModuleMap? (Also, why 'this->'?) How can a >>> top-level module be inferred? Is that a framework-specific thing? >>> >>> +StringRef ModuleMap = this->ModuleMap ? this->ModuleMap->getName() : >>> InFile; >>> >>> Please pick a different variable name rather than shadowing a member of >>> '*this' here. >>> >>> +// Construct the name -.pcm which >>> should >>> +// be globally unique to this particular module. >>> +llvm::APInt Code(64, llvm::hash_value(ModuleMapPath)); >>> +SmallString<128> HashStr; >>> +Code.toStringUnsigned(HashStr); >>> >>> Use base 36, like the module hash. >> >> >> I’ve attached an updated patch. Changes since the previous one: >> 1. Fixed the typos and other issues Richard pointed out >> 2. I’ve added code to canonicalize the module map path (using realpath); I >> was getting spurious failures on case-intensitive filesystems. >> >> This part is probably not OK, because it'll do the wrong thing on some build >> farms (where the canonical path is not predictable, but the path that >> make_absolute returns is, by careful control of $PWD). I'll look into this >> more, but will be traveling for the next couple of days. > > Okay, I have a better idea: I already store the actual module map path in > MODULE_MAP_FILE, so when that is read, we can just get a FileEntry for it and > compare to what header search finds for the current module (since FileEntries > are uniqued by inode). That also means I can give a better diagnostic with > the module map paths rather than the pcm filenames. In case it wasn’t clear that means we don’t need the canonical path. > >> >> 3. I’ve moved the initialization of the MainFileID (in source manager) from >> Execute() to BeginSourceFile(), since we are now potentially creating file >> ids for module map files during pch loading and need to be able to find the >> main file reliably to construct a correct include stack. > > ___ > cfe-commits
Re: [PATCH] PR19095 Undefined reference to friend template function defined inside template class
Gentle Ping !! Please help in reviewing this small patch. Its a humble request :) On Tue, Apr 1, 2014 at 10:25 PM, suyog sarda wrote: > Hi, > > Attaching patch for bug 19095. Please help in reviewing the same. > > Also, I haven't attached a test case yet in the patch as i am not sure how > it should be and in which file it should be. > > In my opinion, the test case would go into > *tools/clang/test/SemaCXX/friend.cpp > *would be something like below (similar to that mentioned in the bug) > > > > *template void f(T); * > > *void h(T);* > > > > > > > > *template class C{ template friend void f(T) {* > > * int x = 1;* > > * }* > > * template * > > * friend void h(T); * > > > > > > > * public : void g() { f(3.0); // OK* > > * h(2.0); // error : undefined reference to function h* > > > > > > > > * }int i;};void h () { f(7); // OK* > > * h(6); // error : undefined reference to function h* > > > * C c; c.g();}* > > Please help in reviewing the patch as well as the test case. > > > -- > With regards, > Suyog Sarda > -- With regards, Suyog Sarda ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[PATCH] PR 19339 - Parser confusion between lambda and designated initializer
Hi rsmith, krememek, http://llvm.org/bugs/show_bug.cgi?id=19339 Here is a reduced test case: struct Foo { template Foo(T) {} }; Foo f3 { [i{0}]{} }; The Parser gets confused in bool Parser::MayBeDesignationStart() in trying to distinguish between a lambda or a designated initializer when parsing [i{0}] FYI: Sema does not yet implement (recently voted into C++14? or is it C++17 I forget) the proper deduction for one element in a brace initializer yet. This is a very cursory rough stab (includes a use of goto) at addressing this - just to draw attention to where i think the problem lies. I will leave it to those who grok the Parser better than me to either help me refine it - or conjure up their own fix. http://llvm-reviews.chandlerc.com/D3290 Files: lib/Parse/ParseInit.cpp Index: lib/Parse/ParseInit.cpp === --- lib/Parse/ParseInit.cpp +++ lib/Parse/ParseInit.cpp @@ -75,11 +75,24 @@ case tok::amp: case tok::identifier: case tok::kw_this: +case tok::numeric_constant: // These tokens can occur in a capture list or a constant-expression. // Keep looking. ConsumeToken(); continue; - + +case tok::l_brace: +case tok::r_brace: + if (PP.getLangOpts().CPlusPlus11) { +// These tokens can occur in a capture list or a constant-expression. +// Keep looking. +// Vec{[i{0}]() { }}; +// int a[10] = { [Literal{}] = 3, 4 }; +ConsumeBrace(); +continue; + } else { +goto default_label; + } case tok::comma: // Since a comma cannot occur in a constant-expression, this must // be a lambda. @@ -99,11 +112,13 @@ } default: +default_label: // Anything else cannot occur in a lambda capture list, so it // must be a designator. Tentative.Revert(); return true; } + } } Index: lib/Parse/ParseInit.cpp === --- lib/Parse/ParseInit.cpp +++ lib/Parse/ParseInit.cpp @@ -75,11 +75,24 @@ case tok::amp: case tok::identifier: case tok::kw_this: +case tok::numeric_constant: // These tokens can occur in a capture list or a constant-expression. // Keep looking. ConsumeToken(); continue; - + +case tok::l_brace: +case tok::r_brace: + if (PP.getLangOpts().CPlusPlus11) { +// These tokens can occur in a capture list or a constant-expression. +// Keep looking. +// Vec{[i{0}]() { }}; +// int a[10] = { [Literal{}] = 3, 4 }; +ConsumeBrace(); +continue; + } else { +goto default_label; + } case tok::comma: // Since a comma cannot occur in a constant-expression, this must // be a lambda. @@ -99,11 +112,13 @@ } default: +default_label: // Anything else cannot occur in a lambda capture list, so it // must be a designator. Tentative.Revert(); return true; } + } } ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: clang-format: better detection for multiplication operator
inline On Fri, Apr 04, 2014 at 08:06:00AM +0200, Daniel Jasper wrote: > I think this makes more sense for now. If we stumble upon bugs requiring > the fix for the other operators, we still have your other patch. > > > On Thu, Apr 3, 2014 at 10:58 PM, Johannes Kapfhammer wrote: > > > On Sun, Mar 30, 2014 at 07:45:51PM +0200, Daniel Jasper wrote: > > > All in all, I think a much easier solution for the mentioned bug would be > > > to treat "<<" similar to how we treat assignments, i.e. if we find a <<, > > we > > > assume the RHS to be an expression (that's the very first "if" in > > > determineTokenType()). > > > > > Not sure if that's the right place. The "if" branch you are talking > > about will set all preceding stars (on the same level) to > > TT_PointerOrReference: > > > > Before: cout << a *a << b *b << c *c; > > After: cout << a *a << b *b << c * b; > > My Fix: cout << a * a << b * b << c * c; > > > > So we need the special care for operator<< at least in a new > > "else if". I've added a "light" version of this patch below, note > > that it only handles operator<< and doesn't work with other operators. > > Doing it the way I did in the last mail has the additional benefit > > that it works for all operators and not just "<<" (in particular > > operator+ is hard to get otherwise). But we probably won't need this. > > > > -- > > > > From 16d81e7e3a0dedd5d51f963a134b26eb9c63b777 Mon Sep 17 00:00:00 2001 > > From: Kapfhammer > > Date: Sun, 30 Mar 2014 12:04:17 +0200 > > Subject: [PATCH] Use binary operators as an indicator of for an expression > > and > > extend the test suite. > > > > This solves the issue where the star was too often interpreted as a > > pointer, e.g "cout< > "cout << a * b;". By marking statements more often an expression, the > > function determineStarAmpUsage() is more reliable. > > > > The test suite was extended. > > --- > > lib/Format/TokenAnnotator.cpp | 5 + > > unittests/Format/FormatTest.cpp | 15 +++ > > 2 files changed, 20 insertions(+) > > > > diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp > > index 0034235..7fbbdae 100644 > > --- a/lib/Format/TokenAnnotator.cpp > > +++ b/lib/Format/TokenAnnotator.cpp > > @@ -681,6 +681,11 @@ private: > >Previous->Type = TT_PointerOrReference; > > } > >} > > +} else if (Current.is(tok::lessless) && > > + !Line.First->isOneOf(tok::kw_template, tok::kw_using) && > > > > How does this change the behavior in any way? It was relevant with the old patch. Now it's only different in a few special cases: With it: template Without: template So I think we don't need it. > > > > + (!Current.Previous || > > +Current.Previous->isNot(tok::kw_operator))) { > > > > I wonder whether we can just use: > > } else if (Current.is(tok::lessless) && !Line.MustBeDeclaration) { .. > I can't tell for sure if it does the same, but it seems so. > > > > + Contexts.back().IsExpression = true; > > } else if (Current.isOneOf(tok::kw_return, tok::kw_throw)) { > >Contexts.back().IsExpression = true; > > } else if (Current.is(tok::l_paren) && !Line.MustBeDeclaration && > > diff --git a/unittests/Format/FormatTest.cpp > > b/unittests/Format/FormatTest.cpp > > index 5395fd9..7478a23 100644 > > --- a/unittests/Format/FormatTest.cpp > > +++ b/unittests/Format/FormatTest.cpp > > @@ -4625,6 +4625,21 @@ TEST_F(FormatTest, UnderstandsRvalueReferences) { > >verifyGoogleFormat("#define WHILE(a, b, c) while (a && (b == c))"); > > } > > > > +TEST_F(FormatTest, FormatsMultiplicationOperator) { > > + verifyFormat("operator*(type *a)"); > > + verifyFormat("operator<<(type *a)"); > > + verifyFormat("{ cout << (a * b); }"); > > + verifyFormat("{ cout << a * b; }"); > > + verifyFormat("{ cout << a * b << c * d; }"); > > + verifyFormat("type (*f)(type *a)"); > > + verifyFormat("type (&f)(type *a)"); > > + verifyFormat("void f(type *a)"); > > + verifyFormat("using f = void (*)(a *b)"); > > + verifyFormat("template > + verifyFormat("using f = void (c::*)(a *b)"); > > + verifyFormat("template "); > > > > There are many tests here that don't contain a "<<". What are they testing? > They are not needed anymore. I thought they don't hurt, but you can remove them if you want to. > +} > > > + > > TEST_F(FormatTest, FormatsBinaryOperatorsPrecedingEquals) { > >verifyFormat("void f() {\n" > > " x[a -\n" > > -- > > 1.9.1 > > > > ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow multiple modules with the same name to coexist in the module cache
On Apr 3, 2014, at 7:49 PM, Richard Smith wrote: > On Thu, Apr 3, 2014 at 3:40 PM, Ben Langmuir wrote: > > On Mar 28, 2014, at 2:45 PM, Richard Smith wrote: > >> On Fri, Mar 28, 2014 at 1:12 PM, Ben Langmuir wrote: >> This patch allows multiple modules that have the same name to coexist in the >> module cache. To differentiate between two modules with the same name, we >> will consider the path the module map file that they are defined by* part of >> the ‘key’ for looking up the precompiled module (pcm file). Specifically, >> this patch renames the precompiled module (pcm) files from >> >> cache-path//Foo.pcm >> >> to >> >> cache-path//Foo-.pcm >> >> From a high level, I don't really see why we need a second hash here. >> Shouldn't the -I options be included in the ? If I build the >> same module with different -I flags, that should resolve to different .pcm >> files, regardless of whether it makes the module name resolve to a different >> module.map file. >> >> Are you trying to cope with the case where the -I path finds multiple >> module.map files defining the same module (where it's basically chance which >> one will get built and used)? I don't really feel like this is the right >> solution to that problem either -- we should remove the 'luck' aspect and >> use some sane mechanism to determine which module.map files are loaded, and >> in what order. >> >> Is this addressing some other case? >> >> >> In addition, I’ve taught the ASTReader to re-resolve the names of imported >> modules during module loading so that if the header search context changes >> between when a module was originally built and when it is loaded we can >> rebuild it if necessary. For example, if module A imports module B >> >> first time: >> clang -I /path/to/A -I /path/to/B … >> >> second time: >> clang -I /path/to/A -I /different/path/to/B … >> >> will now rebuild A as expected. >> >> >> * in the case of inferred modules, we use the module map file that *allowed* >> the inference, not the __inferred_module.map file, since the inferred file >> path is the same for every inferred module. >> >> >> Review comments on the patch itself: >> >> + /// the inferrence (e.g. contained 'module *') rather than the virtual >> >> Typo 'inference', 'Module *'. >> >> + /// For an explanaition of \p ModuleMap, see Module::ModuleMap. >> >> Typo 'explanation'. >> >> + // safe becuase the FileManager is shared between the compiler instances. >> >> Typo 'because' >> >> + // the inferred module. If this->ModuleMap is nullptr, then we are using >> + // -emit-module directly, and we cannot have an inferred module. >> >> I don't understand what this comment is trying to say. If we're using >> -emit-module, then we were given a module map on the command-line; should >> that not be referred to by this->ModuleMap? (Also, why 'this->'?) How can a >> top-level module be inferred? Is that a framework-specific thing? >> >> +StringRef ModuleMap = this->ModuleMap ? this->ModuleMap->getName() : >> InFile; >> >> Please pick a different variable name rather than shadowing a member of >> '*this' here. >> >> +// Construct the name -.pcm which >> should >> +// be globally unique to this particular module. >> +llvm::APInt Code(64, llvm::hash_value(ModuleMapPath)); >> +SmallString<128> HashStr; >> +Code.toStringUnsigned(HashStr); >> >> Use base 36, like the module hash. > > > I’ve attached an updated patch. Changes since the previous one: > 1. Fixed the typos and other issues Richard pointed out > 2. I’ve added code to canonicalize the module map path (using realpath); I > was getting spurious failures on case-intensitive filesystems. > > This part is probably not OK, because it'll do the wrong thing on some build > farms (where the canonical path is not predictable, but the path that > make_absolute returns is, by careful control of $PWD). I'll look into this > more, but will be traveling for the next couple of days. Okay, I have a better idea: I already store the actual module map path in MODULE_MAP_FILE, so when that is read, we can just get a FileEntry for it and compare to what header search finds for the current module (since FileEntries are uniqued by inode). That also means I can give a better diagnostic with the module map paths rather than the pcm filenames. > > 3. I’ve moved the initialization of the MainFileID (in source manager) from > Execute() to BeginSourceFile(), since we are now potentially creating file > ids for module map files during pch loading and need to be able to find the > main file reliably to construct a correct include stack. ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Handle properly somewhat special cases of -main-file-name
On Friday 04 of April 2014, David Blaikie wrote: > The comment in CGDebugInfo.cpp says that -main-file-name will contain > only the file name, with no path information. Are there cases where > that is not true when -main-file-name is passed from the Clang driver, > rather than by the user in your example below? > > If the driver provides this guarantee and the user violates it when > passing an undocumented flag manually, I don't see a need to support > it - but if there is such a need, it'd be good to > understand/documemnt/discuss it. I think the driver always only passes the filename, but I do pass it explicitly, since I feed the source from stdin and the source or even its directory don't exist (distributed compiling, the same way with the patch for -dwarf-split-file). And actually case 2) without using the option explicitly results in DW_AT_name becoming "/-" (I don't know why the driver doesn't simply pass the name as it is, which wouldn't need the code that attempts to rebuild it later). As for "undocumented", the documentation status of the option is about the same like with many others - it's just in the .td file including its description. The description says quite clearly what the option does, it doesn't say it's internal, it works fine (except for these small problems), I even checked the sources to be sure, and I don't see why Clang should forbid usage of the option if I know what I'm doing (and would have to resort to ugly hacks otherwise). > On Fri, Apr 4, 2014 at 5:02 AM, Lubos Lunak wrote: > > The handling of -main-file-name in CGDebugInfo::CreateCompileUnit() can > > result in incorrect DW_AT_name in somewhat special cases: > > > > 1) > > $ touch /tmp/a.cpp > > $ > > clang++ -Wall -c /tmp/a.cpp -g -o /tmp/a.o -Xclang -main-file-name > > -Xclang /new/path/a.cpp $ readelf -wi /tmp/a.o | grep DW_AT_name > > <12> DW_AT_name: (indirect string, offset: > > 0x15): /tmp/new/path/a.cpp > > > > 2) > > $ touch /tmp/a.cpp > > $ cd / > > $ cat /tmp/a.cpp | clang++ -Wall -x > > c++ -c - -g -o /tmp/a.o -Xclang -main-file-name -Xclang a.cpp > > $ readelf -wi /tmp/a.o | grep DW_AT_name > > <12> DW_AT_name: (indirect string, offset: 0x15): /a.cpp > > > > The attached patch fixes those. Ok to commit? -- Lubos Lunak ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
On Friday 04 of April 2014, David Blaikie wrote: > Oops, re-add cfe-commits. > > On Fri, Apr 4, 2014 at 8:34 AM, David Blaikie wrote: > > On Fri, Apr 4, 2014 at 8:24 AM, Lubos Lunak wrote: > >> On Friday 04 of April 2014, Eric Christopher wrote: > >>> ... Why would you want to do this? > >> > >> The compile itself happens in a chroot and the expected and actual > >> output locations differ (and don't even exist in the other tree). > > > > In your scenario the .dwo is not side-by-side with the .o? Or are you > > overriding the .o name as well in some way? The scenario is distributed compiling with Icecream (https://github.com/icecc/icecream). If you don't know it, think distcc, except more sophisticated in some ways, one of them being that it ships the compiler to the remote host and uses it in chroot (which avoids a number of problems and allows cross-compiling). Which means the actual compile run lacks a number of things, like the source file itself (piped using stdin), the source file location, or the expected output location. The result goes to a temporary .o file, which is generally not a problem (I don't think the name of the .o file matters), but with split dwarf the .dwo name gets hardcoded to this random location in the .o file. For performance reasons there's a chroot location per compiler, not per compile, so while I could temporarily create the right location, I'm not exactly eager to write the code to do that properly with all the locks etc. when I could use a compiler option that just sets one dwarf info field, if only I actually could use it. If you want a precedent, there's already -fdebug-compilation-dir, which AFAICT is exactly the same, just with a different dwarf info field. > >> I could do with > >> changing DW_AT_GNU_dwo_name explicitly after the build, but that seems > >> needlessly complex given that this seems to be exactly what the option > >> does. I don't see why I would be allowed to override any option except > >> for this one. > > > > -Xclang and the underlying driver arguments aren't really a > > stable/guaranteed interface. I'd be more inclined to accept this > > change if it were just for some debugging, but since it sounds like > > you want to rely on it, it's good for us to understand the goal and > > perhaps suggest or provide the best way of achieving it long-term. It's stable/guaranteed enough for me, and I'd rather have a clean solution that maybe breaks one day than something hackish the whole time. -- Lubos Lunak ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
Oops, re-add cfe-commits. On Fri, Apr 4, 2014 at 8:34 AM, David Blaikie wrote: > On Fri, Apr 4, 2014 at 8:24 AM, Lubos Lunak wrote: >> On Friday 04 of April 2014, Eric Christopher wrote: >>> ... Why would you want to do this? >> >> The compile itself happens in a chroot and the expected and actual output >> locations differ (and don't even exist in the other tree). > > In your scenario the .dwo is not side-by-side with the .o? Or are you > overriding the .o name as well in some way? > >> I could do with >> changing DW_AT_GNU_dwo_name explicitly after the build, but that seems >> needlessly complex given that this seems to be exactly what the option does. >> I don't see why I would be allowed to override any option except for this >> one. > > -Xclang and the underlying driver arguments aren't really a > stable/guaranteed interface. I'd be more inclined to accept this > change if it were just for some debugging, but since it sounds like > you want to rely on it, it's good for us to understand the goal and > perhaps suggest or provide the best way of achieving it long-term. > >> >>> -eric >>> >>> On Apr 4, 2014 12:26 AM, "Lubos Lunak" wrote: >>> > The option -split-dwarf-file is passed by the driver to the compiler >>> > after processing -Xclang options, thus overriding any possible explicitly >>> > specified >>> > option: >>> > >>> > $ clang++ -c -gsplit-dwarf a.cpp -o a.o -Xclang -split-dwarf-file -Xclang >>> > b.dwo >>> > $ readelf -wi a.o | grep dwo_name >>> >DW_AT_GNU_dwo_name: (indirect string, offset: 0x0): a.dwo >>> > >>> > This is because the driver invokes the compiler as >>> > /usr/bin/clang-3.4 -cc1 ... -split-dwarf-file b.dwo -o a.o -x c++ >>> > a.cpp -split-dwarf-file a.dwo >>> > >>> > The attached patch fixes this. Ok to push? >> >> -- >> Lubos Lunak >> ___ >> cfe-commits mailing list >> cfe-commits@cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
On Friday 04 of April 2014, Eric Christopher wrote: > ... Why would you want to do this? The compile itself happens in a chroot and the expected and actual output locations differ (and don't even exist in the other tree). I could do with changing DW_AT_GNU_dwo_name explicitly after the build, but that seems needlessly complex given that this seems to be exactly what the option does. I don't see why I would be allowed to override any option except for this one. > -eric > > On Apr 4, 2014 12:26 AM, "Lubos Lunak" wrote: > > The option -split-dwarf-file is passed by the driver to the compiler > > after processing -Xclang options, thus overriding any possible explicitly > > specified > > option: > > > > $ clang++ -c -gsplit-dwarf a.cpp -o a.o -Xclang -split-dwarf-file -Xclang > > b.dwo > > $ readelf -wi a.o | grep dwo_name > >DW_AT_GNU_dwo_name: (indirect string, offset: 0x0): a.dwo > > > > This is because the driver invokes the compiler as > > /usr/bin/clang-3.4 -cc1 ... -split-dwarf-file b.dwo -o a.o -x c++ > > a.cpp -split-dwarf-file a.dwo > > > > The attached patch fixes this. Ok to push? -- Lubos Lunak ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205629 - In preparation for being able to use simple Boolean logic expressions involving capabilities, the semantics for attributes now looks through the types of the constituent parts of a capabilit
Author: aaronballman Date: Fri Apr 4 10:13:57 2014 New Revision: 205629 URL: http://llvm.org/viewvc/llvm-project?rev=205629&view=rev Log: In preparation for being able to use simple Boolean logic expressions involving capabilities, the semantics for attributes now looks through the types of the constituent parts of a capability expression instead of at the aggregate expression type. Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp cfe/trunk/test/Sema/attr-capabilities.c Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=205629&r1=205628&r2=205629&view=diff == --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Fri Apr 4 10:13:57 2014 @@ -363,8 +363,7 @@ static const RecordType *getRecordType(Q return 0; } -static bool checkRecordTypeForCapability(Sema &S, const AttributeList &Attr, - QualType Ty) { +static bool checkRecordTypeForCapability(Sema &S, QualType Ty) { const RecordType *RT = getRecordType(Ty); if (!RT) @@ -397,8 +396,7 @@ static bool checkRecordTypeForCapability return false; } -static bool checkTypedefTypeForCapability(Sema &S, const AttributeList &Attr, - QualType Ty) { +static bool checkTypedefTypeForCapability(QualType Ty) { const auto *TD = Ty->getAs(); if (!TD) return false; @@ -410,18 +408,40 @@ static bool checkTypedefTypeForCapabilit return TN->hasAttr(); } -/// \brief Checks that the passed in type is qualified as a capability. This -/// type can either be a struct, or a typedef to a built-in type (such as int). -static void checkForCapability(Sema &S, const AttributeList &Attr, - QualType Ty) { - if (checkTypedefTypeForCapability(S, Attr, Ty)) -return; +static bool typeHasCapability(Sema &S, QualType Ty) { + if (checkTypedefTypeForCapability(Ty)) +return true; + + if (checkRecordTypeForCapability(S, Ty)) +return true; + + return false; +} - if (checkRecordTypeForCapability(S, Attr, Ty)) -return; +static bool isCapabilityExpr(Sema &S, const Expr *Ex) { + // Capability expressions are simple expressions involving the boolean logic + // operators &&, || or !, a simple DeclRefExpr, CastExpr or a ParenExpr. Once + // a DeclRefExpr is found, its type should be checked to determine whether it + // is a capability or not. + + if (const auto *E = dyn_cast(Ex)) +return typeHasCapability(S, E->getType()); + else if (const auto *E = dyn_cast(Ex)) +return isCapabilityExpr(S, E->getSubExpr()); + else if (const auto *E = dyn_cast(Ex)) +return isCapabilityExpr(S, E->getSubExpr()); + else if (const auto *E = dyn_cast(Ex)) { +if (E->getOpcode() == UO_LNot) + return isCapabilityExpr(S, E->getSubExpr()); +return false; + } else if (const auto *E = dyn_cast(Ex)) { +if (E->getOpcode() == BO_LAnd || E->getOpcode() == BO_LOr) + return isCapabilityExpr(S, E->getLHS()) && + isCapabilityExpr(S, E->getRHS()); +return false; + } - S.Diag(Attr.getLoc(), diag::warn_thread_attribute_argument_not_lockable) -<< Attr.getName() << Ty; + return false; } /// \brief Checks that all attribute arguments, starting from Sidx, resolve to @@ -491,7 +511,13 @@ static void checkAttrArgsAreCapabilityOb } } -checkForCapability(S, Attr, ArgTy); +// If the type does not have a capability, see if the components of the +// expression have capabilities. This allows for writing C code where the +// capability may be on the type, and the expression is a capability +// boolean logic expression. Eg) requires_capability(A || B && !C) +if (!typeHasCapability(S, ArgTy) && !isCapabilityExpr(S, ArgExp)) + S.Diag(Attr.getLoc(), diag::warn_thread_attribute_argument_not_lockable) + << Attr.getName() << ArgTy; Args.push_back(ArgExp); } Modified: cfe/trunk/test/Sema/attr-capabilities.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/attr-capabilities.c?rev=205629&r1=205628&r2=205629&view=diff == --- cfe/trunk/test/Sema/attr-capabilities.c (original) +++ cfe/trunk/test/Sema/attr-capabilities.c Fri Apr 4 10:13:57 2014 @@ -50,4 +50,13 @@ void Func23(void) __attribute__((try_acq void Func24(void) __attribute__((try_acquire_shared_capability(1, GUI))) {} void Func25(void) __attribute__((try_acquire_capability())) {} // expected-error {{'try_acquire_capability' attribute takes at least 1 argument}} -void Func26(void) __attribute__((try_acquire_shared_capability())) {} // expected-error {{'try_acquire_shared_capability' attribute takes at least 1 argument}} \ No newline at end of file +void Func26(void) __attribute__((try_acquire_shared_capability())) {} // ex
Re: [Patch] Missed evaluation in "type" parameter of va_arg
On Wed, Apr 2, 2014 at 6:55 AM, Rahul Jain <1989.rahulj...@gmail.com> wrote: > Gentle ping! > > Please if someone could help review this small patch! Hello Rahul, This patch needs a testcase. Dmitri -- main(i,j){for(i=2;;i++){for(j=2;j*/ ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Handle properly somewhat special cases of -main-file-name
The comment in CGDebugInfo.cpp says that -main-file-name will contain only the file name, with no path information. Are there cases where that is not true when -main-file-name is passed from the Clang driver, rather than by the user in your example below? If the driver provides this guarantee and the user violates it when passing an undocumented flag manually, I don't see a need to support it - but if there is such a need, it'd be good to understand/documemnt/discuss it. On Fri, Apr 4, 2014 at 5:02 AM, Lubos Lunak wrote: > > The handling of -main-file-name in CGDebugInfo::CreateCompileUnit() can > result in incorrect DW_AT_name in somewhat special cases: > > 1) > $ touch /tmp/a.cpp > $ > clang++ -Wall -c /tmp/a.cpp -g -o /tmp/a.o -Xclang -main-file-name -Xclang > /new/path/a.cpp > $ readelf -wi /tmp/a.o | grep DW_AT_name > <12> DW_AT_name: (indirect string, offset: > 0x15): /tmp/new/path/a.cpp > > 2) > $ touch /tmp/a.cpp > $ cd / > $ cat /tmp/a.cpp | clang++ -Wall -x > c++ -c - -g -o /tmp/a.o -Xclang -main-file-name -Xclang a.cpp > $ readelf -wi /tmp/a.o | grep DW_AT_name > <12> DW_AT_name: (indirect string, offset: 0x15): /a.cpp > > The attached patch fixes those. Ok to commit? > > -- > Lubos Lunak > > ___ > cfe-commits mailing list > cfe-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Allow overriding -split-dwarf-file
... Why would you want to do this? -eric On Apr 4, 2014 12:26 AM, "Lubos Lunak" wrote: > > The option -split-dwarf-file is passed by the driver to the compiler after > processing -Xclang options, thus overriding any possible explicitly > specified > option: > > $ clang++ -c -gsplit-dwarf a.cpp -o a.o -Xclang -split-dwarf-file -Xclang > b.dwo > $ readelf -wi a.o | grep dwo_name >DW_AT_GNU_dwo_name: (indirect string, offset: 0x0): a.dwo > > This is because the driver invokes the compiler as > /usr/bin/clang-3.4 -cc1 ... -split-dwarf-file b.dwo -o a.o -x c++ > a.cpp -split-dwarf-file a.dwo > > The attached patch fixes this. Ok to push? > > -- > Lubos Lunak > > ___ > cfe-commits mailing list > cfe-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
RE: [PATCH] OpenCL - use AST PrintingPolicy when emitting kernel arg metadata
Sure, I committed as r205624! Thanks! From: Fraser Cormack [mailto:fra...@codeplay.com] Sent: 04 April 2014 13:10 To: Joey Gouly; llvm cfe Subject: Re: [PATCH] OpenCL - use AST PrintingPolicy when emitting kernel arg metadata Oh, sorry, I don't have commit privileges. Could you do the honours? Thanks, Fraser On 03/04/14 18:40, Fraser Cormack wrote: Ah yes, sorry about that! An updated patch is attached. On 03/04/14 15:50, Joey Gouly wrote: If you add a test, it LGTM! -Original Message- From: cfe-commits-boun...@cs.uiuc.edu [mailto:cfe-commits-boun...@cs.uiuc.edu] On Behalf Of Fraser Cormack Sent: 03 April 2014 12:04 To: llvm cfe Subject: [PATCH] OpenCL - use AST PrintingPolicy when emitting kernel arg metadata Hi, This patch uses the AST PrintingPolicy when emitting OpenCL kernel arg metadata in order to pretty print the 'half' type, if supported. Cheers, Fraser ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits -- Fraser Cormack Compiler Developer Codeplay Software Ltd 45 York Place, Edinburgh, EH1 3HP Tel: 0131 466 0503 Fax: 0131 557 6600 Website: http://www.codeplay.com Twitter: https://twitter.com/codeplaysoft This email and any attachments may contain confidential and /or privileged information and is for use by the addressee only. If you are not the intended recipient, please notify Codeplay Software Ltd immediately and delete the message from your computer. You may not copy or forward it,or use or disclose its contents to any other person. Any views or other information in this message which do not relate to our business are not authorized by Codeplay software Ltd, nor does this message form part of any contract unless so stated. As internet communications are capable of data corruption Codeplay Software Ltd does not accept any responsibility for any changes made to this message after it was sent. Please note that Codeplay Software Ltd does not accept any liability or responsibility for viruses and it is your responsibility to scan any attachments. Company registered in England and Wales, number: 04567874 Registered office: 81 Linkfield Street, Redhill RH1 6BY ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
r205624 - When printing types for the OpenCL kernel metadata, use the PrintingPolicy.
Author: joey Date: Fri Apr 4 08:43:57 2014 New Revision: 205624 URL: http://llvm.org/viewvc/llvm-project?rev=205624&view=rev Log: When printing types for the OpenCL kernel metadata, use the PrintingPolicy. This allows 'half' to be printed as 'half' and not as '__fp16'. Patch by Fraser Cormack! Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=205624&r1=205623&r2=205624&view=diff == --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original) +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Fri Apr 4 08:43:57 2014 @@ -339,6 +339,8 @@ static void GenOpenCLArgMetadata(const F // Each MDNode is a list in the form of "key", N number of values which is // the same number of values as their are kernel arguments. + const PrintingPolicy &Policy = ASTCtx.getPrintingPolicy(); + // MDNode for the kernel argument address space qualifiers. SmallVector addressQuals; addressQuals.push_back(llvm::MDString::get(Context, "kernel_arg_addr_space")); @@ -372,7 +374,8 @@ static void GenOpenCLArgMetadata(const F pointeeTy.getAddressSpace(; // Get argument type name. - std::string typeName = pointeeTy.getUnqualifiedType().getAsString() + "*"; + std::string typeName = + pointeeTy.getUnqualifiedType().getAsString(Policy) + "*"; // Turn "unsigned type" to "utype" std::string::size_type pos = typeName.find("unsigned"); @@ -398,7 +401,7 @@ static void GenOpenCLArgMetadata(const F addressQuals.push_back(Builder.getInt32(AddrSpc)); // Get argument type name. - std::string typeName = ty.getUnqualifiedType().getAsString(); + std::string typeName = ty.getUnqualifiedType().getAsString(Policy); // Turn "unsigned type" to "utype" std::string::size_type pos = typeName.find("unsigned"); Modified: cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl?rev=205624&r1=205623&r2=205624&view=diff == --- cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl (original) +++ cfe/trunk/test/CodeGenOpenCL/kernel-arg-info.cl Fri Apr 4 08:43:57 2014 @@ -18,3 +18,11 @@ kernel void foo2(read_only image1d_t img // CHECK: metadata !{metadata !"kernel_arg_type", metadata !"image1d_t", metadata !"image2d_t", metadata !"image2d_array_t"} // CHECK: metadata !{metadata !"kernel_arg_type_qual", metadata !"", metadata !"", metadata !""} // CHECK: metadata !{metadata !"kernel_arg_name", metadata !"img1", metadata !"img2", metadata !"img3"} + +kernel void foo3(__global half * X) { +} +// CHECK: metadata !{metadata !"kernel_arg_addr_space", i32 1} +// CHECK: metadata !{metadata !"kernel_arg_access_qual", metadata !"none"} +// CHECK: metadata !{metadata !"kernel_arg_type", metadata !"half*"} +// CHECK: metadata !{metadata !"kernel_arg_type_qual", metadata !""} +// CHECK: metadata !{metadata !"kernel_arg_name", metadata !"X"} ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[PATCH] Handle properly somewhat special cases of -main-file-name
The handling of -main-file-name in CGDebugInfo::CreateCompileUnit() can result in incorrect DW_AT_name in somewhat special cases: 1) $ touch /tmp/a.cpp $ clang++ -Wall -c /tmp/a.cpp -g -o /tmp/a.o -Xclang -main-file-name -Xclang /new/path/a.cpp $ readelf -wi /tmp/a.o | grep DW_AT_name <12> DW_AT_name: (indirect string, offset: 0x15): /tmp/new/path/a.cpp 2) $ touch /tmp/a.cpp $ cd / $ cat /tmp/a.cpp | clang++ -Wall -x c++ -c - -g -o /tmp/a.o -Xclang -main-file-name -Xclang a.cpp $ readelf -wi /tmp/a.o | grep DW_AT_name <12> DW_AT_name: (indirect string, offset: 0x15): /a.cpp The attached patch fixes those. Ok to commit? -- Lubos Lunak From 4d532ab3ae69357e98dc50c02944b30be156090a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Fri, 4 Apr 2014 13:51:49 +0200 Subject: [PATCH] handle properly somewhat special cases of -main-file-name - if the name passed is an absolute path, do not try to prepend anything - do not prepend "/" if source comes from stdin and the current dir is / (the directory for will be originally considered to be "." too, but caching in file handling will replace it with equal "/") --- lib/CodeGen/CGDebugInfo.cpp | 25 + 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/CodeGen/CGDebugInfo.cpp b/lib/CodeGen/CGDebugInfo.cpp index 0e94b51..c580770 100644 --- a/lib/CodeGen/CGDebugInfo.cpp +++ b/lib/CodeGen/CGDebugInfo.cpp @@ -322,18 +322,19 @@ void CGDebugInfo::CreateCompileUnit() { std::string MainFileName = CGM.getCodeGenOpts().MainFileName; if (MainFileName.empty()) MainFileName = ""; - - // The main file name provided via the "-main-file-name" option contains just - // the file name itself with no path information. This file name may have had - // a relative path, so we look into the actual file entry for the main - // file to determine the real absolute path for the file. - std::string MainFileDir; - if (const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID())) { -MainFileDir = MainFile->getDir()->getName(); -if (MainFileDir != ".") { - llvm::SmallString<1024> MainFileDirSS(MainFileDir); - llvm::sys::path::append(MainFileDirSS, MainFileName); - MainFileName = MainFileDirSS.str(); + else if (!llvm::sys::path::is_absolute(MainFileName)) { +// The main file name provided via the "-main-file-name" option contains just +// the file name itself with no path information. This file name may have had +// a relative path, so we look into the actual file entry for the main +// file to determine the real absolute path for the file. +std::string MainFileDir; +if (const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID())) { + MainFileDir = MainFile->getDir()->getName(); + if (MainFileDir != "." && MainFile->getName() != StringRef("")) { +llvm::SmallString<1024> MainFileDirSS(MainFileDir); +llvm::sys::path::append(MainFileDirSS, MainFileName); +MainFileName = MainFileDirSS.str(); + } } } -- 1.8.4.5 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] OpenCL - use AST PrintingPolicy when emitting kernel arg metadata
Oh, sorry, I don't have commit privileges. Could you do the honours? Thanks, Fraser On 03/04/14 18:40, Fraser Cormack wrote: Ah yes, sorry about that! An updated patch is attached. On 03/04/14 15:50, Joey Gouly wrote: If you add a test, it LGTM! -Original Message- From: cfe-commits-boun...@cs.uiuc.edu [mailto:cfe-commits-boun...@cs.uiuc.edu] On Behalf Of Fraser Cormack Sent: 03 April 2014 12:04 To: llvm cfe Subject: [PATCH] OpenCL - use AST PrintingPolicy when emitting kernel arg metadata Hi, This patch uses the AST PrintingPolicy when emitting OpenCL kernel arg metadata in order to pretty print the 'half' type, if supported. Cheers, Fraser ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits -- Fraser Cormack Compiler Developer Codeplay Software Ltd 45 York Place, Edinburgh, EH1 3HP Tel: 0131 466 0503 Fax: 0131 557 6600 Website: http://www.codeplay.com Twitter: https://twitter.com/codeplaysoft This email and any attachments may contain confidential and /or privileged information and is for use by the addressee only. If you are not the intended recipient, please notify Codeplay Software Ltd immediately and delete the message from your computer. You may not copy or forward it,or use or disclose its contents to any other person. Any views or other information in this message which do not relate to our business are not authorized by Codeplay software Ltd, nor does this message form part of any contract unless so stated. As internet communications are capable of data corruption Codeplay Software Ltd does not accept any responsibility for any changes made to this message after it was sent. Please note that Codeplay Software Ltd does not accept any liability or responsibility for viruses and it is your responsibility to scan any attachments. Company registered in England and Wales, number: 04567874 Registered office: 81 Linkfield Street, Redhill RH1 6BY ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Improve error recovery around colon.
Thank you for feedback! Comment at: include/clang/Basic/DiagnosticParseKinds.td:342-343 @@ -341,1 +341,4 @@ "unexpected ':' in nested name specifier; did you mean '::'?">; +def err_unexected_token_in_nested_name_spec : Error< + "unexpected '%0' in nested name specifier; did you mean ':'?">; +def err_nested_name_spec_is_not_class : Error< Richard Smith wrote: > Either the second thing here should be `%1` (and you pass in `tok::colon`), > or this diagnostic should have a better name. `"did you mean ':'?"` is not an > obvious thing to say about a nested name specifier =) > > Also, using `%1` will let you merge this with the previous diagnostic. > > While you're here, please change `unexected` to `unexpected` in both > diagnostic names. :) Yes, I see. Tried to make more descriptive message. Also changed typos in diagnostic names :) Comment at: lib/Parse/ParseExprCXX.cpp:470-483 @@ -452,1 +469,16 @@ + ObjectType, EnteringContext, SS, + ErrMode, &IdDecl)) { +// Identifier is not recognized as a nested name, but we can have +// mistyped '::' instead of ':'. +if (ColonIsSacred && IdDecl) { + Diag(CCLoc, diag::err_nested_name_spec_is_not_class) + << &II << getLangOpts().CPlusPlus + << FixItHint::CreateReplacement(CCLoc, ":"); + Diag(IdDecl->getLocation(), diag::note_declared_at); + ColonColon.setKind(tok::colon); + PP.EnterToken(Tok); + PP.EnterToken(ColonColon); + Tok = Identifier; + break; +} SS.SetInvalid(SourceRange(IdLoc, CCLoc)); Richard Smith wrote: > I think it'd be cleaner to pass a `bool *CorrectToColon` into > `ActOnCXXNestedNameSpecifier`, and issue the diagnostics there (so this > diagnostic is not split into two disparate places). Then, if the flag is set, > just fix up the token stream here. This is good approach. Looks like patch becomes more compact. Comment at: lib/Sema/SemaCXXScopeSpec.cpp:521 @@ +520,3 @@ +<< &Identifier << getLangOpts().CPlusPlus; + if (NamedDecl *ND = R.getAsSingle()) { +if (RecoveryMode == ReportAll) Richard Smith wrote: > This will silently ignore the error if you're in `DontReportWrongMember` mode > and name lookup finds something other than a single result. Remade this test. http://llvm-reviews.chandlerc.com/D2870 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r205506 - [OPENMP] Small update for C++11
On Thu, Apr 3, 2014 at 11:22 PM, Alexey Bataev wrote: > Ok, I'll rename them all to VE (var expr, because actually it is > declrefexprs with refs to vardecls). Is it acceptable? Seems reasonable to me. Thanks! ~Aaron > > > Best regards, > Alexey Bataev > = > Software Engineer > Intel Compiler Team > Intel Corp. > > 4 Апрель 2014 г. 6:51:29, Aaron Ballman писал: > >> On Thu, Apr 3, 2014 at 10:47 PM, Alexey Bataev >> wrote: >>> >>> Hi Richard, >>> Thanks for review. >>> But I made this changes just to be fully compatible with the previous one >>> made by Aaron Ballman (see changes in revision 203937). >> >> >> Most of my changes retained the original variable name. In retrospect, >> I probably should have used it as an opportunity to improve the >> identifiers. >> >> ~Aaron ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [PATCH] Improve error recovery around colon.
Updated patch Hi rsmith, http://llvm-reviews.chandlerc.com/D2870 CHANGE SINCE LAST DIFF http://llvm-reviews.chandlerc.com/D2870?vs=7862&id=8368#toc Files: include/clang/Basic/DiagnosticParseKinds.td include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Parse/ParseExprCXX.cpp lib/Parse/ParseStmt.cpp lib/Parse/ParseTemplate.cpp lib/Sema/SemaCXXScopeSpec.cpp lib/Sema/SemaDecl.cpp test/Parser/recovery.cpp test/SemaCXX/nested-name-spec.cpp Index: include/clang/Basic/DiagnosticParseKinds.td === --- include/clang/Basic/DiagnosticParseKinds.td +++ include/clang/Basic/DiagnosticParseKinds.td @@ -339,8 +339,10 @@ "cannot template a using directive">; def err_templated_using_declaration : Error< "cannot template a using declaration">; -def err_unexected_colon_in_nested_name_spec : Error< +def err_unexpected_colon_in_nested_name_spec : Error< "unexpected ':' in nested name specifier; did you mean '::'?">; +def err_unexpected_token_in_nested_name_spec : Error< + "'%0' cannot be a part of nested name specifier; did you mean ':'?">; def err_bool_redeclaration : Error< "redeclaration of C++ built-in type 'bool'">; def ext_c11_static_assert : Extension< Index: include/clang/Basic/DiagnosticSemaKinds.td === --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -1168,7 +1168,10 @@ def warn_cxx98_compat_enum_nested_name_spec : Warning< "enumeration type in nested name specifier is incompatible with C++98">, InGroup, DefaultIgnore; - +def err_nested_name_spec_is_not_class : Error< + "%0 cannot appear before '::' because it is not a class" + "%select{ or namespace|, namespace, or scoped enumeration}1; did you mean ':'?">; + // C++ class members def err_storageclass_invalid_for_member : Error< "storage class specified for a member declaration">; Index: include/clang/Sema/Sema.h === --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -4386,7 +4386,8 @@ bool EnteringContext, CXXScopeSpec &SS, NamedDecl *ScopeLookupResult, - bool ErrorRecoveryLookup); + bool ErrorRecoveryLookup, + bool *IsCorrectedToColon = 0); /// \brief The parser has parsed a nested-name-specifier 'identifier::'. /// @@ -4409,14 +4410,23 @@ /// output parameter (containing the full nested-name-specifier, /// including this new type). /// + /// \param ErrorRecoveryLookup If true, then this method is called to improve + /// error recovery. In this case do not emit error message. + /// + /// \param IsCorrectedToColon If not null, suggestions to replace '::' -> ':' + /// are allowed. The bool value pointed by this parameter is set to 'true' + /// if the identifier is treated as if it was followed by ':', not '::'. + /// /// \returns true if an error occurred, false otherwise. bool ActOnCXXNestedNameSpecifier(Scope *S, IdentifierInfo &Identifier, SourceLocation IdentifierLoc, SourceLocation CCLoc, ParsedType ObjectType, bool EnteringContext, - CXXScopeSpec &SS); + CXXScopeSpec &SS, + bool ErrorRecoveryLookup = false, + bool *IsCorrectedToColon = 0); ExprResult ActOnDecltypeExpression(Expr *E); Index: lib/Parse/ParseExprCXX.cpp === --- lib/Parse/ParseExprCXX.cpp +++ lib/Parse/ParseExprCXX.cpp @@ -365,15 +365,15 @@ // Consume the template-id token. ConsumeToken(); - + assert(Tok.is(tok::coloncolon) && "NextToken() not working properly!"); SourceLocation CCLoc = ConsumeToken(); HasScopeSpecifier = true; - + ASTTemplateArgsPtr TemplateArgsPtr(TemplateId->getTemplateArgs(), TemplateId->NumArgs); - + if (Actions.ActOnCXXNestedNameSpecifier(getCurScope(), SS, TemplateId->TemplateKWLoc, @@ -393,7 +393,6 @@ continue; } - // The rest of the nested-name-specifier possibilities start with // tok::identifier. if (Tok.isNot(tok::identifier)) @@ -418,9 +417,8 @@ // error, but they probably meant something else strange so don't // recover like this. PP.LookAhead(1).is(tok::identifier)
[PATCH] PR12463 : Warnings about nonsensical comparisons are disabled if macro expansion is involved #2
This is a reduced version of the original patch for PR12463 that handles only string comparisons (I don't care about arrays enough to deal with the details and there the problem is unlikely to happen anyway, but the problem is more likely with strings if e.g. refactoring). -- Lubos Lunak From aa0ab874f8052d33aa256840d898b71e983f6388 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Wed, 27 Nov 2013 17:36:30 +0100 Subject: [PATCH] warn about string literal comparisons even in macros (pr12463) They are unspecified even in macros. --- lib/Sema/SemaExpr.cpp | 12 +++- test/Sema/exprs.c | 8 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 7894682..dbcc690 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -7783,22 +7783,24 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS, if (!LHSType->hasFloatingRepresentation() && !(LHSType->isBlockPointerType() && IsRelational) && - !LHS.get()->getLocStart().isMacroID() && - !RHS.get()->getLocStart().isMacroID() && ActiveTemplateInstantiations.empty()) { // For non-floating point types, check for self-comparisons of the form // x == x, x != x, x < x, etc. These always evaluate to a constant, and // often indicate logic errors in the program. // // NOTE: Don't warn about comparison expressions resulting from macro -// expansion. Also don't warn about comparisons which are only self +// expansion, unless they do not make any sense at all. +// Also don't warn about comparisons which are only self // comparisons within a template specialization. The warnings should catch // obvious cases in the definition of the template anyways. The idea is to // warn when the typed comparison operator will always evaluate to the same // result. +bool InMacro = LHS.get()->getLocStart().isMacroID() || + RHS.get()->getLocStart().isMacroID(); ValueDecl *DL = getCompareDecl(LHSStripped); ValueDecl *DR = getCompareDecl(RHSStripped); -if (DL && DR && DL == DR && !IsWithinTemplateSpecialization(DL)) { +if (DL && DR && DL == DR && !IsWithinTemplateSpecialization(DL) && +!InMacro) { DiagRuntimeBehavior(Loc, 0, PDiag(diag::warn_comparison_always) << 0 // self- << (Opc == BO_EQ @@ -7806,7 +7808,7 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS, || Opc == BO_GE)); } else if (DL && DR && LHSType->isArrayType() && RHSType->isArrayType() && !DL->getType()->isReferenceType() && - !DR->getType()->isReferenceType()) { + !DR->getType()->isReferenceType() && !InMacro) { // what is it always going to eval to? char always_evals_to; switch(Opc) { diff --git a/test/Sema/exprs.c b/test/Sema/exprs.c index 2fb17e4..3e90146 100644 --- a/test/Sema/exprs.c +++ b/test/Sema/exprs.c @@ -125,6 +125,14 @@ int test12b(const char *X) { return sizeof(X == "foo"); // no-warning } +// PR12463 +#define FOO_LITERAL "foo" +#define STRING_LITERAL_COMPARE "a" == "b" +int test12c(const char *X) { + return X == FOO_LITERAL; // expected-warning {{comparison against a string literal is unspecified (use strncmp instead)}} + return STRING_LITERAL_COMPARE; // expected-warning {{comparison against a string literal is unspecified (use strncmp instead)}} +} + // rdar://6719156 void test13( void (^P)()) { // expected-error {{blocks support disabled - compile with -fblocks}} -- 1.8.4.5 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[PATCH] PR15614 : -frewrite-includes causes -Wunused-macros false positives #2
This is a re-send of http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131125/094231.html which went without any answer. See PR15614 for a testcase. -- Lubos Lunak From 710be67e5d01095a723a032676ea178193172b68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Sun, 28 Jul 2013 11:32:55 +0200 Subject: [PATCH] do not emit -Wunused-macros warnings in -frewrite-includes mode (PR15614) -frewrite-includes calls PP.SetMacroExpansionOnlyInDirectives() to avoid macro expansions that are useless in that mode, but this can lead to -Wunused-macros false positives. As -frewrite-includes does not emit normal warnings, block -Wunused-macros too. --- lib/Lex/PPDirectives.cpp | 5 + test/Frontend/rewrite-includes-warnings.c | 7 ++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp index 57dc495..f0e9b29 100644 --- a/lib/Lex/PPDirectives.cpp +++ b/lib/Lex/PPDirectives.cpp @@ -647,6 +647,9 @@ public: pp->DisableMacroExpansion = false; } ~ResetMacroExpansionHelper() { +restore(); + } + void restore() { PP->DisableMacroExpansion = save; } private: @@ -758,6 +761,7 @@ void Preprocessor::HandleDirective(Token &Result) { // C99 6.10.3 - Macro Replacement. case tok::pp_define: + helper.restore(); return HandleDefineDirective(Result, ImmediatelyAfterTopLevelIfndef); case tok::pp_undef: return HandleUndefDirective(Result); @@ -2105,6 +2109,7 @@ void Preprocessor::HandleDefineDirective(Token &DefineTok, // If we need warning for not using the macro, add its location in the // warn-because-unused-macro set. If it gets used it will be removed from set. if (getSourceManager().isInMainFile(MI->getDefinitionLoc()) && + !DisableMacroExpansion && Diags->getDiagnosticLevel(diag::pp_macro_not_used, MI->getDefinitionLoc()) != DiagnosticsEngine::Ignored) { MI->setIsWarnIfUnused(true); diff --git a/test/Frontend/rewrite-includes-warnings.c b/test/Frontend/rewrite-includes-warnings.c index 1fb98db..c4f38ed 100644 --- a/test/Frontend/rewrite-includes-warnings.c +++ b/test/Frontend/rewrite-includes-warnings.c @@ -1,4 +1,9 @@ -// RUN: %clang_cc1 -verify -Wall -Wextra -E -frewrite-includes %s +// RUN: %clang_cc1 -verify -Wall -Wextra -Wunused-macros -E -frewrite-includes %s // expected-no-diagnostics +// PR14831 #pragma GCC visibility push (default) + +// PR15614 +#define FOO 1 +int foo = FOO; -- 1.8.4.5 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[PATCH] do not warn about unknown pragmas in modes that do not handle them (pr9537) #2
This is a re-send of the patch from http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20131202/094696.html , which went without any answers. As can be seen in that mail, the original version of the patch has already been committed as r196372 but was then reverted because of a problem that this new version solves. -- Lubos Lunak From ff544b38587446d5a89e430a4a7ee7a777c867e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Wed, 27 Nov 2013 23:42:16 +0100 Subject: [PATCH] do not warn about unknown pragmas in modes that do not handle them (pr9537) And refactor to have just one place in code that sets up the empty pragma handlers. --- include/clang/Lex/Preprocessor.h | 3 +++ lib/Frontend/FrontendActions.cpp | 2 +- lib/Frontend/PrintPreprocessedOutput.cpp | 2 +- lib/Lex/Pragma.cpp | 22 ++ lib/Rewrite/Frontend/InclusionRewriter.cpp | 8 +--- test/Preprocessor/ignore-pragmas.c | 10 ++ 6 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 test/Preprocessor/ignore-pragmas.c diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h index 2491011..6822424 100644 --- a/include/clang/Lex/Preprocessor.h +++ b/include/clang/Lex/Preprocessor.h @@ -643,6 +643,9 @@ public: RemovePragmaHandler(StringRef(), Handler); } + /// Install empty handlers for all pragmas (making them ignored). + void IgnorePragmas(); + /// \brief Add the specified comment handler to the preprocessor. void addCommentHandler(CommentHandler *Handler); diff --git a/lib/Frontend/FrontendActions.cpp b/lib/Frontend/FrontendActions.cpp index a3ab1be..88044f2 100644 --- a/lib/Frontend/FrontendActions.cpp +++ b/lib/Frontend/FrontendActions.cpp @@ -485,7 +485,7 @@ void PreprocessOnlyAction::ExecuteAction() { Preprocessor &PP = getCompilerInstance().getPreprocessor(); // Ignore unknown pragmas. - PP.AddPragmaHandler(new EmptyPragmaHandler()); + PP.IgnorePragmas(); Token Tok; // Start parsing the specified input file. diff --git a/lib/Frontend/PrintPreprocessedOutput.cpp b/lib/Frontend/PrintPreprocessedOutput.cpp index f3393bf..87fbd04 100644 --- a/lib/Frontend/PrintPreprocessedOutput.cpp +++ b/lib/Frontend/PrintPreprocessedOutput.cpp @@ -704,7 +704,7 @@ static int MacroIDCompare(const id_macro_pair *LHS, const id_macro_pair *RHS) { static void DoPrintMacros(Preprocessor &PP, raw_ostream *OS) { // Ignore unknown pragmas. - PP.AddPragmaHandler(new EmptyPragmaHandler()); + PP.IgnorePragmas(); // -dM mode just scans and ignores all tokens in the files, then dumps out // the macro table at the end. diff --git a/lib/Lex/Pragma.cpp b/lib/Lex/Pragma.cpp index e4059ee..b9f40d9 100644 --- a/lib/Lex/Pragma.cpp +++ b/lib/Lex/Pragma.cpp @@ -1401,3 +1401,25 @@ void Preprocessor::RegisterBuiltinPragmas() { AddPragmaHandler(new PragmaRegionHandler("endregion")); } } + +/// Ignore all pragmas, useful for modes such as -Eonly which would otherwise +/// warn about those pragmas being unknown. +void Preprocessor::IgnorePragmas() { + AddPragmaHandler(new EmptyPragmaHandler()); + // Also ignore all pragmas in all namespaces created + // in Preprocessor::RegisterBuiltinPragmas(). + AddPragmaHandler("GCC", new EmptyPragmaHandler()); + AddPragmaHandler("clang", new EmptyPragmaHandler()); + if (PragmaHandler *NS = PragmaHandlers->FindHandler("STDC")) { +// Preprocessor::RegisterBuiltinPragmas() already registers +// PragmaSTDC_UnknownHandler as the empty handler, so remove it first, +// otherwise there will be an assert about a duplicate handler. +PragmaNamespace *STDCNamespace = NS->getIfNamespace(); +assert(STDCNamespace && + "Invalid namespace, registered as a regular pragma handler!"); +if (PragmaHandler *Existing = STDCNamespace->FindHandler("", false)) { + RemovePragmaHandler("STDC", Existing); +} + } + AddPragmaHandler("STDC", new EmptyPragmaHandler()); +} diff --git a/lib/Rewrite/Frontend/InclusionRewriter.cpp b/lib/Rewrite/Frontend/InclusionRewriter.cpp index 176ea3f..a3b6c49 100644 --- a/lib/Rewrite/Frontend/InclusionRewriter.cpp +++ b/lib/Rewrite/Frontend/InclusionRewriter.cpp @@ -518,13 +518,7 @@ void clang::RewriteIncludesInInput(Preprocessor &PP, raw_ostream *OS, InclusionRewriter *Rewrite = new InclusionRewriter(PP, *OS, Opts.ShowLineMarkers); PP.addPPCallbacks(Rewrite); - // Ignore all pragmas, otherwise there will be warnings about unknown pragmas - // (because there's nothing to handle them). - PP.AddPragmaHandler(new EmptyPragmaHandler()); - // Ignore also all pragma in all namespaces created - // in Preprocessor::RegisterBuiltinPragmas(). - PP.AddPragmaHandler("GCC", new EmptyPragmaHandler()); - PP.AddPragmaHandler("clang", new EmptyPragmaHandler()); + PP.IgnorePragmas(); // First let the preprocessor
[PATCH] Do not use "1" in linemarker for main file in -frewrite-includes
The -frewrite-includes flag incorrectly uses at the beginning a linemarker for the main file with the "1" flag which suggests that the contents have been in fact included from another file. This for example disables -Wunused-macros, which warns only for macros from the main file: $ cat a.cpp #define FOO 1 $ clang++ -E -frewrite-includes a.cpp | clang++ -Wall -Wunused-macros -x c++ -c - $ clang++ -Wall -Wunused-macros -c a.cpp a.cpp:1:9: warning: macro is not used [-Wunused-macros] #define FOO 1 ^ 1 warning generated. The attached patch fixes the code to start the resulting file with the correct linemarker. -- Lubos Lunak From 27778c2cb8eac373636ea3f50cde73ddd770e79e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Thu, 20 Feb 2014 23:27:44 +0100 Subject: [PATCH 1/3] do not use "1" for line marker for the main file "1" means entering a new file (from a different one), but the main file is not included from anything (and this would e.g. confuse -Wunused-macros to not report unused macros in the main file, see pr15610). The line marker is still useful e.g. if the resulting file is renamed or used via a pipe. --- lib/Rewrite/Frontend/InclusionRewriter.cpp | 7 +-- test/Frontend/rewrite-includes.c | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/Rewrite/Frontend/InclusionRewriter.cpp b/lib/Rewrite/Frontend/InclusionRewriter.cpp index 058960d..3704760 100644 --- a/lib/Rewrite/Frontend/InclusionRewriter.cpp +++ b/lib/Rewrite/Frontend/InclusionRewriter.cpp @@ -352,8 +352,11 @@ bool InclusionRewriter::Process(FileID FileId, StringRef EOL = DetectEOL(FromFile); - // Per the GNU docs: "1" indicates the start of a new file. - WriteLineInfo(FileName, 1, FileType, EOL, " 1"); + // Per the GNU docs: "1" indicates entering a new file. + if (FileId == SM.getMainFileID()) +WriteLineInfo(FileName, 1, FileType, EOL, ""); + else if (FileId != PP.getPredefinesFileID()) +WriteLineInfo(FileName, 1, FileType, EOL, " 1"); if (SM.getFileIDSize(FileId) == 0) return false; diff --git a/test/Frontend/rewrite-includes.c b/test/Frontend/rewrite-includes.c index 2158dd0..619d34a 100644 --- a/test/Frontend/rewrite-includes.c +++ b/test/Frontend/rewrite-includes.c @@ -21,6 +21,7 @@ A(1,2) #include "rewrite-includes7.h" #include "rewrite-includes8.h" // ENDCOMPARE +// CHECK: {{^}}# 1 "{{.*}}rewrite-includes.c"{{$}} // CHECK: {{^}}// STARTCOMPARE{{$}} // CHECK-NEXT: {{^}}#define A(a,b) a ## b{{$}} // CHECK-NEXT: {{^}}A(1,2){{$}} -- 1.8.4.5 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[PATCH] Fix "In file included from" for files generated with -frewrite-includes
Option -frewrite-includes comments out #include directives it replaces by enclosing it in #if 0, which moves the included contents, changing their perceived line numbers e.g. in the "In file included from" messages in a follow-up compilation: $ cat b.cpp #include "b.h" $ cat b.h void f() { int unused_variable; } $ clang++ -E -frewrite-includes b.cpp | clang++ -Wall -x c++ -c - In file included from b.cpp:4: ./b.h:3:9: warning: unused variable 'unused_variable' [-Wunused-variable] int unused_variable; ^ 1 warning generated. $ clang++ -Wall -c b.cpp In file included from b.cpp:1: ./b.h:3:9: warning: unused variable 'unused_variable' [-Wunused-variable] int unused_variable; ^ 1 warning generated. The attached patch fixes this. -- Lubos Lunak From 2ccc4c6aa5b7334970fa699a1b99657fa93e9ae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Fri, 21 Feb 2014 00:05:53 +0100 Subject: [PATCH 2/3] write a line marker right before adding included file Enclosing the original #include directive inside #if 0 adds lines, so warning/errors messages would have the line number off in "In file included from ::", so add line marker to fix this. --- lib/Rewrite/Frontend/InclusionRewriter.cpp | 1 + test/Frontend/Inputs/rewrite-includes-messages.h | 4 test/Frontend/rewrite-includes-messages.c| 8 test/Frontend/rewrite-includes-missing.c | 1 + test/Frontend/rewrite-includes-modules.c | 2 ++ test/Frontend/rewrite-includes.c | 10 ++ 6 files changed, 26 insertions(+) create mode 100644 test/Frontend/Inputs/rewrite-includes-messages.h create mode 100644 test/Frontend/rewrite-includes-messages.c diff --git a/lib/Rewrite/Frontend/InclusionRewriter.cpp b/lib/Rewrite/Frontend/InclusionRewriter.cpp index 3704760..cc087d1 100644 --- a/lib/Rewrite/Frontend/InclusionRewriter.cpp +++ b/lib/Rewrite/Frontend/InclusionRewriter.cpp @@ -386,6 +386,7 @@ bool InclusionRewriter::Process(FileID FileId, case tok::pp_import: { CommentOutDirective(RawLex, HashToken, FromFile, EOL, NextToWrite, Line); +WriteLineInfo(FileName, Line - 1, FileType, EOL, ""); StringRef LineInfoExtra; if (const FileChange *Change = FindFileChangeLocation( HashToken.getLocation())) { diff --git a/test/Frontend/Inputs/rewrite-includes-messages.h b/test/Frontend/Inputs/rewrite-includes-messages.h new file mode 100644 index 000..e5f0eb2 --- /dev/null +++ b/test/Frontend/Inputs/rewrite-includes-messages.h @@ -0,0 +1,4 @@ +void f() +{ +int unused_variable; +} diff --git a/test/Frontend/rewrite-includes-messages.c b/test/Frontend/rewrite-includes-messages.c new file mode 100644 index 000..37b9706 --- /dev/null +++ b/test/Frontend/rewrite-includes-messages.c @@ -0,0 +1,8 @@ +// RUN: %clang -E -frewrite-includes %s -I%S/Inputs/ | %clang -Wall -Wunused-macros -x c -c - 2> %t.1 +// RUN: %clang -I%S/Inputs/ -Wall -Wunused-macros -c %s 2> %t.2 +// RUN: cmp -s %t.1 %t.2 +// expected-no-diagnostics +// REQUIRES: shell + +#include "rewrite-includes-messages.h" +#define UNUSED_MACRO diff --git a/test/Frontend/rewrite-includes-missing.c b/test/Frontend/rewrite-includes-missing.c index da4e209..25a59a0 100644 --- a/test/Frontend/rewrite-includes-missing.c +++ b/test/Frontend/rewrite-includes-missing.c @@ -4,4 +4,5 @@ // CHECK: {{^}}#if 0 /* expanded by -frewrite-includes */{{$}} // CHECK-NEXT: {{^}}#include "foobar.h" // CHECK-NEXT: {{^}}#endif /* expanded by -frewrite-includes */{{$}} +// CHECK-NEXT: {{^}}# 3 "{{.*}}rewrite-includes-missing.c"{{$}} // CHECK-NEXT: {{^}}# 4 "{{.*}}rewrite-includes-missing.c"{{$}} diff --git a/test/Frontend/rewrite-includes-modules.c b/test/Frontend/rewrite-includes-modules.c index 783a967..58d7809 100644 --- a/test/Frontend/rewrite-includes-modules.c +++ b/test/Frontend/rewrite-includes-modules.c @@ -10,11 +10,13 @@ int foo(); // CHECK-NEXT: #if 0 /* expanded by -frewrite-includes */{{$}} // CHECK-NEXT: #include {{$}} // CHECK-NEXT: #endif /* expanded by -frewrite-includes */{{$}} +// CHECK-NEXT: # 5 "{{.*[/\\]}}rewrite-includes-modules.c"{{$}} // CHECK-NEXT: @import Module; /* clang -frewrite-includes: implicit import */{{$}} // CHECK-NEXT: # 6 "{{.*[/\\]}}rewrite-includes-modules.c"{{$}} // CHECK-NEXT: int foo();{{$}} // CHECK-NEXT: #if 0 /* expanded by -frewrite-includes */{{$}} // CHECK-NEXT: #include {{$}} // CHECK-NEXT: #endif /* expanded by -frewrite-includes */{{$}} +// CHECK-NEXT: # 7 "{{.*[/\\]}}rewrite-includes-modules.c"{{$}} // CHECK-NEXT: @import Module; /* clang -frewrite-includes: implicit import */{{$}} // CHECK-NEXT: # 8 "{{.*[/\\]}}rewrite-includes-modules.c"{{$}} diff --git a/test/Frontend/rewrite-includes.c b/test/Frontend/rewrite-includes.c index 619d34a..bed87ef 100644 --- a/test/Frontend/rewrite-includes.c +++ b/test/Frontend/rewrite-includes.c @@ -28,6 +28,7 @@ A(1,2
r205620 - [OPENMP][C++11] Renamed loop vars properly.
Author: abataev Date: Fri Apr 4 05:02:14 2014 New Revision: 205620 URL: http://llvm.org/viewvc/llvm-project?rev=205620&view=rev Log: [OPENMP][C++11] Renamed loop vars properly. Modified: cfe/trunk/lib/Sema/TreeTransform.h cfe/trunk/lib/Serialization/ASTWriterStmt.cpp Modified: cfe/trunk/lib/Sema/TreeTransform.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=205620&r1=205619&r2=205620&view=diff == --- cfe/trunk/lib/Sema/TreeTransform.h (original) +++ cfe/trunk/lib/Sema/TreeTransform.h Fri Apr 4 05:02:14 2014 @@ -6387,8 +6387,8 @@ OMPClause * TreeTransform::TransformOMPPrivateClause(OMPPrivateClause *C) { llvm::SmallVector Vars; Vars.reserve(C->varlist_size()); - for (auto *I : C->varlists()) { -ExprResult EVar = getDerived().TransformExpr(cast(I)); + for (auto *VE : C->varlists()) { +ExprResult EVar = getDerived().TransformExpr(cast(VE)); if (EVar.isInvalid()) return 0; Vars.push_back(EVar.take()); @@ -6405,8 +6405,8 @@ TreeTransform::TransformOMPFirs OMPFirstprivateClause *C) { llvm::SmallVector Vars; Vars.reserve(C->varlist_size()); - for (auto *I : C->varlists()) { -ExprResult EVar = getDerived().TransformExpr(cast(I)); + for (auto *VE : C->varlists()) { +ExprResult EVar = getDerived().TransformExpr(cast(VE)); if (EVar.isInvalid()) return 0; Vars.push_back(EVar.take()); @@ -6422,8 +6422,8 @@ OMPClause * TreeTransform::TransformOMPSharedClause(OMPSharedClause *C) { llvm::SmallVector Vars; Vars.reserve(C->varlist_size()); - for (auto *I : C->varlists()) { -ExprResult EVar = getDerived().TransformExpr(cast(I)); + for (auto *VE : C->varlists()) { +ExprResult EVar = getDerived().TransformExpr(cast(VE)); if (EVar.isInvalid()) return 0; Vars.push_back(EVar.take()); @@ -6439,8 +6439,8 @@ OMPClause * TreeTransform::TransformOMPCopyinClause(OMPCopyinClause *C) { llvm::SmallVector Vars; Vars.reserve(C->varlist_size()); - for (auto *I : C->varlists()) { -ExprResult EVar = getDerived().TransformExpr(cast(I)); + for (auto *VE : C->varlists()) { +ExprResult EVar = getDerived().TransformExpr(cast(VE)); if (EVar.isInvalid()) return 0; Vars.push_back(EVar.take()); Modified: cfe/trunk/lib/Serialization/ASTWriterStmt.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterStmt.cpp?rev=205620&r1=205619&r2=205620&view=diff == --- cfe/trunk/lib/Serialization/ASTWriterStmt.cpp (original) +++ cfe/trunk/lib/Serialization/ASTWriterStmt.cpp Fri Apr 4 05:02:14 2014 @@ -1698,29 +1698,29 @@ void OMPClauseWriter::VisitOMPDefaultCla void OMPClauseWriter::VisitOMPPrivateClause(OMPPrivateClause *C) { Record.push_back(C->varlist_size()); Writer->Writer.AddSourceLocation(C->getLParenLoc(), Record); - for (auto *I : C->varlists()) -Writer->Writer.AddStmt(I); + for (auto *VE : C->varlists()) +Writer->Writer.AddStmt(VE); } void OMPClauseWriter::VisitOMPFirstprivateClause(OMPFirstprivateClause *C) { Record.push_back(C->varlist_size()); Writer->Writer.AddSourceLocation(C->getLParenLoc(), Record); - for (auto *I : C->varlists()) -Writer->Writer.AddStmt(I); + for (auto *VE : C->varlists()) +Writer->Writer.AddStmt(VE); } void OMPClauseWriter::VisitOMPSharedClause(OMPSharedClause *C) { Record.push_back(C->varlist_size()); Writer->Writer.AddSourceLocation(C->getLParenLoc(), Record); - for (auto *I : C->varlists()) -Writer->Writer.AddStmt(I); + for (auto *VE : C->varlists()) +Writer->Writer.AddStmt(VE); } void OMPClauseWriter::VisitOMPCopyinClause(OMPCopyinClause *C) { Record.push_back(C->varlist_size()); Writer->Writer.AddSourceLocation(C->getLParenLoc(), Record); - for (auto *I : C->varlists()) -Writer->Writer.AddStmt(I); + for (auto *VE : C->varlists()) +Writer->Writer.AddStmt(VE); } //===--===// ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: r205609 - Basic: rename VisualStudio to Windows
The change LGTM, just wanted to note the commit message is not in sync with the change. 2014-04-04 9:08 GMT+04:00 Saleem Abdulrasool : > Author: compnerd > Date: Fri Apr 4 00:08:53 2014 > New Revision: 205609 > > URL: http://llvm.org/viewvc/llvm-project?rev=205609&view=rev > Log: > Basic: rename VisualStudio to Windows > > Visual Studio is the Integrated Development Environment. The toolchain is > generally referred to MSVC. Rename the target information to be more precise > as > per the recommendation of Reid Kleckner. > > Modified: > cfe/trunk/lib/Basic/Targets.cpp > > Modified: cfe/trunk/lib/Basic/Targets.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=205609&r1=205608&r2=205609&view=diff > == > --- cfe/trunk/lib/Basic/Targets.cpp (original) > +++ cfe/trunk/lib/Basic/Targets.cpp Fri Apr 4 00:08:53 2014 > @@ -3066,9 +3066,9 @@ public: > namespace { > > // x86-32 Windows Visual Studio target > -class VisualStudioWindowsX86_32TargetInfo : public WindowsX86_32TargetInfo { > +class MicrosoftX86_32TargetInfo : public WindowsX86_32TargetInfo { > public: > - VisualStudioWindowsX86_32TargetInfo(const llvm::Triple &Triple) > + MicrosoftX86_32TargetInfo(const llvm::Triple &Triple) >: WindowsX86_32TargetInfo(Triple) { > LongDoubleWidth = LongDoubleAlign = 64; > LongDoubleFormat = &llvm::APFloat::IEEEdouble; > @@ -3300,9 +3300,9 @@ public: > > namespace { > // x86-64 Windows Visual Studio target > -class VisualStudioWindowsX86_64TargetInfo : public WindowsX86_64TargetInfo { > +class MicrosoftX86_64TargetInfo : public WindowsX86_64TargetInfo { > public: > - VisualStudioWindowsX86_64TargetInfo(const llvm::Triple &Triple) > + MicrosoftX86_64TargetInfo(const llvm::Triple &Triple) >: WindowsX86_64TargetInfo(Triple) { > LongDoubleWidth = LongDoubleAlign = 64; > LongDoubleFormat = &llvm::APFloat::IEEEdouble; > @@ -6290,7 +6290,7 @@ static TargetInfo *AllocateTarget(const >case llvm::Triple::GNU: > return new MinGWX86_32TargetInfo(Triple); >case llvm::Triple::MSVC: > -return new VisualStudioWindowsX86_32TargetInfo(Triple); > +return new MicrosoftX86_32TargetInfo(Triple); >} > } > case llvm::Triple::Haiku: > @@ -6333,7 +6333,7 @@ static TargetInfo *AllocateTarget(const >case llvm::Triple::GNU: > return new MinGWX86_64TargetInfo(Triple); >case llvm::Triple::MSVC: > -return new VisualStudioWindowsX86_64TargetInfo(Triple); > +return new MicrosoftX86_64TargetInfo(Triple); >} > } > case llvm::Triple::NaCl: > > > ___ > cfe-commits mailing list > cfe-commits@cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Re: [LLVMdev] 3.4.1 Release Plans
Hi, > > I think the latter option is the right choice. The pain of preserving the > > ABI belongs with our hard-working branch maintainers (and they always have > > the option of rejecting a change because it would break the ABI). > > Absolutely! > > --renato > Sorry, I 'm not sure I'm following you guys correctly. Do you mean we should not allow any LLVMWhatever.so ABI broken, so we need to guarantee the ordering of those intrinsic enums generated by Intrinsicsxxx.td? If yes, I don't think it's worthwhile tuning them with whatever tricky method to make abi-compliance-checker give 100% backward compatibility, so I'd want to give up adding the complete NEON support in 3.4.1 release. Therefore, plan B is to port the followings to 3.4.1 release only, CLANG: 198940 Enable -fuse-init-array for all AArch64 ELF targets by default, not just linux. LLVM: 198937 Make sure -use-init-array has intended effect on all AArch64 ELF targets, not just linux. 198941 Silence unused variable warning for non-asserting builds that was introduced in r198937. 199369 For ARM, fix assertuib failures for some ld/st 3/4 instruction with wirteback. 201541 Fix a typo about lowering AArch64 va_copy. 201841 [AArch64] Add register constraints to avoid generating STLXR and STXR with unpredictable behavior. 204304 [ARM]Fix an assertion failure in A15SDOptimizer about DPair reg class by treating DPair as QPR. I tried them and the regression tests can all pass. -- Thanks, -Jiangning ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[PATCH] Allow overriding -split-dwarf-file
The option -split-dwarf-file is passed by the driver to the compiler after processing -Xclang options, thus overriding any possible explicitly specified option: $ clang++ -c -gsplit-dwarf a.cpp -o a.o -Xclang -split-dwarf-file -Xclang b.dwo $ readelf -wi a.o | grep dwo_name DW_AT_GNU_dwo_name: (indirect string, offset: 0x0): a.dwo This is because the driver invokes the compiler as /usr/bin/clang-3.4 -cc1 ... -split-dwarf-file b.dwo -o a.o -x c++ a.cpp -split-dwarf-file a.dwo The attached patch fixes this. Ok to push? -- Lubos Lunak From 993f4a10cf4396ad107a58ff764290ad8b984c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lubo=C5=A1=20Lu=C5=88=C3=A1k?= Date: Fri, 4 Apr 2014 09:12:59 +0200 Subject: [PATCH] allow -split-dwarf-file to be overriden by command line The driver passed it after -XClang options, thus overriding anything that was given on the command line. --- lib/Driver/Tools.cpp | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp index 3671880..6262965 100644 --- a/lib/Driver/Tools.cpp +++ b/lib/Driver/Tools.cpp @@ -3804,6 +3804,18 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, // Forward -fparse-all-comments to -cc1. Args.AddAllArgs(CmdArgs, options::OPT_fparse_all_comments); + // Add the split debug info name to the command lines here so we + // can propagate it to the backend. + bool SplitDwarf = Args.hasArg(options::OPT_gsplit_dwarf) && +getToolChain().getTriple().isOSLinux() && +(isa(JA) || isa(JA)); + const char *SplitDwarfOut; + if (SplitDwarf) { +CmdArgs.push_back("-split-dwarf-file"); +SplitDwarfOut = SplitDebugName(Args, Inputs); +CmdArgs.push_back(SplitDwarfOut); + } + // Forward -Xclang arguments to -cc1, and -mllvm arguments to the LLVM option // parser. Args.AddAllArgValues(CmdArgs, options::OPT_Xclang); @@ -3862,18 +3874,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA, CmdArgs.push_back(Args.MakeArgString(Flags.str())); } - // Add the split debug info name to the command lines here so we - // can propagate it to the backend. - bool SplitDwarf = Args.hasArg(options::OPT_gsplit_dwarf) && -getToolChain().getTriple().isOSLinux() && -(isa(JA) || isa(JA)); - const char *SplitDwarfOut; - if (SplitDwarf) { -CmdArgs.push_back("-split-dwarf-file"); -SplitDwarfOut = SplitDebugName(Args, Inputs); -CmdArgs.push_back(SplitDwarfOut); - } - // Finally add the compile command to the compilation. if (Args.hasArg(options::OPT__SLASH_fallback) && Output.getType() == types::TY_Object) { -- 1.8.4.5 ___ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits