r177126 - Don't try to typo-correct 'super' in an objc method.

[Argyrios Kyrtzidis]

On Mar 15, 2013, at 5:13 PM, Richard Smith <richard at metafoo.co.uk> wrote:

> On Thu, Mar 14, 2013 at 3:56 PM, Argyrios Kyrtzidis <akyrtzi at gmail.com> wrote:
> Author: akirtzidis
> Date: Thu Mar 14 17:56:43 2013
> New Revision: 177126
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=177126&view=rev
> Log:
> Don't try to typo-correct 'super' in an objc method.
> 
> This created 2 issues:
> 
> 1) Performance issue, since typo-correction with PCH/modules is rather expensive.
> 2) Correctness issue, since if it managed to "correct" 'super' then bogus compiler errors would
> be emitted, like this:
> 
> 3.m:8:3: error: unknown type name 'super'; did you mean 'super1'?
>   super.x = 0;
>   ^~~~~
>   super1
> t3.m:5:13: note: 'super1' declared here
> typedef int super1;
>             ^
> t3.m:8:8: error: expected identifier or '('
>   super.x = 0;
>        ^
> 
> Could you use this as a test case, instead of adding a diagnostic? (I would expect those folks using -Weverything will find the new diagnostic rather confusing...)

Good point about -Weverything.
I removed the warning and switched to emitting a custom diagnostic using a diagnostic flag. r177218

Thanks for reviewing!


>  
> Added:
>     cfe/trunk/test/SemaObjC/typo-correction.m
> Modified:
>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
>     cfe/trunk/include/clang/Sema/Sema.h
>     cfe/trunk/lib/Sema/Sema.cpp
>     cfe/trunk/lib/Sema/SemaCodeComplete.cpp
>     cfe/trunk/lib/Sema/SemaLookup.cpp
> 
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=177126&r1=177125&r2=177126&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Mar 14 17:56:43 2013
> @@ -6074,6 +6074,8 @@ def warn_direct_ivar_access : Warning<"i
>    "directly accessed">, InGroup<DiagGroup<"direct-ivar-access">>, DefaultIgnore;
> 
>  // Spell-checking diagnostics
> +def warn_spellcheck_initiated : Warning<"spell-checking initiated for %0">,
> +  InGroup<DiagGroup<"spellcheck">>, DefaultIgnore;
>  def err_unknown_type_or_class_name_suggest : Error<
>    "unknown %select{type|class}2 name %0; did you mean %1?">;
>  def err_unknown_typename_suggest : Error<
> 
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=177126&r1=177125&r2=177126&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Mar 14 17:56:43 2013
> @@ -7430,6 +7430,8 @@ private:
>    /// The parser maintains this state here.
>    Scope *CurScope;
> 
> +  mutable IdentifierInfo *Ident_super;
> +
>  protected:
>    friend class Parser;
>    friend class InitializationSequence;
> @@ -7447,6 +7449,8 @@ public:
>    /// template substitution or instantiation.
>    Scope *getCurScope() const { return CurScope; }
> 
> +  IdentifierInfo *getSuperIdentifier() const;
> +
>    Decl *getObjCDeclContext() const;
> 
>    DeclContext *getCurLexicalContext() const {
> 
> Modified: cfe/trunk/lib/Sema/Sema.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=177126&r1=177125&r2=177126&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/Sema.cpp (original)
> +++ cfe/trunk/lib/Sema/Sema.cpp Thu Mar 14 17:56:43 2013
> @@ -90,7 +90,7 @@ Sema::Sema(Preprocessor &pp, ASTContext
>      AccessCheckingSFINAE(false), InNonInstantiationSFINAEContext(false),
>      NonInstantiationEntries(0), ArgumentPackSubstitutionIndex(-1),
>      CurrentInstantiationScope(0), TyposCorrected(0),
> -    AnalysisWarnings(*this)
> +    AnalysisWarnings(*this), Ident_super(0)
>  {
>    TUScope = 0;
> 
> @@ -1302,3 +1302,9 @@ bool Sema::tryToRecoverWithCall(ExprResu
>    E = ExprError();
>    return true;
>  }
> +
> +IdentifierInfo *Sema::getSuperIdentifier() const {
> +  if (!Ident_super)
> +    Ident_super = &Context.Idents.get("super");
> +  return Ident_super;
> +}
> 
> Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=177126&r1=177125&r2=177126&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Thu Mar 14 17:56:43 2013
> @@ -5315,7 +5315,7 @@ void Sema::CodeCompleteObjCSuperMessage(
>    } else {
>      // "super" may be the name of a type or variable. Figure out which
>      // it is.
> -    IdentifierInfo *Super = &Context.Idents.get("super");
> +    IdentifierInfo *Super = getSuperIdentifier();
>      NamedDecl *ND = LookupSingleName(S, Super, SuperLoc,
>                                       LookupOrdinaryName);
>      if ((CDecl = dyn_cast_or_null<ObjCInterfaceDecl>(ND))) {
> 
> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=177126&r1=177125&r2=177126&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu Mar 14 17:56:43 2013
> @@ -3734,6 +3734,16 @@ TypoCorrection Sema::CorrectTypo(const D
>    if (!ActiveTemplateInstantiations.empty())
>      return TypoCorrection();
> 
> +  // Don't try to correct 'super'.
> +  if (S && S->isInObjcMethodScope() && Typo == getSuperIdentifier())
> +    return TypoCorrection();
> +
> +  // This is for regression testing. It's disabled by default.
> +  if (Diags.getDiagnosticLevel(diag::warn_spellcheck_initiated,
> +                               TypoName.getLoc()) != DiagnosticsEngine::Ignored)
> +    Diag(TypoName.getLoc(), diag::warn_spellcheck_initiated)
> +      << TypoName.getName();
> +
>    NamespaceSpecifierSet Namespaces(Context, CurContext, SS);
> 
>    TypoCorrectionConsumer Consumer(*this, Typo);
> 
> Added: cfe/trunk/test/SemaObjC/typo-correction.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/typo-correction.m?rev=177126&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/typo-correction.m (added)
> +++ cfe/trunk/test/SemaObjC/typo-correction.m Thu Mar 14 17:56:43 2013
> @@ -0,0 +1,21 @@
> +// RUN: %clang_cc1 %s -verify -fsyntax-only -Wspellcheck
> +
> + at interface B
> + at property int x;
> + at end
> +
> + at interface S : B
> + at end
> +
> +// Spell-checking 'undefined' is ok.
> +undefined var; // expected-warning {{spell-checking initiated}} \
> +               // expected-error {{unknown type name}}
> +
> +typedef int super1;
> + at implementation S
> +-(void)foo {
> +  // Spell-checking 'super' is not ok.
> +  super.x = 0;
> +  self.x = 0;
> +}
> + at end
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130315/eab5825c/attachment.html>